eolivelli commented on code in PR #4188:
URL: https://github.com/apache/bookkeeper/pull/4188#discussion_r1467370022
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java:
##########
@@ -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,
Review Comment:
why did you change this name ?V3CompletionKey > TxnCompletionKey
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java:
##########
@@ -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>() {
Review 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
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/MockBookies.java:
##########
@@ -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,
Review Comment:
nit: batchReadEntries ? (plural)
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/MockBookies.java:
##########
@@ -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,
+ int maxCount, long maxSize) throws BKException {
+ MockLedgerData ledger = getBookieData(bookieId).get(ledgerId);
+
+ if (ledger == null) {
+ LOG.warn("[{};L{}] ledger not found", bookieId, ledgerId);
+ throw new BKException.BKNoSuchLedgerExistsException();
+ }
+
+ if ((flags & BookieProtocol.FLAG_DO_FENCING) ==
BookieProtocol.FLAG_DO_FENCING) {
+ ledger.fence();
+ }
+ //Refer: BatchedReadEntryProcessor.readData
+ ByteBufList data = null;
+ if (maxCount <= 0) {
+ maxCount = Integer.MAX_VALUE;
+ }
+ long frameSize = 24 + 8 + 4;
+ for (long i = startEntryId; i < startEntryId + maxCount; i++) {
+ try {
+ ByteBuf entry = ledger.getEntry(i);
+ frameSize += entry.readableBytes() + 4;
+ if (data == null) {
+ data = ByteBufList.get(entry);
+ } else {
+ if (frameSize > maxSize) {
+ entry.release();
+ break;
+ }
+ data.add(entry);
+ }
+ } catch (Throwable e) {
Review Comment:
catching Throwable is always a code smell, why ? what do you want to catch
here ?
--
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]
