[BP-62] Bookkeeper client introduce batch read request api. #4188
Conversation
|
rerun failure checks |
| @@ -562,6 +562,10 @@ class BatchedReadResponse extends Response implements ReferenceCounted { | |||
| final long requestId; | |||
| final ByteBufList data; | |||
|
|
|||
| BatchedReadResponse(byte protocolVersion, int errorCode, long ledgerId, long entryId, long requestId) { | |||
| this(protocolVersion, errorCode, ledgerId, entryId, requestId, ByteBufList.get()); | |||
There was a problem hiding this comment.
Reminder: No matter what the error code is, don't forget to release the ByteBufList in the client side.
There was a problem hiding this comment.
Sure, the error BatchedReadResponse will also be released.
| @@ -123,6 +123,47 @@ public ByteBuf readEntry(BookieId bookieId, int flags, long ledgerId, long entry | |||
| return entry; | |||
| } | |||
|
|
|||
| public ByteBufList batchReadEntry(BookieId bookieId, int flags, long ledgerId, long startEntryId, | |||
There was a problem hiding this comment.
nit: batchReadEntries ? (plural)
| } | ||
| data.add(entry); | ||
| } | ||
| } catch (Throwable e) { |
There was a problem hiding this comment.
catching Throwable is always a code smell, why ? what do you want to catch here ?
There was a problem hiding this comment.
Make sense, I copy the code from the BatchedReadEntryProcessor, here we don't need catch the exception.
| @@ -687,7 +688,7 @@ void connectIfNeededAndDoOp(GenericCallback<PerChannelBookieClient> op) { | |||
| void writeLac(final long ledgerId, final byte[] masterKey, final long lac, ByteBufList toSend, WriteLacCallback cb, | |||
| Object ctx) { | |||
| final long txnId = getTxnId(); | |||
| final CompletionKey completionKey = new V3CompletionKey(txnId, | |||
| final CompletionKey completionKey = new TxnCompletionKey(txnId, | |||
There was a problem hiding this comment.
why did you change this name ?V3CompletionKey > TxnCompletionKey
There was a problem hiding this comment.
The V3CompletionKey may mislead. In this PR, it's used for the v2 batch read. The V3CompletionKey uses txnId to trace, so I change the name to TxnCompletionKe.
| @@ -2340,28 +2467,28 @@ private long getTxnId() { | |||
| return txnIdGenerator.incrementAndGet(); | |||
| } | |||
|
|
|||
| private final Recycler<V2CompletionKey> v2KeyRecycler = new Recycler<V2CompletionKey>() { | |||
| private final Recycler<EntryCompletionKey> v2KeyRecycler = new Recycler<EntryCompletionKey>() { | |||
There was a problem hiding this comment.
why are you changing this name ? V2CompletionKey > EntryCompletionKey
it will make porting patches to other branches harder.
if it is not strictly needed I won't change the class names
There was a problem hiding this comment.
We are renaming V3CompletionKey to TxnCompletionKey. Therefore, I want to downplay the concepts of V2 and V3 and name it based on its purpose.
Descriptions of the changes in this PR:
This is the third PR for the batch read(#4051) feature.
Bookkeeper client introduces batch read request API.