Jackie-Jiang commented on PR #15780: URL: https://github.com/apache/pinot/pull/15780#issuecomment-2895956554
> Isn't that an overkill? > > Requirements of `LeafStageWorkerAssignmentRule` and `WorkerManager` are different, but from an abstraction point of view we can say that `SegmentPartitionMetadataManager` manages `TablePartitionInfo` which: > > * Provides an easy to use API to work with segment partitions. > * Is a complete view, in that all processed segments will be part of the `TablePartitionInfo` POJO. > > Maybe I am missing something, is there a deeper reason to create another POJO here? > > Maintaining two separate POJOs with mostly the same information will be an anti-pattern in my view. The main difference between them is as following: - `WorkerManager` is trying to find segments of the same partition served by the same server - `LeafStageWorkerAssignmentRule` only needs to find segments of the same partition The reason why `WorkerManager` is treating new added segments differently is because they might not be available on the same server and we don't want to fail the query, thus it excludes them. What you are doing within this PR is to add the excluded ones back. Because the info needed for them are actually different, even though they share common properties (e.g. which partition a segment belongs to), it will be easier to maintain if keep the logic separate. With current approach, there are overhead for both use cases: - When called from `WorkerManager`, storing the excluded segments is overhead - When called from `SegmentPartitionMetadataManager`, maintaining the fully replicated servers is overhead -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
