jtao15 commented on code in PR #10979:
URL: https://github.com/apache/pinot/pull/10979#discussion_r1276536454
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java:
##########
@@ -663,18 +672,26 @@ private List<PinotTaskConfig>
createPinotTaskConfigs(List<SegmentZKMetadata> sel
int numRecordsPerTask = 0;
List<List<String>> segmentNamesList = new ArrayList<>();
List<List<String>> downloadURLsList = new ArrayList<>();
+ List<SegmentZKMetadata> segmentZKMetadataList = new ArrayList<>();
List<String> segmentNames = new ArrayList<>();
List<String> downloadURLs = new ArrayList<>();
for (int i = 0; i < segments.size(); i++) {
SegmentZKMetadata targetSegment = segments.get(i);
+ segmentZKMetadataList.add(targetSegment);
segmentNames.add(targetSegment.getSegmentName());
downloadURLs.add(targetSegment.getDownloadUrl());
numRecordsPerTask += targetSegment.getTotalDocs();
if (numRecordsPerTask >= maxNumRecordsPerTask || i == segments.size()
- 1) {
- segmentNamesList.add(segmentNames);
- downloadURLsList.add(downloadURLs);
+ // If "skipAllMerged" mode is enabled, skip generating task with all
merged segments
+ boolean skipAllMerged =
MergeTask.SKIP_ALL_MERGED_MODE.equalsIgnoreCase(taskConfigs.get(MergeTask.MODE));
Review Comment:
Considering that `processAll` and `skipAllMerged` are not mutually
exclusive, a table can have multiple modes. One approach is to treat the task
modes as a comma-separated list and then verify if `skipAllMerged` is enabled
within that list.(nit) put this line outside of the for loop
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java:
##########
@@ -663,18 +672,26 @@ private List<PinotTaskConfig>
createPinotTaskConfigs(List<SegmentZKMetadata> sel
int numRecordsPerTask = 0;
List<List<String>> segmentNamesList = new ArrayList<>();
List<List<String>> downloadURLsList = new ArrayList<>();
+ List<SegmentZKMetadata> segmentZKMetadataList = new ArrayList<>();
List<String> segmentNames = new ArrayList<>();
List<String> downloadURLs = new ArrayList<>();
for (int i = 0; i < segments.size(); i++) {
SegmentZKMetadata targetSegment = segments.get(i);
+ segmentZKMetadataList.add(targetSegment);
segmentNames.add(targetSegment.getSegmentName());
downloadURLs.add(targetSegment.getDownloadUrl());
numRecordsPerTask += targetSegment.getTotalDocs();
if (numRecordsPerTask >= maxNumRecordsPerTask || i == segments.size()
- 1) {
- segmentNamesList.add(segmentNames);
- downloadURLsList.add(downloadURLs);
+ // If "skipAllMerged" mode is enabled, skip generating task with all
merged segments
+ boolean skipAllMerged =
MergeTask.SKIP_ALL_MERGED_MODE.equalsIgnoreCase(taskConfigs.get(MergeTask.MODE));
Review Comment:
Considering that `processAll` and `skipAllMerged` are not mutually
exclusive, a table can have multiple modes. One approach is to treat the task
modes as a comma-separated list and then verify if `skipAllMerged` is enabled
within that list.
(nit) put this line outside of the for loop
--
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]
