Fix promise lost in ClientSide AuthHandler

[hangc0276]

Motivation

#3783 changed the AddEntry Request type from BookieProtocol.AddRequest to ByteBuf or ByteBufList, leading to the AddEntry request being added into waitingForAuth queue in the client-side AuthHandler

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java

Lines 787 to 791 in 0171a40

	if (toSend instanceof ByteBuf) {
	request = ((ByteBuf) toSend).retainedDuplicate();
	} else {
	request = ByteBufList.clone((ByteBufList) toSend);
	}

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/AuthHandler.java

Lines 363 to 364 in 0171a40

	} else if (msg instanceof ByteBuf || msg instanceof ByteBufList) {
	waitingForAuth.add(msg);

For the requests in the waitingForAuth, they will be polled out by AuthHandshakeCompleteCallback#operationComplete.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/AuthHandler.java

Lines 426 to 436 in 0171a40

	public void operationComplete(int rc, Void v) {
	if (rc == BKException.Code.OK) {
	synchronized (this) {
	authenticated = true;
	Object msg = waitingForAuth.poll();
	while (msg != null) {
	ctx.writeAndFlush(msg);
	msg = waitingForAuth.poll();
	}
	}
	} else {

However, the waitingForAuth queue only stores the AddEntry request, and it ignores the related ChannelPromise. When polling requests from the waitingForAuth queue in AuthHandshakeCompleteCallback#operationComplete, those requests will be written and flushed into the Netty channel without passing any ChannelPromises, and Netty will use voidPromise by default.

In a word, the AddEntry request side passed ChannelPromise will be replaced with voidPromise, and the original promise will never be complete and the following operations won't execute.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java

Lines 1159 to 1169 in 0171a40

	ChannelPromise promise = channel.newPromise().addListener(future -> {
	if (future.isSuccess()) {
	nettyOpLogger.registerSuccessfulEvent(MathUtils.elapsedNanos(startTime), TimeUnit.NANOSECONDS);
	CompletionValue completion = completionObjects.get(key);
	if (completion != null) {
	completion.setOutstanding();
	}
	} else {
	nettyOpLogger.registerFailedEvent(MathUtils.elapsedNanos(startTime), TimeUnit.NANOSECONDS);
	}
	});

There are two impactions:

• The nettyOpLogger metric won't update
• The addEntryOutstanding counter metric won't update

Changes

Store the ChannelPromise along with the request in the client-side AuthHandle and pass it to the Netty channel when polling out.

[zymap]

Do we need to have a test?

[horizonzy]

Nice catch

[dlg99]

Please add a test

[wenbingshen]

Nice Catch!

It doesn't seem very easy to add an AddEntry request before the auth handshake is complete, especially getting it into the waitingForAuth queue, and the auth request will also trigger the promise execution.

[hangc0276]

We need to add a unit test for this change, move to the next release

[shoothzj]

As dicussed in #4293 I think we can close this PR now.