feat: update GrpcBlobReadChannel to allow seek/limit after read#1834
feat: update GrpcBlobReadChannel to allow seek/limit after read#1834BenWhitehead merged 2 commits intomainfrom
Conversation
c92ac2c to
3310f2c
Compare
### Refactoring Extract the majority of BlobReadChannelV2 to a new abstract base class BaseStorageReadChannel which effectively adapts a LazyReadChannel to a StorageReadChannel. BaseStorageReadChannel now defines an abstract method newLazyReadChannel which each implementation is responsible for implementing to integrate into the lifecycle.
3310f2c to
ce06f6f
Compare
frankyn
left a comment
There was a problem hiding this comment.
I saw that the PR is mainly a refactor while enabling seek after read in gRPC; just have 2 follow-up questions otherwise LGTM.
| } | ||
| } catch (IOException e) { | ||
| throw e; | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
This is broad; is there a specific set of Exception types that are expected?
There was a problem hiding this comment.
For grpc, the scope is more narrow, but for JSON there isn't really a common base since things can be socket issues, ssl issues or any other number of exceptions.
This catch is primarily here to guarantee we're always doing our best to present an actionable exception to whatever is catching.
|
|
||
| protected abstract LazyReadChannel<T> newLazyReadChannel(); | ||
|
|
||
| private void maybeResetChannel(boolean umallocBuffer) throws IOException { |
There was a problem hiding this comment.
what does umallocBuffer mean in this case; is it just to effectively delete the old buffer and start with a new one?
If that's the case, can you name it freeBuffer?
There was a problem hiding this comment.
Yes, I'll rename to freeBuffer
Refactoring
Extract the majority of BlobReadChannelV2 to a new abstract base class BaseStorageReadChannel which effectively adapts a LazyReadChannel to a StorageReadChannel.
BaseStorageReadChannel now defines an abstract method newLazyReadChannel which each implementation is responsible for implementing to integrate into the lifecycle.