Skip to content

feat: introduce a full server timeline view on Broker#19212

Open
Fly-Style wants to merge 3 commits intoapache:masterfrom
Fly-Style:server-timeline
Open

feat: introduce a full server timeline view on Broker#19212
Fly-Style wants to merge 3 commits intoapache:masterfrom
Fly-Style:server-timeline

Conversation

@Fly-Style
Copy link
Copy Markdown
Contributor

@Fly-Style Fly-Style commented Mar 26, 2026

This patch:

  • introduces a new class BrokerServerViewOfLatestUsedSegments on the Broker. This class implements TimelineServerView and maintains a full timeline of all segments (available on historicals, available on indexers/peons, not available). Here we keep a single timeline for each datasource. When a callback is received for a segment (with or without server) added or removed, we simply add it to the respective datasource timeline.
  • when MetadataSegmentView finishes a poll from the Coordinator, it determines the delta of segments added/removed so that it can invoke the new callback accordingly.
  • timeline.lookup() will always return the latest complete partition set in the timeline object, irrespective of whether those segments are available or not.
  • if metadataSegmentCacheEnable is false, the new server view class will throw an exception when any method is invoked since we do not have info of unavailable segments.

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • 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.

@Fly-Style Fly-Style self-assigned this Mar 26, 2026
@Fly-Style Fly-Style changed the title Introduce a full server timeline view on Broker feat: introduce a full server timeline view on Broker Mar 26, 2026

synchronized (lock) {
for (SegmentId segmentId : segmentIds) {
final DataSegment metadataSegment = metadataSegments.remove(segmentId);

Check notice

Code scanning / CodeQL

Unread local variable Note

Variable 'DataSegment metadataSegment' is never read.
@jtuglu1
Copy link
Copy Markdown
Contributor

jtuglu1 commented Mar 26, 2026

Is this related to: #18716?

@Fly-Style
Copy link
Copy Markdown
Contributor Author

@jtuglu1 hi! Haven't seen this issue, will take a look tomorrow! Thanks!

@jtuglu1
Copy link
Copy Markdown
Contributor

jtuglu1 commented Mar 27, 2026

If this is trying to solve the problem in the issue I linked above, can we add the embedded test added in: #18737

@cecemei
Copy link
Copy Markdown
Contributor

cecemei commented Mar 30, 2026

I find the logic for merging mergedSelectors quite complex and difficult to reason about. Introducing two additional state-tracking variables, metadataSegments and metadataRemovedSegmentIds, makes it even harder to follow.

My two cents: avoid merging entirely and use MetadataSegmentView as the source of truth for determining which segments should be visible. With this approach, we would not need an additional MetadataSegmentViewCallback. Instead, in the poll() function, we can directly remove segments from mergedSelectors and mergedTimelines when they are marked as removed. These removals typically come from MarkSegmentsAsUnusedAction, and since we rarely reverse them, simply removing those segments should keep things mostly consistent.

For newly added segments, we can reuse the ServerSelector from BrokerServerView, or create an empty one if necessary. We can still keep the BsvCallback to listen for segment location changes and update them only if the segment is visible according to metadata.

BrokerServerView would remain the source of truth for which segments are available on which servers, while the updated MetadataSegmentView would be the source of truth for determining which segments should be queried and their locations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants