snleee commented on code in PR #9815:
URL: https://github.com/apache/pinot/pull/9815#discussion_r1024550450
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java:
##########
@@ -539,6 +542,13 @@ private List<PinotTaskConfig>
createPinotTaskConfigs(List<SegmentZKMetadata> sel
}
List<PinotTaskConfig> pinotTaskConfigs = new ArrayList<>();
+ StringBuilder partitionSuffixBuilder = new StringBuilder();
+ if (partition != null && !partition.isEmpty()) {
+ for (int columnPartition : partition) {
+
partitionSuffixBuilder.append(DELIMITER_IN_SEGMENT_NAME).append(columnPartition);
Review Comment:
This can potentially result in a very long segment name if the upstream user
doesn't actually partition properly. We have added `System.currentTimeMillis()`
and `i - task sequence number` to avoid the segment name issue in general.
```
e.g. Kafka with 100 partition
segment_name_0_1_2_3_4...._100_...
```
@jtao15 Can you double check on this?
--
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]
