RongtongJin commented on code in PR #10443:
URL: https://github.com/apache/rocketmq/pull/10443#discussion_r3379569031
##########
remoting/src/main/java/org/apache/rocketmq/remoting/netty/RemotingCodeDistributionHandler.java:
##########
@@ -32,19 +32,37 @@ public class RemotingCodeDistributionHandler extends
ChannelDuplexHandler {
private final ConcurrentMap<Integer, LongAdder> inboundDistribution;
private final ConcurrentMap<Integer, LongAdder> outboundDistribution;
+ private volatile int lastInCode = Integer.MIN_VALUE;
+ private volatile LongAdder lastInAdder;
+ private volatile int lastOutCode = Integer.MIN_VALUE;
+ private volatile LongAdder lastOutAdder;
public RemotingCodeDistributionHandler() {
inboundDistribution = new ConcurrentHashMap<>();
outboundDistribution = new ConcurrentHashMap<>();
}
private void countInbound(int requestCode) {
+ LongAdder adder = lastInAdder;
+ if (requestCode == lastInCode && adder != null) {
+ adder.increment();
+ return;
+ }
LongAdder item = inboundDistribution.computeIfAbsent(requestCode, k ->
new LongAdder());
+ lastInAdder = item;
+ lastInCode = requestCode;
item.increment();
Review Comment:
`lastInCode` and `lastInAdder` are updated as separate volatile fields, but
this handler is `@Sharable` and can be invoked concurrently by multiple channel
event loops. A racing thread can observe a mixed pair, for example the new
`lastInAdder` for code 2 while `lastInCode` is still code 1, and then increment
the wrong `LongAdder`. Please publish the cached pair atomically, e.g. via one
immutable `LastCounter { code, adder }` stored in a single volatile field, or
remove this fast path.
--
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]
