[opt](point query) use fastUniqueId instead of UUID.randomUUID to reduce contention#63028
Open
HonestManXin wants to merge 1 commit intoapache:masterfrom
Open
[opt](point query) use fastUniqueId instead of UUID.randomUUID to reduce contention#63028HonestManXin wants to merge 1 commit intoapache:masterfrom
HonestManXin wants to merge 1 commit intoapache:masterfrom
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
Review result: no blocking issues found in this PR. The change replaces FE query-id generation in StmtExecutor with a ThreadLocalRandom-based UUID v4-compatible TUniqueId generator, which matches the stated goal of avoiding SecureRandom contention for non-security request correlation IDs.
Critical checkpoint conclusions:
- Goal and tests: The implementation addresses the stated contention source in StmtExecutor query-id generation. Existing CI CheckStyle passed, and I also ran
mvn -pl fe-core -DskipTests -Dskip.doc=true checkstyle:checksuccessfully. No dedicated UUID utility unit test is added; residual risk is low because the bit layout is straightforward and mirrors UUID v4 masking. - Scope: The modification is small and focused: one utility class plus replacements at query/retry/internal-query id generation sites.
- Concurrency: This path is concurrency-related by design. Using ThreadLocalRandom removes the shared SecureRandom monitor contention from these call sites; no shared mutable state or new lock ordering is introduced.
- Lifecycle/static initialization: The SecureRandom holder is lazy and only used by the secure fallback helper, not by the new hot path. No cross-TU/static initialization concern applies in Java here.
- Configuration: No configuration items are added.
- Compatibility/storage format: Generated IDs remain 128-bit TUniqueId values with UUID v4 version/variant bits; no storage, RPC, or mixed-version protocol format change is introduced.
- Parallel paths: The PR updates the StmtExecutor query-id generation sites related to the reported query execution path. Other UUID.randomUUID uses appear to be labels/session IDs/tests and are outside this specific non-cryptographic query-id scope.
- Conditional checks: No new special conditional logic is added.
- Test coverage/results: No result files are changed. CheckStyle passed locally; broader FE UT/compile/performance checks were still pending in GitHub/TeamCity when reviewed.
- Observability: Existing query-id logging/audit behavior is preserved through DebugUtil.printId(TUniqueId). No additional metrics/logs appear necessary.
- Transaction/persistence/data writes: The reviewed changes do not modify transaction, persistence, or data write semantics.
- FE-BE variable passing: No new transmitted field is added; existing TUniqueId propagation is preserved.
- Performance: The hot-path contention issue is improved without obvious added allocation beyond the existing TUniqueId object.
User focus: no additional user-provided review focus was present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In high‑concurrency point‑query scenarios, a large number of requests require generating a UUID for internal identification. When the request rate is high, frequent calls to
UUID.randomUUID()can introduce contention and become a minor performance bottleneck.For query, the UUID is only required to provide a unique identifier for request correlation, and cryptographic strength is not necessary. To reduce contention and improve throughput, this change introduces
fastUniqueId, which generates RFC‑4122 compatible UUID v4 values usingThreadLocalRandom.By using a per‑thread random generator instead of the shared random source used by
UUID.randomUUID(), UUID generation scales better under high concurrency while still preserving the standard UUID format.