You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "jackluo923 (via GitHub)" <gi...@apache.org> on 2024/08/08 19:37:15 UTC

[PR] [Feature] Add the ability to compress and decompress tar archive file with ZStandard and LZ4 compressors [pinot]

jackluo923 opened a new pull request, #13782:
URL: https://github.com/apache/pinot/pull/13782

   This pull request enhances tar archive compression and decompression by adding support for ZStandard and LZ4, alongside GZip. The code is refactored for easy extensibility, allowing new compressors to be added with minimal effort if supported by Apache Commons Compress.
   
   Compression typically infers the compressor from the file extension, but it can also be specified by name. Decompression uses magic numbers to automatically identify the compressor, utilizing Apache Commons Compress functionality.
   
   This update is a step towards enabling ZStandard compression for segment data in Deepstore. As noted in the flamegraph profiling in issue #13319, GZip compression is currently a CPU bottleneck, consuming 5-10% of overall CPU usage in petabyte-scale production environments.


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] [Feature] Add the ability to compress and decompress tar archive file with ZStandard and LZ4 compressors [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #13782:
URL: https://github.com/apache/pinot/pull/13782#issuecomment-2276615307

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/13782?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `59.74026%` with `31 lines` in your changes missing coverage. Please review.
   > Project coverage is 61.93%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`53f9644`)](https://app.codecov.io/gh/apache/pinot/commit/53f96441ec8ab320a0170c5230a783b584ef377f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 858 commits behind head on master.
   
   | [Files](https://app.codecov.io/gh/apache/pinot/pull/13782?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...he/pinot/segment/local/utils/SegmentPushUtils.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Futils%2FSegmentPushUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9TZWdtZW50UHVzaFV0aWxzLmphdmE=) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...apache/pinot/common/utils/TarCompressionUtils.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Futils%2FTarCompressionUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvVGFyQ29tcHJlc3Npb25VdGlscy5qYXZh) | 90.62% | [1 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Frealtime%2FRealtimeSegmentDataManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...n/batch/spark/SparkSegmentGenerationJobRunner.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-plugins%2Fpinot-batch-ingestion%2Fpinot-batch-ingestion-spark-2.4%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fplugin%2Fingestion%2Fbatch%2Fspark%2FSparkSegmentGenerationJobRunner.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcGx1Z2lucy9waW5vdC1iYXRjaC1pbmdlc3Rpb24vcGlub3QtYmF0Y2gtaW5nZXN0aW9uLXNwYXJrLTIuNC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2luZ2VzdGlvbi9iYXRjaC9zcGFyay9TcGFya1NlZ21lbnRHZW5lcmF0aW9uSm9iUnVubmVyLmphdmE=) | 25.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [.../batch/spark3/SparkSegmentGenerationJobRunner.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-plugins%2Fpinot-batch-ingestion%2Fpinot-batch-ingestion-spark-3%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fplugin%2Fingestion%2Fbatch%2Fspark3%2FSparkSegmentGenerationJobRunner.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcGx1Z2lucy9waW5vdC1iYXRjaC1pbmdlc3Rpb24vcGlub3QtYmF0Y2gtaW5nZXN0aW9uLXNwYXJrLTMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9pbmdlc3Rpb24vYmF0Y2gvc3BhcmszL1NwYXJrU2VnbWVudEdlbmVyYXRpb25Kb2JSdW5uZXIuamF2YQ==) | 25.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [.../tasks/BaseMultipleSegmentsConversionExecutor.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-plugins%2Fpinot-minion-tasks%2Fpinot-minion-builtin-tasks%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fplugin%2Fminion%2Ftasks%2FBaseMultipleSegmentsConversionExecutor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvQmFzZU11bHRpcGxlU2VnbWVudHNDb252ZXJzaW9uRXhlY3V0b3IuamF2YQ==) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...c/main/java/org/apache/pinot/compat/SegmentOp.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-compatibility-verifier%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcompat%2FSegmentOp.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tcGF0aWJpbGl0eS12ZXJpZmllci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29tcGF0L1NlZ21lbnRPcC5qYXZh) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...er/api/resources/LLCSegmentCompletionHandlers.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-controller%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcontroller%2Fapi%2Fresources%2FLLCSegmentCompletionHandlers.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ion/tasks/BaseSingleSegmentConversionExecutor.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-plugins%2Fpinot-minion-tasks%2Fpinot-minion-builtin-tasks%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fplugin%2Fminion%2Ftasks%2FBaseSingleSegmentConversionExecutor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvQmFzZVNpbmdsZVNlZ21lbnRDb252ZXJzaW9uRXhlY3V0b3IuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...org/apache/pinot/common/utils/http/HttpClient.java](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Futils%2Fhttp%2FHttpClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaHR0cC9IdHRwQ2xpZW50LmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | ... and [4 more](https://app.codecov.io/gh/apache/pinot/pull/13782?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #13782      +/-   ##
   ============================================
   + Coverage     61.75%   61.93%   +0.18%     
   + Complexity      207      198       -9     
   ============================================
     Files          2436     2557     +121     
     Lines        133233   141087    +7854     
     Branches      20636    21940    +1304     
   ============================================
   + Hits          82274    87383    +5109     
   - Misses        44911    47036    +2125     
   - Partials       6048     6668     +620     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
   | [java-11](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.93% <59.74%> (+0.22%)` | :arrow_up: |
   | [java-21](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.78% <19.48%> (-33.84%)` | :arrow_down: |
   | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.93% <59.74%> (+0.18%)` | :arrow_up: |
   | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.78% <19.48%> (+0.05%)` | :arrow_up: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.93% <59.74%> (+0.18%)` | :arrow_up: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.93% <59.74%> (+0.18%)` | :arrow_up: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.39% <68.88%> (-0.51%)` | :arrow_down: |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/13782/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.78% <19.48%> (+0.05%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/13782?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] [Feature] Add the ability to compress and decompress tar archive file with ZStandard and LZ4 compressors [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #13782:
URL: https://github.com/apache/pinot/pull/13782#discussion_r1715943217


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java:
##########
@@ -1209,7 +1210,7 @@ public int downloadFile(URI uri, File dest, AuthProvider authProvider, List<Head
    */
   public File downloadUntarFileStreamed(URI uri, File dest, AuthProvider authProvider, List<Header> httpHeaders,
       long maxStreamRateInByte)
-      throws IOException, HttpErrorStatusException {
+      throws IOException, CompressorException, HttpErrorStatusException {

Review Comment:
   Consider wrapping `CompressorException` into `IOException` or `RuntimeException` in the `TarCompressionUtils` so that we don't need to change caller code. I don't think it is useful to propagate this exception



##########
pinot-common/src/main/java/org/apache/pinot/common/utils/TarCompressionUtils.java:
##########
@@ -66,106 +67,122 @@ public class TarGzCompressionUtils {
    */
 
 
-  private TarGzCompressionUtils() {
+  private TarCompressionUtils() {
   }
 
   public static final String TAR_GZ_FILE_EXTENSION = ".tar.gz";
+  public static final String TAR_LZ4_FILE_EXTENSION = ".tar.lz4";
+  public static final String TAR_ZST_FILE_EXTENSION = ".tar.zst";
+  public static final Map<String, String> COMPRESSOR_NAME_BY_FILE_EXTENSIONS = Map.of(
+      TAR_GZ_FILE_EXTENSION, CompressorStreamFactory.GZIP,
+      TAR_LZ4_FILE_EXTENSION, CompressorStreamFactory.LZ4_FRAMED,
+      TAR_ZST_FILE_EXTENSION, CompressorStreamFactory.ZSTANDARD
+  );
+  private static final CompressorStreamFactory COMPRESSOR_STREAM_FACTORY = new CompressorStreamFactory();

Review Comment:
   There is already a singleton defined in `CompressorStreamFactory`



##########
pinot-common/src/main/java/org/apache/pinot/common/utils/TarCompressionUtils.java:
##########
@@ -66,106 +67,122 @@ public class TarGzCompressionUtils {
    */
 
 
-  private TarGzCompressionUtils() {
+  private TarCompressionUtils() {
   }
 
   public static final String TAR_GZ_FILE_EXTENSION = ".tar.gz";
+  public static final String TAR_LZ4_FILE_EXTENSION = ".tar.lz4";
+  public static final String TAR_ZST_FILE_EXTENSION = ".tar.zst";
+  public static final Map<String, String> COMPRESSOR_NAME_BY_FILE_EXTENSIONS = Map.of(
+      TAR_GZ_FILE_EXTENSION, CompressorStreamFactory.GZIP,
+      TAR_LZ4_FILE_EXTENSION, CompressorStreamFactory.LZ4_FRAMED,
+      TAR_ZST_FILE_EXTENSION, CompressorStreamFactory.ZSTANDARD
+  );
+  private static final CompressorStreamFactory COMPRESSOR_STREAM_FACTORY = new CompressorStreamFactory();
   private static final char ENTRY_NAME_SEPARATOR = '/';
 
   /**
-   * Creates a tar.gz file from the input file/directory to the output file. The output file must have ".tar.gz" as the
-   * file extension.
+   * Creates a compressed tar file from the input file/directory to the output file. The output file must have
+   * a supported compressed tar file extension as the file extension such as ".tar.gz" or ".tar.zst"
    */
-  public static void createTarGzFile(File inputFile, File outputFile)
-      throws IOException {
-    createTarGzFile(new File[]{inputFile}, outputFile);
+  public static void createCompressedTarFile(File inputFile, File outputFile)
+      throws IOException, CompressorException {
+    createCompressedTarFile(new File[]{inputFile}, outputFile);
   }
 
   /**
-   * Creates a tar.gz file from a list of input file/directories to the output file. The output file must have
-   * ".tar.gz" as the file extension.
+   * Creates a compressed tar file from a list of input file/directories to the output file. The output file must have
+   * a supported file extension such as "tar.gz" or "tar.zst"
    */
-  public static void createTarGzFile(File[] inputFiles, File outputFile)
-      throws IOException {
-    Preconditions.checkArgument(outputFile.getName().endsWith(TAR_GZ_FILE_EXTENSION),
-        "Output file: %s does not have '.tar.gz' file extension", outputFile);
+  public static void createCompressedTarFile(File[] inputFiles, File outputFile)
+      throws IOException, CompressorException {
+    String compressorName = null;
+    for (String supportedCompressorExtension : COMPRESSOR_NAME_BY_FILE_EXTENSIONS.keySet()) {
+      if (outputFile.getName().endsWith(supportedCompressorExtension)) {
+        compressorName = COMPRESSOR_NAME_BY_FILE_EXTENSIONS.get(supportedCompressorExtension);
+        break;
+      }
+    }
+    Preconditions.checkState(null != compressorName,
+        "Output file: %s does not have a supported compressed tar file extension", outputFile);
     try (OutputStream fileOut = Files.newOutputStream(outputFile.toPath());
-        BufferedOutputStream bufferedOut = new BufferedOutputStream(fileOut);
-        OutputStream gzipOut = new GzipCompressorOutputStream(bufferedOut);
-        TarArchiveOutputStream tarGzOut = new TarArchiveOutputStream(gzipOut)) {
-      tarGzOut.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_STAR);
-      tarGzOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);
+         BufferedOutputStream bufferedOut = new BufferedOutputStream(fileOut);
+         OutputStream compressorOut =
+             COMPRESSOR_STREAM_FACTORY.createCompressorOutputStream(compressorName, bufferedOut);
+         TarArchiveOutputStream tarOut = new TarArchiveOutputStream(compressorOut)) {
+      tarOut.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_STAR);
+      tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);
 
       for (File inputFile : inputFiles) {
-        addFileToTarGz(tarGzOut, inputFile, "");
+        addFileToCompressedTar(tarOut, inputFile, "");
       }
     }
   }
 
   /**
-   * Helper method to write a file into the tar.gz file output stream. The base entry name is the relative path of the
-   * file to the root directory.
+   * Helper method to write a file into the compressed tar file output stream. The base entry name is the relative
+   * path of the file to the root directory.
    */
-  private static void addFileToTarGz(ArchiveOutputStream tarGzOut, File file, String baseEntryName)
+  private static void addFileToCompressedTar(ArchiveOutputStream tarOut, File file, String baseEntryName)
       throws IOException {
     String entryName = baseEntryName + file.getName();
     TarArchiveEntry entry = new TarArchiveEntry(file, entryName);
-    tarGzOut.putArchiveEntry(entry);
+    tarOut.putArchiveEntry(entry);
     if (file.isFile()) {
       try (InputStream in = Files.newInputStream(file.toPath())) {
-        IOUtils.copy(in, tarGzOut);
+        IOUtils.copy(in, tarOut);
       }
-      tarGzOut.closeArchiveEntry();
+      tarOut.closeArchiveEntry();
     } else {
-      tarGzOut.closeArchiveEntry();
+      tarOut.closeArchiveEntry();
 
       File[] children = file.listFiles();
       assert children != null;
       String baseEntryNameForChildren = entryName + ENTRY_NAME_SEPARATOR;
       for (File child : children) {
-        addFileToTarGz(tarGzOut, child, baseEntryNameForChildren);
+        addFileToCompressedTar(tarOut, child, baseEntryNameForChildren);
       }
     }
   }
 
   /**
-   * Un-tars a tar.gz file into a directory, returns all the untarred files/directories.
+   * Un-tars a compressed tar file into a directory, returns all the untarred files/directories.
    * <p>For security reason, the untarred files must reside in the output directory.
    */
   public static List<File> untar(File inputFile, File outputDir)
-      throws IOException {
+      throws IOException, CompressorException {
     try (InputStream fileIn = Files.newInputStream(inputFile.toPath())) {
       return untar(fileIn, outputDir);
     }
   }
 
   /**
-   * Un-tars an inputstream of a tar.gz file into a directory, returns all the untarred files/directories.
+   * Un-tars an inputstream of a compressed tar file into a directory, returns all the untarred files/directories.
    * <p>For security reason, the untarred files must reside in the output directory.
    */
   public static List<File> untar(InputStream inputStream, File outputDir)
-      throws IOException {
+      throws IOException, CompressorException {
     return untarWithRateLimiter(inputStream, outputDir, NO_DISK_WRITE_RATE_LIMIT);
   }
 
   /**
-   * Un-tars an inputstream of a tar.gz file into a directory, returns all the untarred files/directories.
+   * Un-tars an inputstream of a compressed tar file into a directory, returns all the untarred files/directories.
    * RateLimit limits the untar rate
    * <p>For security reason, the untarred files must reside in the output directory.
    */
   public static List<File> untarWithRateLimiter(InputStream inputStream, File outputDir, long maxStreamRateInByte)
-      throws IOException {
+      throws IOException, CompressorException {
     String outputDirCanonicalPath = outputDir.getCanonicalPath();
     // Prevent partial path traversal
     if (!outputDirCanonicalPath.endsWith(File.separator)) {
       outputDirCanonicalPath += File.separator;
     }
     List<File> untarredFiles = new ArrayList<>();
     try (InputStream bufferedIn = new BufferedInputStream(inputStream);

Review Comment:
   (format) Please follow [Pinot Style](https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#set-up-ide), same for other places



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] [Feature] Add the ability to compress and decompress tar archive file with ZStandard and LZ4 compressors [pinot]

Posted by "jackluo923 (via GitHub)" <gi...@apache.org>.
jackluo923 commented on code in PR #13782:
URL: https://github.com/apache/pinot/pull/13782#discussion_r1717393491


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/TarCompressionUtils.java:
##########
@@ -66,106 +67,122 @@ public class TarGzCompressionUtils {
    */
 
 
-  private TarGzCompressionUtils() {
+  private TarCompressionUtils() {
   }
 
   public static final String TAR_GZ_FILE_EXTENSION = ".tar.gz";
+  public static final String TAR_LZ4_FILE_EXTENSION = ".tar.lz4";
+  public static final String TAR_ZST_FILE_EXTENSION = ".tar.zst";
+  public static final Map<String, String> COMPRESSOR_NAME_BY_FILE_EXTENSIONS = Map.of(
+      TAR_GZ_FILE_EXTENSION, CompressorStreamFactory.GZIP,
+      TAR_LZ4_FILE_EXTENSION, CompressorStreamFactory.LZ4_FRAMED,
+      TAR_ZST_FILE_EXTENSION, CompressorStreamFactory.ZSTANDARD
+  );
+  private static final CompressorStreamFactory COMPRESSOR_STREAM_FACTORY = new CompressorStreamFactory();

Review Comment:
   Switch to using singleleton



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] [Feature] Add the ability to compress and decompress tar archive file with ZStandard and LZ4 compressors [pinot]

Posted by "jackluo923 (via GitHub)" <gi...@apache.org>.
jackluo923 commented on code in PR #13782:
URL: https://github.com/apache/pinot/pull/13782#discussion_r1717393889


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java:
##########
@@ -1209,7 +1210,7 @@ public int downloadFile(URI uri, File dest, AuthProvider authProvider, List<Head
    */
   public File downloadUntarFileStreamed(URI uri, File dest, AuthProvider authProvider, List<Header> httpHeaders,
       long maxStreamRateInByte)
-      throws IOException, HttpErrorStatusException {
+      throws IOException, CompressorException, HttpErrorStatusException {

Review Comment:
   Wrapped `CompressorException` into `IOException`



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] [Feature] Add the ability to compress and decompress tar archive file with ZStandard and LZ4 compressors [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #13782:
URL: https://github.com/apache/pinot/pull/13782


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] [Feature] Add the ability to compress and decompress tar archive file with ZStandard and LZ4 compressors [pinot]

Posted by "jackluo923 (via GitHub)" <gi...@apache.org>.
jackluo923 commented on code in PR #13782:
URL: https://github.com/apache/pinot/pull/13782#discussion_r1717394353


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/TarCompressionUtils.java:
##########
@@ -66,106 +67,122 @@ public class TarGzCompressionUtils {
    */
 
 
-  private TarGzCompressionUtils() {
+  private TarCompressionUtils() {
   }
 
   public static final String TAR_GZ_FILE_EXTENSION = ".tar.gz";
+  public static final String TAR_LZ4_FILE_EXTENSION = ".tar.lz4";
+  public static final String TAR_ZST_FILE_EXTENSION = ".tar.zst";
+  public static final Map<String, String> COMPRESSOR_NAME_BY_FILE_EXTENSIONS = Map.of(
+      TAR_GZ_FILE_EXTENSION, CompressorStreamFactory.GZIP,
+      TAR_LZ4_FILE_EXTENSION, CompressorStreamFactory.LZ4_FRAMED,
+      TAR_ZST_FILE_EXTENSION, CompressorStreamFactory.ZSTANDARD
+  );
+  private static final CompressorStreamFactory COMPRESSOR_STREAM_FACTORY = new CompressorStreamFactory();
   private static final char ENTRY_NAME_SEPARATOR = '/';
 
   /**
-   * Creates a tar.gz file from the input file/directory to the output file. The output file must have ".tar.gz" as the
-   * file extension.
+   * Creates a compressed tar file from the input file/directory to the output file. The output file must have
+   * a supported compressed tar file extension as the file extension such as ".tar.gz" or ".tar.zst"
    */
-  public static void createTarGzFile(File inputFile, File outputFile)
-      throws IOException {
-    createTarGzFile(new File[]{inputFile}, outputFile);
+  public static void createCompressedTarFile(File inputFile, File outputFile)
+      throws IOException, CompressorException {
+    createCompressedTarFile(new File[]{inputFile}, outputFile);
   }
 
   /**
-   * Creates a tar.gz file from a list of input file/directories to the output file. The output file must have
-   * ".tar.gz" as the file extension.
+   * Creates a compressed tar file from a list of input file/directories to the output file. The output file must have
+   * a supported file extension such as "tar.gz" or "tar.zst"
    */
-  public static void createTarGzFile(File[] inputFiles, File outputFile)
-      throws IOException {
-    Preconditions.checkArgument(outputFile.getName().endsWith(TAR_GZ_FILE_EXTENSION),
-        "Output file: %s does not have '.tar.gz' file extension", outputFile);
+  public static void createCompressedTarFile(File[] inputFiles, File outputFile)
+      throws IOException, CompressorException {
+    String compressorName = null;
+    for (String supportedCompressorExtension : COMPRESSOR_NAME_BY_FILE_EXTENSIONS.keySet()) {
+      if (outputFile.getName().endsWith(supportedCompressorExtension)) {
+        compressorName = COMPRESSOR_NAME_BY_FILE_EXTENSIONS.get(supportedCompressorExtension);
+        break;
+      }
+    }
+    Preconditions.checkState(null != compressorName,
+        "Output file: %s does not have a supported compressed tar file extension", outputFile);
     try (OutputStream fileOut = Files.newOutputStream(outputFile.toPath());
-        BufferedOutputStream bufferedOut = new BufferedOutputStream(fileOut);
-        OutputStream gzipOut = new GzipCompressorOutputStream(bufferedOut);
-        TarArchiveOutputStream tarGzOut = new TarArchiveOutputStream(gzipOut)) {
-      tarGzOut.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_STAR);
-      tarGzOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);
+         BufferedOutputStream bufferedOut = new BufferedOutputStream(fileOut);
+         OutputStream compressorOut =
+             COMPRESSOR_STREAM_FACTORY.createCompressorOutputStream(compressorName, bufferedOut);
+         TarArchiveOutputStream tarOut = new TarArchiveOutputStream(compressorOut)) {
+      tarOut.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_STAR);
+      tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);
 
       for (File inputFile : inputFiles) {
-        addFileToTarGz(tarGzOut, inputFile, "");
+        addFileToCompressedTar(tarOut, inputFile, "");
       }
     }
   }
 
   /**
-   * Helper method to write a file into the tar.gz file output stream. The base entry name is the relative path of the
-   * file to the root directory.
+   * Helper method to write a file into the compressed tar file output stream. The base entry name is the relative
+   * path of the file to the root directory.
    */
-  private static void addFileToTarGz(ArchiveOutputStream tarGzOut, File file, String baseEntryName)
+  private static void addFileToCompressedTar(ArchiveOutputStream tarOut, File file, String baseEntryName)
       throws IOException {
     String entryName = baseEntryName + file.getName();
     TarArchiveEntry entry = new TarArchiveEntry(file, entryName);
-    tarGzOut.putArchiveEntry(entry);
+    tarOut.putArchiveEntry(entry);
     if (file.isFile()) {
       try (InputStream in = Files.newInputStream(file.toPath())) {
-        IOUtils.copy(in, tarGzOut);
+        IOUtils.copy(in, tarOut);
       }
-      tarGzOut.closeArchiveEntry();
+      tarOut.closeArchiveEntry();
     } else {
-      tarGzOut.closeArchiveEntry();
+      tarOut.closeArchiveEntry();
 
       File[] children = file.listFiles();
       assert children != null;
       String baseEntryNameForChildren = entryName + ENTRY_NAME_SEPARATOR;
       for (File child : children) {
-        addFileToTarGz(tarGzOut, child, baseEntryNameForChildren);
+        addFileToCompressedTar(tarOut, child, baseEntryNameForChildren);
       }
     }
   }
 
   /**
-   * Un-tars a tar.gz file into a directory, returns all the untarred files/directories.
+   * Un-tars a compressed tar file into a directory, returns all the untarred files/directories.
    * <p>For security reason, the untarred files must reside in the output directory.
    */
   public static List<File> untar(File inputFile, File outputDir)
-      throws IOException {
+      throws IOException, CompressorException {
     try (InputStream fileIn = Files.newInputStream(inputFile.toPath())) {
       return untar(fileIn, outputDir);
     }
   }
 
   /**
-   * Un-tars an inputstream of a tar.gz file into a directory, returns all the untarred files/directories.
+   * Un-tars an inputstream of a compressed tar file into a directory, returns all the untarred files/directories.
    * <p>For security reason, the untarred files must reside in the output directory.
    */
   public static List<File> untar(InputStream inputStream, File outputDir)
-      throws IOException {
+      throws IOException, CompressorException {
     return untarWithRateLimiter(inputStream, outputDir, NO_DISK_WRITE_RATE_LIMIT);
   }
 
   /**
-   * Un-tars an inputstream of a tar.gz file into a directory, returns all the untarred files/directories.
+   * Un-tars an inputstream of a compressed tar file into a directory, returns all the untarred files/directories.
    * RateLimit limits the untar rate
    * <p>For security reason, the untarred files must reside in the output directory.
    */
   public static List<File> untarWithRateLimiter(InputStream inputStream, File outputDir, long maxStreamRateInByte)
-      throws IOException {
+      throws IOException, CompressorException {
     String outputDirCanonicalPath = outputDir.getCanonicalPath();
     // Prevent partial path traversal
     if (!outputDirCanonicalPath.endsWith(File.separator)) {
       outputDirCanonicalPath += File.separator;
     }
     List<File> untarredFiles = new ArrayList<>();
     try (InputStream bufferedIn = new BufferedInputStream(inputStream);

Review Comment:
   Switch to using Pinot Style and adjusted style in various files.



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org