snleee commented on code in PR #10284:
URL: https://github.com/apache/pinot/pull/10284#discussion_r1106668355
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3247,42 +3247,44 @@ public String startReplaceSegments(String
tableNameWithType, List<String> segmen
// Check that all the segments from 'segmentsFrom' exist in the table
Set<String> segmentsForTable = new
HashSet<>(getSegmentsFor(tableNameWithType, true));
- Set<String> unavailableSegmentsInFrom = Sets.difference(new
HashSet<>(segmentsFrom), segmentsForTable);
- Preconditions.checkArgument(unavailableSegmentsInFrom.isEmpty(),
String.format(
- "'%s' from 'segmentsFrom' are unavailable in the table. (tableName =
'%s', segmentsFrom = '%s', "
- + "segmentsTo = '%s')", unavailableSegmentsInFrom,
tableNameWithType, segmentsFrom, segmentsTo));
-
- // Check that all the segments from 'segmentTo' does not exist in the
table.
- Set<String> availableSegmentsInTo = Sets.intersection(new
HashSet<>(segmentsTo), segmentsForTable);
- Preconditions.checkArgument(availableSegmentsInTo.isEmpty(), String.format(
- "'%s' from 'segmentsTo' should not be available in the table at this
point. (tableName = '%s', "
- + "segmentsFrom = '%s', segmentsTo = '%s')",
availableSegmentsInTo, tableNameWithType, segmentsFrom,
- segmentsTo));
+ for (String segment : segmentsFrom) {
+ Preconditions.checkState(segmentsForTable.contains(segment),
Review Comment:
The new code will never log the full `segmentsFrom/segmentsTo` in case of
error. I think that those may be helpful when building the context during the
debugging process. Please think about it.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3247,42 +3247,44 @@ public String startReplaceSegments(String
tableNameWithType, List<String> segmen
// Check that all the segments from 'segmentsFrom' exist in the table
Set<String> segmentsForTable = new
HashSet<>(getSegmentsFor(tableNameWithType, true));
- Set<String> unavailableSegmentsInFrom = Sets.difference(new
HashSet<>(segmentsFrom), segmentsForTable);
- Preconditions.checkArgument(unavailableSegmentsInFrom.isEmpty(),
String.format(
- "'%s' from 'segmentsFrom' are unavailable in the table. (tableName =
'%s', segmentsFrom = '%s', "
- + "segmentsTo = '%s')", unavailableSegmentsInFrom,
tableNameWithType, segmentsFrom, segmentsTo));
-
- // Check that all the segments from 'segmentTo' does not exist in the
table.
- Set<String> availableSegmentsInTo = Sets.intersection(new
HashSet<>(segmentsTo), segmentsForTable);
- Preconditions.checkArgument(availableSegmentsInTo.isEmpty(), String.format(
- "'%s' from 'segmentsTo' should not be available in the table at this
point. (tableName = '%s', "
- + "segmentsFrom = '%s', segmentsTo = '%s')",
availableSegmentsInTo, tableNameWithType, segmentsFrom,
- segmentsTo));
+ for (String segment : segmentsFrom) {
+ Preconditions.checkState(segmentsForTable.contains(segment),
Review Comment:
The new code will never log the full `segmentsFrom/segmentsTo` in case of
error. I think that those information may be helpful when building the context
during the debugging process. Please think about it.
--
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]
