Copilot commented on code in PR #16661:
URL: https://github.com/apache/pinot/pull/16661#discussion_r2292237820
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BrokerRequestIdGenerator.java:
##########
@@ -18,32 +18,26 @@
*/
package org.apache.pinot.broker.requesthandler;
+import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.atomic.AtomicLong;
-/**
- * An ID generator to produce a global unique identifier for each query, used
in v1/v2 engine for tracking and
- * inter-stage communication(v2 only). It's guaranteed by:
- * <ol>
- * <li>
- * Using a mask computed using the hash-code of the broker-id to ensure
two brokers don't arrive at the same
- * requestId. This mask becomes the most significant 9 digits (in base-10).
- * </li>
- * <li>
- * Using a auto-incrementing counter for the least significant 9 digits
(in base-10).
- * </li>
- * </ol>
- */
+
+/// An ID generator to produce a global unique identifier for each query, used
in single-stage/multi-stage engine for
+/// tracking and inter-stage communication (multi-stage engine only). It's
guaranteed by:
+/// - Using a random number to ensure two brokers (or restarted broker) don't
arrive at the same request ID. This random
+/// number becomes the most significant 10 digits of the request ID (in
base-10).
+/// - Using an auto-incrementing counter for the least significant 9 digits
(in base-10).
public class BrokerRequestIdGenerator {
private static final long OFFSET = 1_000_000_000L;
- private final long _mask;
+ private final long _base;
private final AtomicLong _incrementingId = new AtomicLong(0);
- public BrokerRequestIdGenerator(String brokerId) {
- _mask = ((long) (brokerId.hashCode() & Integer.MAX_VALUE)) * OFFSET;
+ public BrokerRequestIdGenerator() {
+ _base = ThreadLocalRandom.current().nextInt(Integer.MAX_VALUE) * OFFSET;
Review Comment:
The comment states the random number becomes the 'most significant 10
digits' but ThreadLocalRandom.current().nextInt(Integer.MAX_VALUE) can only
generate up to ~2.1 billion, which is 10 digits maximum, not guaranteed 10
digits. Consider updating the documentation to reflect the actual range or
using a different approach if exactly 10 digits is required.
```suggestion
// Generate a random 10-digit number for the most significant digits of
the request ID (in base-10).
_base = ThreadLocalRandom.current().nextLong(1_000_000_000L,
10_000_000_000L) * OFFSET;
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
