hachikuji commented on pull request #9819:
URL: https://github.com/apache/kafka/pull/9819#issuecomment-761119104
@dengziming @jsancio I'll give my take what I think we should do here, but
let's try to agree before we go any further. Basically I am not really
convinced that we need a new type, at least not in the near term. The snapshot
data will be record data, so I cannot see a strong argument not to use the
existing "records" type which already handles zero-copy.
The one difference between `FetchSnapshot` and `Fetch` is that the latter is
aligned on offsets while the former can indicate an arbitrary position in a
file. I assume that is the main reason we have diverged here. However, I do not
think this presents a major hurdle. On the server side, I think we have
everything we need. `FileRawSnapshotReader` already has a `FileRecords` object
and we can return a slice at an arbitrary position using `slice(int position,
int size)`. On the client side, when we parse a snapshot response, `Readable`
will give us back a reference to a `MemoryRecords` which will not necessarily
be aligned. However, `MemoryRecords` gives us direct access to the underlying
buffer, so the misalignment does not cause any problems.
Longer term, I think we can make an effort to replace the "records" type and
just rely on `type=bytes, zeroCopy=true`. For this to work, we need a type
which generalizes both a `ByteBuffer` and a `FileChannel` slice. For example,
we could have something like this:
```java
interface ZeroCopyBuffer {
ByteBuffer read(int length);
Send toSend();
}
```
When parsing a request or response received over the network, it will be in
memory anyway, so we need some hook to get access to the ByteBuffer. On the
other hand, when transmitting a request or response, we let the implementation
build the Send itself. This gives us a way to work with `RecordsSend`. There's
probably more to it than that, but that's probably enough to get started. Then
we would change the generated code to rely on the new type and let
`SendBuilder` invoke `ZeroCopyBuffer.toSend` as appropriate.
Thoughts?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
