hangc0276 commented on code in PR #4293:
URL: https://github.com/apache/bookkeeper/pull/4293#discussion_r1612547410
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/AuthHandler.java:
##########
@@ -433,10 +443,19 @@ public void operationComplete(int rc, Void v) {
if (rc == BKException.Code.OK) {
synchronized (this) {
authenticated = true;
- Object msg = waitingForAuth.poll();
- while (msg != null) {
- NettyChannelUtil.writeAndFlushWithVoidPromise(ctx,
msg);
- msg = waitingForAuth.poll();
+ while (true) {
+ Object msg = waitingForAuth.poll();
+ if (msg == null) {
+ break;
+ }
+ ChannelPromise promise;
+ // check if the message has an associated promise
as the next element in the queue
+ if (waitingForAuth.peek() instanceof
ChannelPromise) {
+ promise = (ChannelPromise)
waitingForAuth.poll();
Review Comment:
If found this issue before
https://github.com/apache/bookkeeper/pull/3902/files. Your solution is better.
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java:
##########
@@ -803,19 +807,11 @@ void addEntry(final long ledgerId, byte[] masterKey,
final long entryId, Referen
headerBuilder.setPriority(DEFAULT_HIGH_PRIORITY_VALUE);
}
- ByteString body = null;
ByteBufList bufToSend = (ByteBufList) toSend;
-
- if (bufToSend.hasArray()) {
- body = UnsafeByteOperations.unsafeWrap(bufToSend.array(),
bufToSend.arrayOffset(),
- bufToSend.readableBytes());
- } else {
- for (int i = 0; i < bufToSend.size(); i++) {
- ByteString piece =
UnsafeByteOperations.unsafeWrap(bufToSend.getBuffer(i).nioBuffer());
- // use ByteString.concat to avoid byte[] allocation when
toSend has multiple ByteBufs
- body = (body == null) ? piece : body.concat(piece);
- }
- }
+ ByteString body =
ByteStringUtil.byteBufListToByteString(bufToSend);
+ bufToSend.retain();
Review Comment:
Do not need retain the bufToSend?
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java:
##########
@@ -700,14 +700,10 @@ void writeLac(final long ledgerId, final byte[]
masterKey, final long lac, ByteB
.setVersion(ProtocolVersion.VERSION_THREE)
.setOperation(OperationType.WRITE_LAC)
.setTxnId(txnId);
- ByteString body;
- if (toSend.hasArray()) {
- body = UnsafeByteOperations.unsafeWrap(toSend.array(),
toSend.arrayOffset(), toSend.readableBytes());
- } else if (toSend.size() == 1) {
- body =
UnsafeByteOperations.unsafeWrap(toSend.getBuffer(0).nioBuffer());
- } else {
- body = UnsafeByteOperations.unsafeWrap(toSend.toArray());
- }
+ ByteString body = ByteStringUtil.byteBufListToByteString(toSend);
+ toSend.retain();
Review Comment:
The toSend already convert to ByteString, why do we need to retain and
release after flushed no matter the flush succeed or not?
--
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]
