snleee commented on code in PR #10964:
URL: https://github.com/apache/pinot/pull/10964#discussion_r1239387645
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/lineage/DefaultLineageManager.java:
##########
@@ -105,6 +107,11 @@ public void updateLineageForRetention(TableConfig
tableConfig, SegmentLineage li
}
}
+ @Override
+ public List<List<SegmentZKMetadata>>
getSegmentGroupsForMergeRollupTask(TableConfig tableConfig,
Review Comment:
I feel that this function doesn't belong to `LineageManager`? LineageManager
should not know about `MergeRollupTask`. MergeRollupTask should be the user of
the segment replace protocol.
Can you add a bit more information on what it means by `SegmentGroup` here?
If your intention is to inject the custom code to the MergeRollupTask,
please create a dedicated interface for that.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/lineage/LineageManager.java:
##########
@@ -70,4 +71,14 @@ void updateLineageForRevertReplaceSegments(TableConfig
tableConfig, String linea
*/
void updateLineageForRetention(TableConfig tableConfig, SegmentLineage
lineage, List<String> allSegments,
List<String> segmentsToDelete, Set<String> consumingSegments);
+
+ /**
+ * Get segment zk metadata groups for MergeRollupTask
+ * @param tableConfig
+ * @param lineage
+ * @param segments
+ * @return segment zk metadata groups
Review Comment:
Can you elaborate on the `segment zk metadata groups`?
--
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]
