Skip to content

acquire segment references in bulk for ServerManager to simplify reference handling#18054

Merged
clintropolis merged 5 commits intoapache:masterfrom
clintropolis:acquire-segment-references-up-front
May 30, 2025
Merged

acquire segment references in bulk for ServerManager to simplify reference handling#18054
clintropolis merged 5 commits intoapache:masterfrom
clintropolis:acquire-segment-references-up-front

Conversation

@clintropolis
Copy link
Member

Description

Follow-up to #18005, this PR changes ServerManager to acquire segment references in bulk before building query runners instead of lazily during the processing of the segment. I think that this should also help reduce the chance for segments to be dropped before being processed since grabbing the reference for each segment is not dependent on when that segments runner runs, in exchange for holding the reference a bit longer. ReferenceCountingSegmentQueryRunner is no longer needed and has been deleted.

Also did some minor code cleanup in SegmentManager to tidy things up (no logic changes).


This PR has:

  • been self-reviewed.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@clintropolis clintropolis merged commit da45eda into apache:master May 30, 2025
74 checks passed
@clintropolis clintropolis deleted the acquire-segment-references-up-front branch May 30, 2025 23:52
clintropolis added a commit to clintropolis/druid that referenced this pull request Jun 1, 2025
clintropolis added a commit to clintropolis/druid that referenced this pull request Jun 1, 2025
@capistrant capistrant added this to the 34.0.0 milestone Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants