cecemei commented on PR #19212: URL: https://github.com/apache/druid/pull/19212#issuecomment-4158445827
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. -- 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]
