ORC-817, ORC-1088: Support ZStandard compression using zstd-jni #1743

cxzl25 · 2024-01-11T12:02:49Z

What changes were proposed in this pull request?

Original PR: #988
Original author: @dchristle

This PR will support the use of zstd-jni library as the implementation of ORC zstd, with better performance than aircompressor. (#988 (comment))

This PR also exposes the compression level and "long mode" settings to ORC users. These settings allow the user to select different speed/compression trade-offs that were not supported by the original aircompressor.

Add zstd-jni dependency, and add a new CompressionCodec ZstdCodec that uses it. Add ORC conf to set compression level.
Add ORC conf to use long mode, and add configuration setters for windowLog.
Add tests that verify the correctness of writing and reading across compression levels, window sizes, and long mode use.
Add test for compatibility between Zstd aircompressor and zstd-jni implementations.

Why are the changes needed?

These change makes sense for a few reasons:

ORC users will gain all the improvements from the main zstd library. It is under active development and receives regular speed and compression improvements. In contrast, aircompressor's zstd implementation is older and stale.

ORC users will be able to use the entire speed/compression tradeoff space. Today, aircompressor's implementation has only one of eight compression strategies (link). This means only a small range of faster but less compressive strategies can be exposed to ORC users. ORC storage with high compression (e.g. for large-but-infrequently-used data) is a clear use case that this PR would unlock.

It will harmonize the Java ORC implementation with other projects in the Hadoop ecosystem. Parquet, Spark, and even the C++ ORC reader/writers all rely on the official zstd implementation either via zstd-jni or directly. In this way, the Java reader/writer code is an outlier.

Detection and fixing any bugs or regressions will generally happen much faster, given the larger number of users and active developer community of zstd and zstd-jni.

The largest tradeoff is that zstd-jni wraps compiled code. That said, many microprocessor architectures are already targeted & bundled into zstd-jni, so this should be a rare hurdle.

How was this patch tested?

Unit tests for reading and writing ORC files using a variety of compression levels, window logs, all pass.
Unit test to compress and decompress between aircompressor and zstd-jni passes. Note that the current aircompressor implementation uses a small subset of levels, so the test only compares data using the default compression settings.

Was this patch authored or co-authored using generative AI tooling?

No

* Add zstd-jni dependency, and add a new CompressionCodec ZstdCodec that uses it. Add ORC conf to set compression level. * Add ORC conf to use long mode, and add configuration setters for windowLog and longModeEnable. * Add tests that verify the correctness of writing and reading across compression levels, window sizes, and long mode use. * Add test for compatibility between Zstd aircompressor and zstd-jni implementations. * Fix filterWithSeek test with a smaller percentage.

dongjoon-hyun

Thank you. Could you fix the checkstyle issues?

Error:  src/java/org/apache/orc/impl/WriterImpl.java:[311,13] (indentation) Indentation: 'new' has incorrect indentation level 12, expected level should be 14.
Error:  src/java/org/apache/orc/impl/WriterImpl.java:[317,15] (indentation) Indentation: 'new' has incorrect indentation level 14, expected level should be 16.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[23,1] (imports) CustomImportOrder: Import statement for 'com.github.luben.zstd.Zstd' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[24,1] (imports) CustomImportOrder: Import statement for 'com.github.luben.zstd.ZstdCompressCtx' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[25,1] (imports) CustomImportOrder: Import statement for 'com.github.luben.zstd.ZstdDecompressCtx' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[27,1] (imports) CustomImportOrder: Import statement for 'org.apache.orc.CompressionCodec' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[28,1] (imports) CustomImportOrder: Import statement for 'org.apache.orc.CompressionKind' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[29,1] (imports) CustomImportOrder: Import statement for 'org.slf4j.Logger' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[30,1] (imports) CustomImportOrder: Import statement for 'org.slf4j.LoggerFactory' is in the wrong order. Should be in the 'THIRD_PARTY_PACKAGE' group, expecting not assigned imports on this line.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[105] (regexp) RegexpSinglelineJava: No starting LAND and LOR allowed.
Error:  src/java/org/apache/orc/impl/ZstdCodec.java:[143] (regexp) RegexpSinglelineJava: No starting LAND and LOR allowed.
Error:  src/java/org/apache/orc/impl/OrcCodecPool.java:[49] (sizes) LineLength: Line is longer than 100 characters (found 103).
Error:  src/test/org/apache/orc/TestRowFilteringComplexTypesNulls.java:[36,8] (imports) UnusedImports: Unused import -

cxzl25 · 2024-01-11T13:59:26Z

java/core/src/java/org/apache/orc/OrcConf.java

+  COMPRESSION_ZSTD_IMPL("orc.compression.zstd.impl",
+          "hive.exec.orc.compression.zstd.impl", "java",
+          "Define the implementation used with the ZStandard codec, java or jni."),
+  COMPRESSION_ZSTD_LEVEL("orc.compression.zstd.level",


Also fix ORC-1088

source table: ORC zlib 4408374802439 4TB

zstd-jni

orc.compression.zstd.level=3 (default)
zstd compress size: 3119313447131 2905G

orc.compression.zstd.level=10
zstd compress size: 2621369844393 2441G

aircompressor

zstd compress size: 3138804372295 2923G

java/core/src/java/org/apache/orc/OrcConf.java

java/core/src/java/org/apache/orc/impl/OrcCodecPool.java

This reverts commit 7ff24ba

java/pom.xml

maven properties keep the alphabetical order

java/core/src/java/org/apache/orc/impl/PhysicalFsWriter.java

java/core/src/java/org/apache/orc/impl/WriterImpl.java

dongjoon-hyun

Since we can control the compression level, I'd like to propose to use orc.compression.zstd.level=1 like Apache Spark.

https://github.com/apache/spark/blob/ebd2b78f87fa6086c41d5e6bcade5efeefac75d0/core/src/main/scala/org/apache/spark/internal/config/package.scala#L1930-L1936

  private[spark] val IO_COMPRESSION_ZSTD_LEVEL =
    ConfigBuilder("spark.io.compression.zstd.level")
      .doc("Compression level for Zstd compression codec. Increasing the compression " +
        "level will result in better compression at the expense of more CPU and memory")
      .version("2.3.0")
      .intConf
      .createWithDefault(1)

For zstd-jni library, Apache Spark community decides to use Level 1 since Apache Spark 2.3.0 because of the compression speed.

================================================================================================
Benchmark ZStandardCompressionCodec
================================================================================================

OpenJDK 64-Bit Server VM 17.0.9+9-LTS on Linux 5.15.0-1053-azure
AMD EPYC 7763 64-Core Processor
Benchmark ZStandardCompressionCodec:                    Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
--------------------------------------------------------------------------------------------------------------------------------------
Compression 10000 times at level 1 without buffer pool            661            665           4          0.0       66093.0       1.0X
Compression 10000 times at level 2 without buffer pool            705            707           2          0.0       70472.6       0.9X
Compression 10000 times at level 3 without buffer pool            796            796           0          0.0       79570.7       0.8X
Compression 10000 times at level 1 with buffer pool               588            589           1          0.0       58835.3       1.1X
Compression 10000 times at level 2 with buffer pool               620            621           1          0.0       61982.9       1.1X
Compression 10000 times at level 3 with buffer pool               725            726           1          0.0       72460.9       0.9X

dongjoon-hyun

BTW, for the rest of the code looks good to me, @cxzl25 . Thank you for your hard work on this. Let me test a little more with your PR.

dongjoon-hyun · 2024-01-16T22:27:08Z

I changed the default level to 1 and compared quickly with the generate benchmark. Level 1 is still smaller.

JAVA (Aircompressor)

$ java -Dorc.compression.zstd.impl=java -jar core/target/orc-benchmarks-core-*-uber.jar generate data -f orc -d sales -s 100000
$ ls -alR data | tail -n3
-rw-r--r--  1 dongjoon  staff  10746324 Jan 16 14:23 orc.gz
-rw-r--r--  1 dongjoon  staff  12133885 Jan 16 14:23 orc.snappy
-rw-r--r--  1 dongjoon  staff  10642346 Jan 16 14:23 orc.zstd

ZSTD-JNI

$ java -jar core/target/orc-benchmarks-core-*-uber.jar generate data -f orc -d sales -s 100000
$ ls -alR data | tail -n3
-rw-r--r--  1 dongjoon  staff  10746324 Jan 16 14:23 orc.gz
-rw-r--r--  1 dongjoon  staff  12133885 Jan 16 14:23 orc.snappy
-rw-r--r--  1 dongjoon  staff  10543260 Jan 16 14:23 orc.zstd

dongjoon-hyun

+1, LGTM (Pending CIs)

dongjoon-hyun · 2024-01-16T22:44:55Z

java/core/src/java/org/apache/orc/impl/ZstdCodec.java

+    zstdCompressCtx = new ZstdCompressCtx();
+    zstdCompressCtx.setLevel(zso.level);
+    zstdCompressCtx.setLong(zso.windowLog);
+    zstdCompressCtx.setChecksum(false);


For the record, the default value is false already.

https://javadoc.io/static/com.github.luben/zstd-jni/1.5.5-11/com/github/luben/zstd/ZstdCompressCtx.html#setChecksum(boolean)

### What changes were proposed in this pull request? Original PR: #988 Original author: dchristle This PR will support the use of [zstd-jni](https://github.com/luben/zstd-jni) library as the implementation of ORC zstd, with better performance than [aircompressor](https://github.com/airlift/aircompressor). (#988 (comment)) This PR also exposes the compression level and "long mode" settings to ORC users. These settings allow the user to select different speed/compression trade-offs that were not supported by the original aircompressor. - Add zstd-jni dependency, and add a new CompressionCodec ZstdCodec that uses it. Add ORC conf to set compression level. - Add ORC conf to use long mode, and add configuration setters for windowLog. - Add tests that verify the correctness of writing and reading across compression levels, window sizes, and long mode use. - Add test for compatibility between Zstd aircompressor and zstd-jni implementations. ### Why are the changes needed? These change makes sense for a few reasons: ORC users will gain all the improvements from the main zstd library. It is under active development and receives regular speed and compression improvements. In contrast, aircompressor's zstd implementation is older and stale. ORC users will be able to use the entire speed/compression tradeoff space. Today, aircompressor's implementation has only one of eight compression strategies ([link](https://github.com/airlift/aircompressor/blob/c5e6972bd37e1d3834514957447028060a268eea/src/main/java/io/airlift/compress/zstd/CompressionParameters.java#L143)). This means only a small range of faster but less compressive strategies can be exposed to ORC users. ORC storage with high compression (e.g. for large-but-infrequently-used data) is a clear use case that this PR would unlock. It will harmonize the Java ORC implementation with other projects in the Hadoop ecosystem. Parquet, Spark, and even the C++ ORC reader/writers all rely on the official zstd implementation either via zstd-jni or directly. In this way, the Java reader/writer code is an outlier. Detection and fixing any bugs or regressions will generally happen much faster, given the larger number of users and active developer community of zstd and zstd-jni. The largest tradeoff is that zstd-jni wraps compiled code. That said, many microprocessor architectures are already targeted & bundled into zstd-jni, so this should be a rare hurdle. ### How was this patch tested? - Unit tests for reading and writing ORC files using a variety of compression levels, window logs, all pass. - Unit test to compress and decompress between aircompressor and zstd-jni passes. Note that the current aircompressor implementation uses a small subset of levels, so the test only compares data using the default compression settings. ### Was this patch authored or co-authored using generative AI tooling? No Closes #1743 from cxzl25/ORC-817. Lead-authored-by: sychen <sychen@ctrip.com> Co-authored-by: Dongjoon Hyun <dongjoon@apache.org> Co-authored-by: David Christle <dchristle@squareup.com> Co-authored-by: Yiqun Zhang <guiyanakuang@gmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 33be571) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

dongjoon-hyun · 2024-01-16T23:08:28Z

Thank you, @cxzl25 and all!

cxzl25 · 2024-01-17T04:00:20Z

Thanks for all the help!

Migrating from zlib to zstd, a table has a compression rate of 35% through aircompressor. By adjusting some parameters of zstd-jni, a compression rate of 44% is achieved.

dongjoon-hyun · 2024-01-17T16:29:45Z

My bad. It seems that I made a regression at Taxi data compression.

ORC 1.9

data/generated//taxi:
total 2196176
drwxr-xr-x  5 dongjoon  staff   160B Jan 17 08:02 .
drwxr-xr-x  5 dongjoon  staff   160B Jan 17 08:07 ..
-rw-r--r--  1 dongjoon  staff   299M Jan 17 08:03 orc.zstd

ORC 2.0

-rw-r--r--  1 dongjoon  staff   334M Jan 17 07:56 orc.zstd (level 1)
-rw-r--r--  1 dongjoon  staff   299M Jan 17 08:16 orc.zstd (level 3)
-rw-r--r--  1 dongjoon  staff   302M Jan 17 08:21 orc.zstd (level 4)
-rw-r--r--  1 dongjoon  staff   300M Jan 17 08:27 orc.zstd (level 5)

ZStd compression level looks inconsistent with this dataset and let me change the zstd level change back to 3 like the original proposal.

### What changes were proposed in this pull request? This PR aims to Upgrade Apache ORC to 2.0.0 for Apache Spark 4.0.0. Apache ORC community has 3-year support policy which is longer than Apache Spark. It's aligned like the following. - Apache ORC 2.0.x <-> Apache Spark 4.0.x - Apache ORC 1.9.x <-> Apache Spark 3.5.x - Apache ORC 1.8.x <-> Apache Spark 3.4.x - Apache ORC 1.7.x (Supported) <-> Apache Spark 3.3.x (End-Of-Support) ### Why are the changes needed? **Release Note** - https://github.com/apache/orc/releases/tag/v2.0.0 **Milestone** - https://github.com/apache/orc/milestone/20?closed=1 - apache/orc#1728 - apache/orc#1801 - apache/orc#1498 - apache/orc#1627 - apache/orc#1497 - apache/orc#1509 - apache/orc#1554 - apache/orc#1708 - apache/orc#1733 - apache/orc#1760 - apache/orc#1743 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #45443 from dongjoon-hyun/SPARK-44115. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

### What changes were proposed in this pull request? This PR aims to Upgrade Apache ORC to 2.0.0 for Apache Spark 4.0.0. Apache ORC community has 3-year support policy which is longer than Apache Spark. It's aligned like the following. - Apache ORC 2.0.x <-> Apache Spark 4.0.x - Apache ORC 1.9.x <-> Apache Spark 3.5.x - Apache ORC 1.8.x <-> Apache Spark 3.4.x - Apache ORC 1.7.x (Supported) <-> Apache Spark 3.3.x (End-Of-Support) ### Why are the changes needed? **Release Note** - https://github.com/apache/orc/releases/tag/v2.0.0 **Milestone** - https://github.com/apache/orc/milestone/20?closed=1 - apache/orc#1728 - apache/orc#1801 - apache/orc#1498 - apache/orc#1627 - apache/orc#1497 - apache/orc#1509 - apache/orc#1554 - apache/orc#1708 - apache/orc#1733 - apache/orc#1760 - apache/orc#1743 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#45443 from dongjoon-hyun/SPARK-44115. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

dchristle and others added 5 commits January 11, 2024 18:54

fix conflict

50906d1

jni fallback

f444a25

style

a0c4e90

add config(orc.compression.zstd.impl)

7ff24ba

cxzl25 marked this pull request as draft January 11, 2024 12:02

github-actions bot added BUILD JAVA labels Jan 11, 2024

dongjoon-hyun reviewed Jan 11, 2024

View reviewed changes

dongjoon-hyun added this to the 2.0.0 milestone Jan 11, 2024

cxzl25 added 3 commits January 11, 2024 20:56

checkstyle

27ab72e

checkstyle

a6ad79a

Refactor the compress and decompress methods

a2d3efd

cxzl25 commented Jan 11, 2024

View reviewed changes

java/core/src/java/org/apache/orc/OrcConf.java Outdated Show resolved Hide resolved

cxzl25 added 3 commits January 11, 2024 22:08

spotbugs

fc45249

trigger test

5d14db0

use jni to test

6f91dc4

dongjoon-hyun reviewed Jan 11, 2024

View reviewed changes

java/core/src/java/org/apache/orc/OrcConf.java Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jan 11, 2024

View reviewed changes

java/core/src/java/org/apache/orc/OrcConf.java Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jan 11, 2024

View reviewed changes

java/core/src/java/org/apache/orc/OrcConf.java Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jan 11, 2024

View reviewed changes

java/core/src/java/org/apache/orc/impl/OrcCodecPool.java Outdated Show resolved Hide resolved

Revert "add config(orc.compression.zstd.impl)"

f860f2f

This reverts commit 7ff24ba

paliwalashish reviewed Jan 12, 2024

View reviewed changes

java/pom.xml Outdated Show resolved Hide resolved

cxzl25 and others added 4 commits January 12, 2024 11:30

indentation

ee51e29

zstd-jni version property name

932eb2e

Update pom.xml

202276a

maven properties keep the alphabetical order

checkstyle

b4eb87d

paliwalashish reviewed Jan 12, 2024

View reviewed changes

java/core/src/java/org/apache/orc/impl/PhysicalFsWriter.java Show resolved Hide resolved

paliwalashish reviewed Jan 12, 2024

View reviewed changes

java/core/src/java/org/apache/orc/impl/WriterImpl.java Outdated Show resolved Hide resolved

cxzl25 added 4 commits January 12, 2024 18:23

java16 instanceOf

0292ebe

doc level range

7b8c3d6

remove unused field

7364e59

remove orc.compression.zstd.longmode

f876ead

dongjoon-hyun reviewed Jan 12, 2024

View reviewed changes

java/core/src/java/org/apache/orc/impl/WriterImpl.java Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jan 12, 2024

View reviewed changes

java/core/src/java/org/apache/orc/impl/WriterImpl.java Outdated Show resolved Hide resolved

cxzl25 added 5 commits January 13, 2024 21:51

java props orc.compression.zstd.impl

48a529f

ut

366ac5b

method order

662456e

remove setSpeed method

10d5220

wrapper class ZstdCompressOptions

5f674cb

cxzl25 marked this pull request as ready for review January 14, 2024 09:46

dongjoon-hyun reviewed Jan 16, 2024

View reviewed changes

Set ZSTD level to 1

4ae4adb

dongjoon-hyun changed the title ~~ORC-817: Support ZStandard compression using zstd-jni~~ ORC-817, ORC-1088: Support ZStandard compression using zstd-jni Jan 16, 2024

dongjoon-hyun approved these changes Jan 16, 2024

View reviewed changes

Update ZstdCodec.java

b2edbb1

dongjoon-hyun reviewed Jan 16, 2024

View reviewed changes

dongjoon-hyun closed this in 33be571 Jan 16, 2024

dongjoon-hyun mentioned this pull request Mar 8, 2024

[SPARK-44115][BUILD] Upgrade Apache ORC to 2.0.0 apache/spark#45443

Closed

zyl891229 mentioned this pull request Mar 20, 2024

[Format]ORC Support ZStandard compression using zstd-jni apache/paimon#3056

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ORC-817, ORC-1088: Support ZStandard compression using zstd-jni #1743

ORC-817, ORC-1088: Support ZStandard compression using zstd-jni #1743

cxzl25 commented Jan 11, 2024 •

edited

Loading

dongjoon-hyun left a comment

cxzl25 Jan 11, 2024

cxzl25 Jan 14, 2024

dongjoon-hyun left a comment

dongjoon-hyun left a comment

dongjoon-hyun commented Jan 16, 2024

dongjoon-hyun left a comment

dongjoon-hyun Jan 16, 2024

dongjoon-hyun commented Jan 16, 2024

cxzl25 commented Jan 17, 2024

dongjoon-hyun commented Jan 17, 2024

ORC-817, ORC-1088: Support ZStandard compression using zstd-jni #1743

ORC-817, ORC-1088: Support ZStandard compression using zstd-jni #1743

Conversation

cxzl25 commented Jan 11, 2024 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

dongjoon-hyun left a comment

Choose a reason for hiding this comment

cxzl25 Jan 11, 2024

Choose a reason for hiding this comment

cxzl25 Jan 14, 2024

Choose a reason for hiding this comment

zstd-jni

aircompressor

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented Jan 16, 2024

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun Jan 16, 2024

Choose a reason for hiding this comment

dongjoon-hyun commented Jan 16, 2024

cxzl25 commented Jan 17, 2024

dongjoon-hyun commented Jan 17, 2024

cxzl25 commented Jan 11, 2024 •

edited

Loading