Fix broker request id generator to avoid generating same id

[Jackie-Jiang]

Summary

This PR fixes the broker request ID generator to prevent the generation of duplicate request IDs across different brokers, broker restarts, and different query engines.

Problem

The previous implementation used the broker ID hash code as a mask for generating request IDs. This approach had several issues:

• Brokers with similar IDs could generate the same hash code, leading to duplicate request IDs
• Broker restarts would regenerate the same sequence of request IDs
• Hash collisions could occur between different broker instances
• Different query engines (single-stage vs multi-stage) could potentially generate the same request IDs

Solution

• Replaced deterministic hash-based approach with random-based approach
• Centralized request ID generator creation in BaseBrokerStarter
• Updated all broker request handlers to use shared generator instance
• Ensures different query engines use the same ID generation mechanism
• Improved documentation to reflect the new random-based approach

Changes

Core Changes

• BrokerRequestIdGenerator: Changed from hash-based to random-based ID generation
• BaseBrokerStarter: Creates a single shared BrokerRequestIdGenerator instance
• All Request Handlers: Updated constructors to accept the shared generator

Files Modified

• BaseBrokerStarter.java - Creates and injects shared request ID generator
• BaseBrokerRequestHandler.java - Updated constructor to accept generator
• BaseSingleStageBrokerRequestHandler.java - Updated constructor signature
• BrokerRequestIdGenerator.java - Changed from hash-based to random-based generation
• GrpcBrokerRequestHandler.java - Updated constructor
• MultiStageBrokerRequestHandler.java - Updated constructor
• SingleConnectionBrokerRequestHandler.java - Updated constructor
• TimeSeriesRequestHandler.java - Updated constructor
• Test files updated to use new constructor signatures

Technical Details

ID Generation Strategy

• Base: Random integer multiplied by 1,000,000,000
• Counter: Auto-incrementing counter for uniqueness
• Result: Unique 19-digit request ID with high probability of uniqueness

Benefits

1. Eliminates hash collisions between brokers
2. Handles broker restarts gracefully
3. Maintains performance with atomic counter
4. Preserves uniqueness across distributed system
5. Ensures consistent ID generation across different query engines (single-stage, multi-stage, time-series)

Query Engine Consistency

By centralizing the request ID generator in BaseBrokerStarter and injecting it into all request handlers:

• SingleConnectionBrokerRequestHandler (single-stage engine)
• GrpcBrokerRequestHandler (single-stage engine with gRPC)
• MultiStageBrokerRequestHandler (multi-stage engine)
• TimeSeriesRequestHandler (time-series engine)

All query engines now use the same ID generation mechanism, preventing conflicts between different execution paths.

Testing

• Code compiles successfully
• All existing tests updated and passing
• Manual testing with multiple broker instances
• Verified no duplicate IDs generated across restarts
• Tested with different query engines to ensure ID uniqueness

This change improves the reliability of request tracking across all query execution engines in Pinot.

[Copilot]

Pull Request Overview

This PR fixes the broker request ID generator to prevent duplicate request IDs across different brokers, broker restarts, and query engines by replacing the deterministic hash-based approach with a random-based approach.

Key changes:

• Centralized ID generation: Creates a single BrokerRequestIdGenerator instance in BaseBrokerStarter and injects it into all request handlers
• Random-based generation: Replaces hash-code masking with random number generation to eliminate collisions
• Unified approach: Ensures all query engines (single-stage, multi-stage, time-series) use the same ID generation mechanism

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
BaseBrokerStarter.java	Creates shared BrokerRequestIdGenerator instance and injects it into all request handlers
BrokerRequestIdGenerator.java	Changes from hash-based to random-based ID generation using ThreadLocalRandom
BaseBrokerRequestHandler.java	Updates constructor to accept injected requestIdGenerator instead of creating one
BaseSingleStageBrokerRequestHandler.java	Updates constructor signature to pass through requestIdGenerator parameter
SingleConnectionBrokerRequestHandler.java	Updates constructor to accept and pass through requestIdGenerator parameter
GrpcBrokerRequestHandler.java	Updates constructor to accept and pass through requestIdGenerator parameter
MultiStageBrokerRequestHandler.java	Updates constructor to accept and pass through requestIdGenerator parameter
TimeSeriesRequestHandler.java	Updates constructor to accept and pass through requestIdGenerator parameter
LiteralOnlyBrokerRequestTest.java	Updates test instantiations to provide BrokerRequestIdGenerator instance
BaseSingleStageBrokerRequestHandlerTest.java	Updates test instantiation to provide BrokerRequestIdGenerator instance

[codecov-commenter]

Codecov Report

❌ Patch coverage is 61.53846% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.31%. Comparing base (76f2b7f) to head (6e9f986).

Files with missing lines	Patch %	Lines
...e/pinot/broker/broker/helix/BaseBrokerStarter.java	33.33%	2 Missing ⚠️
...roker/requesthandler/GrpcBrokerRequestHandler.java	0.00%	1 Missing ⚠️
...requesthandler/MultiStageBrokerRequestHandler.java	50.00%	0 Missing and 1 partial ⚠️
...roker/requesthandler/TimeSeriesRequestHandler.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #16661      +/-   ##
============================================
- Coverage     63.32%   63.31%   -0.01%     
+ Complexity     1380     1379       -1     
============================================
  Files          3035     3035              
  Lines        176714   176715       +1     
  Branches      27121    27121              
============================================
- Hits         111896   111884      -12     
- Misses        56232    56245      +13     
  Partials       8586     8586              

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-11	63.27% <61.53%> (+<0.01%)	⬆️
java-21	63.28% <61.53%> (-0.02%)	⬇️
temurin	63.31% <61.53%> (-0.01%)	⬇️
unittests	63.30% <61.53%> (-0.01%)	⬇️
unittests1	56.46% <ø> (ø)
unittests2	33.11% <61.53%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.