xiangfu0 commented on code in PR #19898:
URL: https://github.com/apache/pinot/pull/19898#discussion_r4289317649
##########
pinot-spi/src/main/java/org/apache/pinot/spi/ingestion/batch/spec/Constants.java:
##########
@@ -18,11 +18,23 @@
*/
package org.apache.pinot.spi.ingestion.batch.spec;
+import java.util.Map;
+
public class Constants {
private Constants() {
}
/// By default Pinot segments are compressed in 'tar.gz' format then pushed
to controller.
public static final String TAR_GZ_FILE_EXT = ".tar.gz";
public static final String METADATA_TAR_GZ_FILE_EXT = ".metadata.tar.gz";
+
+ /// Archive extensions understood by segment upload and metadata extraction,
including existing LZ4 archives.
+ public static final Map<String, String> SEGMENT_TAR_FILE_EXTENSIONS =
Review Comment:
Added an opt-in upgrade caveat: every controller, server, minion, and
external tool that consumes these archives must support .tar.zst before
generation is enabled. TarCompressionUtils already supports Zstandard on the
current base; this does not require this PR itself to be deployed on every
consumer.
Follow-up:
[38ef4bb6ea](https://github.com/apache/pinot/commit/38ef4bb6eaf4b97c645768b7fe8dacf00f8ce734).
##########
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-standalone/src/main/java/org/apache/pinot/plugin/ingestion/batch/standalone/SegmentGenerationJobRunner.java:
##########
@@ -230,9 +232,17 @@ private void submitSegmentGenTask(File localTempDir, URI
inputFileURI, int seqId
File localOutputTempDir = new File(localTempDir, "output");
FileUtils.forceMkdir(localOutputTempDir);
- //copy input path to local
- File localInputDataFile = createLocalInputDateFile(inputFileURI,
localInputTempDir);
- _inputDirFS.copyToLocalFile(inputFileURI, localInputDataFile);
+ // Gzip readers can unpack beside the input, so keep those writes inside
an owned staging directory.
+ File inputFile = new File(URIUtils.decode(inputFileURI.getRawPath()));
+ boolean borrowedInputFile =
PinotFSFactory.isFileSystemInstanceOf(_inputDirFS, LocalPinotFS.class)
+ && !RecordReaderUtils.isGZippedFile(inputFile);
Review Comment:
I checked the previous path: copyToLocalFile also runs before task
submission, outside the per-file generation catch. The gzip probe opens the
decoded absolute input path, so FileNotFoundException already identifies the
missing local input. There is no lost per-file wrapper to restore; I kept this
optional diagnostic change out to preserve the existing exception behavior.
--
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]
