lhotari commented on code in PR #24832:
URL: https://github.com/apache/pulsar/pull/24832#discussion_r2415803685
##########
pulsar-common/src/main/java/org/apache/pulsar/common/protocol/PulsarDecoder.java:
##########
@@ -483,6 +483,7 @@ public void channelRead(ChannelHandlerContext ctx, Object
msg) throws Exception
}
} finally {
buffer.release();
+ cmd.clear();
Review Comment:
I'll add a comment. This change actually revealed a major bug that has been
impacting Pulsar's reliability for a long time. The command is essentially
reused and no references should be kept to it. This was violated in multiple
locations and for example lookups and partition metadata requests have been
failing because of this bug.
Yes, I'm aware that cmd.parseFrom calls clear. Calling clear twice is not
going to be a performance problem. The flags will get reset and it's just going
to be some ifs that are going to execute. I'd assume that a modern CPU can
execute 100s of millions of such code lines per second, so this is not going to
be a problem. In this case, it's essential to call clear so that possible bugs
where the cmd reference is kept in other threads will come to the surface in CI.
--
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]
