Jackie-Jiang commented on code in PR #19947:
URL: https://github.com/apache/pinot/pull/19947#discussion_r4319107048
##########
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-spark-3/src/main/java/org/apache/pinot/plugin/ingestion/batch/spark3/SparkSegmentMetadataPushJobRunner.java:
##########
@@ -107,6 +105,9 @@ public void run()
// Collect files to process
List<String> segmentsToPush = getSegmentsToPush(outputDirFS, outputDirURI);
+ if (segmentsToPush.isEmpty()) {
+ return;
+ }
Review Comment:
[P1] Preserve empty REFRESH replacements
For a populated REFRESH table with consistent push enabled and
pushParallelism=1 (the default), this return skips the replacement protocol
when the new snapshot is empty. Previously, handleConsistentPush() called
preUpload() with an empty replacement set and then postUpload(), atomically
removing the old segments. The controller explicitly supports this case;
PinotHelixResourceManagerStatelessTest tests empty segmentsTo as atomic
removal. The job now reports success while leaving the previous snapshot
queryable. Please determine consistent-push mode before returning and preserve
the replacement lifecycle for empty snapshots, while skipping unnecessary
upload dispatch.
##########
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 = "file".equals(outputDirURI.getScheme()) && new
File(file).isAbsolute()
+ ? new File(file).toURI() : URI.create(file);
Review Comment:
[P2] Preserve nullable output-directory support
IngestionUtils.uploadSegment() passes null here when output.segment.dir.uri
is omitted. METADATA uploads of local archives with a nonempty
push.segmentUriPrefix previously worked: getOutputPinotFS() defaults a null
directory to the local filesystem, and generateSegmentTarURI() uses the prefix
without accessing the directory. This new outputDirURI.getScheme() dereference
throws NullPointerException before upload. Please guard outputDirURI before
accessing its scheme and add coverage for this configuration.
--
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]
