xiangfu0 commented on code in PR #19947:
URL: https://github.com/apache/pinot/pull/19947#discussion_r4364890563
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SegmentPushUtils.java:
##########
@@ -684,17 +702,20 @@ public static Map<String, String>
getSegmentUriToTarPathMap(URI outputDirURI, Pu
}
}
- URI uri = URI.create(file);
+ URI uri = outputDirURI != null &&
"file".equals(outputDirURI.getScheme()) && new File(file).isAbsolute()
Review Comment:
Fixed in
[a8dbb604b3](https://github.com/apache/pinot/commit/a8dbb604b3453f67f35ecab6acf68580a2f1ab3b):
when outputDirURI and prefix/suffix are absent, generateSegmentTarURI returns
the input URI unchanged. Extended the existing test with an absolute path and a
fully qualified HTTPS URI without a prefix, retaining the prefix case. All
targeted tests passed.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SegmentPushUtils.java:
##########
@@ -462,6 +464,20 @@ private static File
getSegmentMetadataFile(SegmentGenerationJobSpec spec, PinotF
return generateSegmentMetadataFile(fileSystem, URI.create(tarFilePath));
}
+ /// Rejects ambiguous uploads before HTTP or segment replacement starts.
Archives in different output directories
+ /// can otherwise overwrite the same segment in an arbitrary order.
+ /// Extensionless deep-store segment paths remain supported for metadata
push.
+ public static void validateUniqueSegmentNames(Collection<String>
segmentPaths) {
+ Map<String, String> pathsBySegmentName = new HashMap<>();
+ for (String path : segmentPaths) {
+ String segmentName = getSegmentName(path);
+ String previous = pathsBySegmentName.putIfAbsent(segmentName, path);
+ Preconditions.checkArgument(previous == null,
+ "Multiple archives for segment %s: %s and %s. Remove stale archives
or narrow the push file pattern.",
Review Comment:
Keeping the first conflicting pair to preserve fail-fast validation and
minimal scope. The loop throws on the second archive for a name, so it never
processes the third or any intermediate collision. The error identifies both
conflicting paths and suggests removing stale output or narrowing
pushFileNamePattern.
--
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]
