sajjad-moradi commented on code in PR #12092:
URL: https://github.com/apache/pinot/pull/12092#discussion_r1631661977
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java:
##########
@@ -717,10 +724,19 @@ private List<PinotTaskConfig>
createPinotTaskConfigs(List<SegmentZKMetadata> sel
// Segment name conflict happens when the current method
"createPinotTaskConfigs" is invoked more than once
// within the same epoch millisecond, which may happen when there are
multiple partitions.
// To prevent such name conflict, we include a partitionSeqSuffix to
the segment name.
+ String segmentNamePrefixValue;
+ if (segmentGroups.size() == 1) {
+ segmentNamePrefixValue = MergeRollupTask.MERGED_SEGMENT_NAME_PREFIX
+ mergeLevel + DELIMITER_IN_SEGMENT_NAME
+ + segmentPrefixTimestamp + partitionSuffix +
DELIMITER_IN_SEGMENT_NAME + i
+ + DELIMITER_IN_SEGMENT_NAME +
TableNameBuilder.extractRawTableName(tableNameWithType);
+ } else {
+ segmentNamePrefixValue = MergeRollupTask.MERGED_SEGMENT_NAME_PREFIX
+ mergeLevel + DELIMITER_IN_SEGMENT_NAME
+ + segmentPrefixTimestamp + partitionSuffix +
DELIMITER_IN_SEGMENT_NAME + segmentGroupIndex
+ + DELIMITER_IN_SEGMENT_NAME + i + DELIMITER_IN_SEGMENT_NAME
+ + TableNameBuilder.extractRawTableName(tableNameWithType);
+ }
Review Comment:
There are a lot of parts and they're almost identical in both branches. It's
hard to identify which part is changing. I suggest removing duplicated parts,
something like:
```suggestion
String segmentGroupIdentifier = segmentGroups.size() == 1 ? "" :
segmentGroupIndex + DELIMITER_IN_SEGMENT_NAME;
String segmentNamePrefixValue =
MergeRollupTask.MERGED_SEGMENT_NAME_PREFIX + mergeLevel +
DELIMITER_IN_SEGMENT_NAME
+ System.currentTimeMillis() + partitionSuffix +
DELIMITER_IN_SEGMENT_NAME + segmentGroupIdentifier + i
+ DELIMITER_IN_SEGMENT_NAME +
TableNameBuilder.extractRawTableName(tableNameWithType);
```
--
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
