ege-st commented on code in PR #13716:
URL: https://github.com/apache/pinot/pull/13716#discussion_r1697385000
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerDebug.java:
##########
@@ -272,6 +272,9 @@ public String getServerRoutingStats() {
}
private long getRequestId() {
+ if (_requestIdGenerator.get() == Long.MAX_VALUE) {
+ _requestIdGenerator.set(0);
+ }
Review Comment:
Actually, that still has an error, since, if we had three threads they could
execute the operations in this order: `T1: CAS; T2: CAS; T3: CAS; T3: Inc; T2:
Inc; T1: Inc` which would lead to an overflow.
I think it needs to be something the following. Which insures that after we
check if the Atomic is reset to 0 we check to see if another thread changed the
Atomic before the getAndIncrement. If it was changed then we have to go back
and check if we're at MAX_VALUE again before making the increment.:
```
long requestId;
boolean ready = false;
while (!ready) {
requestId = _requestIdGenerator.get();
if (requestId == Long.MAX_VALUE) {
ready = _requestIdGenerator.compareAndSet(Long.MAX_VALUE, 0);
requestId = 0L;
} else {
ready = _requestIdGenerator.compareAndSet(requestId,
requestId + 1);
requestId = requestId + 1;
}
}
return requestId;
```
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
