GH-49548: [C++][FlightRPC] Decouple Flight Serialize/Deserialize from gRPC transport#49549
GH-49548: [C++][FlightRPC] Decouple Flight Serialize/Deserialize from gRPC transport#49549raulcd merged 3 commits intoapache:mainfrom
Conversation
|
|
|
@github-actions crossbow submit -g cpp |
|
Revision: e852d66 Submitted crossbow builds: ursacomputing/crossbow @ actions-c88270abef |
|
CI failures are unrelated. I've run some comparison benchmarks between the old code path and the new code path, the full info can be seen here: Basically run the following to validate: Summary:DoGet benchmark, 4 streams, 1M records/stream (arrow-flight-benchmark)
All values in MB/s. |
|
@zanmato1984 I saw you were taking a look, would you mind a review? |
|
Also cc @benibus in case the topic is of interest. |
lidavidm
left a comment
There was a problem hiding this comment.
Sorry for the late response, this seems reasonable to me.
Rationale for this change
Currently the Serialize/Deserialize APIs are gRPC dependent. This means that any code that needs to encode or decode Flight data must depend on gRPC C++ internals. After some discussions around trying to build a PoC using gRPC's generic API with gRPC's BidiReactor we discussed that these primitives should be made gRPC agnostic.
What changes are included in this PR?
cpp/src/arrow/flight/transport/grpc/serialization_internal.{h,cc}tocpp/src/arrow/flight/serialization_internal.ccarrow::Result<arrow::BufferVector> SerializePayloadToBuffers(const FlightPayload& msg)gRPC agnostic function.arrow::Result<arrow::flight::internal::FlightData> DeserializeFlightData(const std::shared_ptr<arrow::Buffer>& buffer)gRPC agnostic function.grpc::ByteBufferandgrpc::Slicedetails.arrow::Result<BufferVector> SerializeToBuffers() const;toFlightPayloadstruct.Are these changes tested?
Yes, both by existing tests and new tests.
Are there any user-facing changes?
No