Vanlightly commented on a change in pull request #2888:
URL: https://github.com/apache/bookkeeper/pull/2888#discussion_r754192713
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java
##########
@@ -66,7 +66,9 @@ protected boolean isVersionCompatible() {
}
protected void sendResponse(int rc, Object response, OpStatsLogger
statsLogger) {
- channel.writeAndFlush(response, channel.voidPromise());
+ if (channel.isActive()) {
Review comment:
Currently what happens when the BookieServer is shutdown is that for
each in-progress op, when the response is sent it will fail because the channel
is closed, and that will get logged as a metric:
```
if (!future.isSuccess()) {
requestProcessor.getRequestStats().getChannelWriteStats()
.registerFailedEvent(writeElapsedNanos,
TimeUnit.NANOSECONDS);
} else {
requestProcessor.getRequestStats().getChannelWriteStats()
.registerSuccessfulEvent(writeElapsedNanos,
TimeUnit.NANOSECONDS);
}
```
So either we add an ELSE branch with the same registerFailedEvent call, or
we don't use an IF at all and allow it to use the existing logic flow.
@pradeepbn what was the reason for avoiding `channel.writeAndFlush`?
--
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]
