raulcd commented on PR #49339:
URL: https://github.com/apache/arrow/pull/49339#issuecomment-4054210770
@lidavidm @pitrou I've moved the serialization / Deserialization logic from
`arrow/flight/transport/grpc/serialization_internal.cc` to
`arrow/flight/serialization_internal.cc`.
Those are now serializing a `FlightPayload` to `arrow::BufferVector` or
deserializing from an `arrow::Buffer` to `FlightData`. Those are still supposed
to be internal:
```c++
/// \brief Serialize a FlightPayload to a vector of buffers.
///
/// The first buffer contains the protobuf wire format header. Subsequent
/// buffers are zero-copy references to the IPC body buffers, with padding
/// buffers inserted as needed for 8-byte alignment.
arrow::Result<arrow::BufferVector> SerializePayloadToBuffers(
const FlightPayload& payload);
/// \brief Deserialize FlightData from a contiguous buffer.
arrow::Result<FlightData> DeserializeFlightData(
const std::shared_ptr<arrow::Buffer>& buffer);
```
The external API to be used is either `FlightPayload::SerializeToBuffers()`
or a new `FlightMessageDecoder::Consume(std::shared_ptr<Buffer> buffer)`.
The gRPC transport side serialization/deserialization is still handled on
`arrow/flight/transport/grpc/serialization_internal.cc` but those are just tiny
wrappers for `arrow/flight/serialization_internal.cc` only managing the
`grpc::Slices / ByteBuffer` on top of it.
I've adapted the gRPC Async PoC to show how this would work with the bidi
reactor.
The only point where I am not entirely clear is whether utilities specific
to gRPC should be exposed. This matters for users building their own bidi
reactors (see the async grpc example on the PR). If our stand-point is that
users
should manage gRPC themselves then those users need a way to convert between
`arrow::Buffer` and `grpc::ByteBuffer/Slice` which isn't trivial.
```c++
/// Convert an Arrow Buffer to a gRPC Slice.
arrow::Result<::grpc::Slice> SliceFromBuffer(const
std::shared_ptr<arrow::Buffer>& buf);
/// Wrap a gRPC ByteBuffer as a zero-copy Arrow Buffer (and clear the
ByteBuffer).
arrow::Status WrapGrpcBuffer(::grpc::ByteBuffer* grpc_buf,
std::shared_ptr<arrow::Buffer>* out);
```
I'd like to move this forward, maybe an initial PR only moving the
serialization / deserialization logic to handle `arrow::Buffer/BufferVector`
instead of `grpc::ByteBuffers/Slice` is the best approach?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
