yashmayya commented on code in PR #16096:
URL: https://github.com/apache/pinot/pull/16096#discussion_r2153723994
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -2185,7 +2185,7 @@ private void processBatchesSequentially(List<Set<String>>
segmentBatchList, Stri
}
}
- private void waitUntilPrevBatchIsComplete(String tableNameWithType,
Set<String> segmentBatchToCommit,
+ public void waitUntilPrevBatchIsComplete(String tableNameWithType,
Set<String> segmentBatchToCommit,
Review Comment:
> this feels like a very internal implementation detail on how forceCommit
works in batches internally to me and IMO
I think the name is a bit of a misnomer - the method takes in a list of
segments and only checks whether those have been committed or not. IMO it's
fine to use this method directly in `TableRebalancer` (alongside a rename
perhaps) but if it's easy enough to refactor it out into a separate util, that
works too. In either case, we definitely shouldn't duplicate this code in
`TableRebalancer` like this commit does -
https://github.com/apache/pinot/pull/16096/commits/b912b6a47af5a72f9c11fc9ef62ae0d67b8fad52.
--
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]
