Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: use ZSTD compressed parquet by default #8158

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .palantir/revapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,11 @@ acceptedBreaks:
- code: "java.method.removed"
old: "method org.apache.iceberg.view.ViewBuilder org.apache.iceberg.view.ViewBuilder::withQueryColumnNames(java.util.List<java.lang.String>)"
justification: "Acceptable break due to updating View APIs and the View Spec"
org.apache.iceberg:iceberg-core:
- code: "java.field.constantValueChanged"
old: "field org.apache.iceberg.TableProperties.PARQUET_COMPRESSION_DEFAULT"
new: "field org.apache.iceberg.TableProperties.PARQUET_COMPRESSION_DEFAULT"
justification: "{Changing the default compression codec from gzip to zstd}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
justification: "{Changing the default compression codec from gzip to zstd}"
justification: "Changing the default compression codec from gzip to zstd"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. Thanks.

apache-iceberg-0.14.0:
org.apache.iceberg:iceberg-api:
- code: "java.class.defaultSerializationChanged"
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/iceberg/TableProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private TableProperties() {}

public static final String PARQUET_COMPRESSION = "write.parquet.compression-codec";
public static final String DELETE_PARQUET_COMPRESSION = "write.delete.parquet.compression-codec";
public static final String PARQUET_COMPRESSION_DEFAULT = "gzip";
public static final String PARQUET_COMPRESSION_DEFAULT = "zstd";

public static final String PARQUET_COMPRESSION_LEVEL = "write.parquet.compression-level";
public static final String DELETE_PARQUET_COMPRESSION_LEVEL =
Expand Down
2 changes: 1 addition & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ Iceberg tables support table properties to configure table behavior, like the de
| write.parquet.page-size-bytes | 1048576 (1 MB) | Parquet page size |
| write.parquet.page-row-limit | 20000 | Parquet page row limit |
| write.parquet.dict-size-bytes | 2097152 (2 MB) | Parquet dictionary page size |
| write.parquet.compression-codec | gzip | Parquet compression codec: zstd, brotli, lz4, gzip, snappy, uncompressed |
| write.parquet.compression-codec | zstd | Parquet compression codec: zstd, brotli, lz4, gzip, snappy, uncompressed |
| write.parquet.compression-level | null | Parquet compression level |
| write.parquet.bloom-filter-enabled.column.col1 | (not set) | Hint to parquet to write a bloom filter for the column: col1 |
| write.parquet.bloom-filter-max-bytes | 1048576 (1 MB) | The maximum number of bytes for a bloom filter bitset |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,27 +219,27 @@ public void testPrimitiveColumns() throws Exception {

Row binaryCol =
Row.of(
59L,
52L,
4L,
2L,
null,
Base64.getDecoder().decode("1111"),
Base64.getDecoder().decode("2222"));
Row booleanCol = Row.of(44L, 4L, 0L, null, false, true);
Row decimalCol = Row.of(97L, 4L, 1L, null, new BigDecimal("1.00"), new BigDecimal("2.00"));
Row doubleCol = Row.of(99L, 4L, 0L, 1L, 1.0D, 2.0D);
Row booleanCol = Row.of(32L, 4L, 0L, null, false, true);
Row decimalCol = Row.of(85L, 4L, 1L, null, new BigDecimal("1.00"), new BigDecimal("2.00"));
Row doubleCol = Row.of(85L, 4L, 0L, 1L, 1.0D, 2.0D);
Row fixedCol =
Row.of(
55L,
44L,
4L,
2L,
null,
Base64.getDecoder().decode("1111"),
Base64.getDecoder().decode("2222"));
Row floatCol = Row.of(90L, 4L, 0L, 2L, 0f, 0f);
Row intCol = Row.of(91L, 4L, 0L, null, 1, 2);
Row longCol = Row.of(91L, 4L, 0L, null, 1L, 2L);
Row stringCol = Row.of(99L, 4L, 0L, null, "1", "2");
Row floatCol = Row.of(71L, 4L, 0L, 2L, 0f, 0f);
Row intCol = Row.of(71L, 4L, 0L, null, 1, 2);
Row longCol = Row.of(79L, 4L, 0L, null, 1L, 2L);
Row stringCol = Row.of(79L, 4L, 0L, null, "1", "2");

List<Row> expected =
Lists.newArrayList(
Expand Down Expand Up @@ -291,7 +291,7 @@ public void testSelectNestedValues() throws Exception {
public void testNestedValues() throws Exception {
createNestedTable();

Row leafDoubleCol = Row.of(53L, 3L, 1L, 1L, 0.0D, 0.0D);
Row leafDoubleCol = Row.of(46L, 3L, 1L, 1L, 0.0D, 0.0D);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szehon-ho @RussellSpitzer do you know what leafDoubleCol is? Is it some kind of metrics that could be changed if the compression codec is changed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I think it include the size, sorry for lack of comment, lets disable zstd in :

    Table table =
        catalog.createTable(
            TableIdentifier.of(Namespace.of(database()), tableName()),
            PRIMITIVE_SCHEMA,
            PartitionSpec.unpartitioned(),
            ImmutableMap.of());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can confirm it's the metric of size. Because of changing to zstd, the sizes are reduced overall. I updated it to the new metrics. We can have a followup PR to change those metric related test with uncompressed parquet.

Copy link
Collaborator

@szehon-ho szehon-ho Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbtsai , i looked into it. I think we have to put it where we write data files, but its a bigger change. There's a test helper we use called FileHelpers.writeDataFile() and we need to change the code to take in map of properties.

Then,

  public static DataFile writeDataFile(Table table, OutputFile out, List<Record> rows, Map<String, String> properties)
      throws IOException {
    FileFormat format = defaultFormat(table.properties());
    GenericAppenderFactory factory = new GenericAppenderFactory(table.schema());
    properties.forEach(factory::set);

but its ok to do in separate pr if you want.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s do it in a followup PR to reduce the changes

Copy link
Collaborator

@szehon-ho szehon-ho Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chat offline with @dbtsai , will make follow up to make expected metrics take from DataFile.fileSizeInBytes()

Row leafLongCol = Row.of(54L, 3L, 1L, null, 0L, 1L);
Row metrics = Row.of(Row.of(leafDoubleCol, leafLongCol));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,27 +219,27 @@ public void testPrimitiveColumns() throws Exception {

Row binaryCol =
Row.of(
59L,
52L,
4L,
2L,
null,
Base64.getDecoder().decode("1111"),
Base64.getDecoder().decode("2222"));
Row booleanCol = Row.of(44L, 4L, 0L, null, false, true);
Row decimalCol = Row.of(97L, 4L, 1L, null, new BigDecimal("1.00"), new BigDecimal("2.00"));
Row doubleCol = Row.of(99L, 4L, 0L, 1L, 1.0D, 2.0D);
Row booleanCol = Row.of(32L, 4L, 0L, null, false, true);
Row decimalCol = Row.of(85L, 4L, 1L, null, new BigDecimal("1.00"), new BigDecimal("2.00"));
Row doubleCol = Row.of(85L, 4L, 0L, 1L, 1.0D, 2.0D);
Row fixedCol =
Row.of(
55L,
44L,
4L,
2L,
null,
Base64.getDecoder().decode("1111"),
Base64.getDecoder().decode("2222"));
Row floatCol = Row.of(90L, 4L, 0L, 2L, 0f, 0f);
Row intCol = Row.of(91L, 4L, 0L, null, 1, 2);
Row longCol = Row.of(91L, 4L, 0L, null, 1L, 2L);
Row stringCol = Row.of(99L, 4L, 0L, null, "1", "2");
Row floatCol = Row.of(71L, 4L, 0L, 2L, 0f, 0f);
Row intCol = Row.of(71L, 4L, 0L, null, 1, 2);
Row longCol = Row.of(79L, 4L, 0L, null, 1L, 2L);
Row stringCol = Row.of(79L, 4L, 0L, null, "1", "2");

List<Row> expected =
Lists.newArrayList(
Expand Down Expand Up @@ -291,7 +291,7 @@ public void testSelectNestedValues() throws Exception {
public void testNestedValues() throws Exception {
createNestedTable();

Row leafDoubleCol = Row.of(53L, 3L, 1L, 1L, 0.0D, 0.0D);
Row leafDoubleCol = Row.of(46L, 3L, 1L, 1L, 0.0D, 0.0D);
Row leafLongCol = Row.of(54L, 3L, 1L, null, 0L, 1L);
Row metrics = Row.of(Row.of(leafDoubleCol, leafLongCol));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,27 +219,27 @@ public void testPrimitiveColumns() throws Exception {

Row binaryCol =
Row.of(
59L,
52L,
4L,
2L,
null,
Base64.getDecoder().decode("1111"),
Base64.getDecoder().decode("2222"));
Row booleanCol = Row.of(44L, 4L, 0L, null, false, true);
Row decimalCol = Row.of(97L, 4L, 1L, null, new BigDecimal("1.00"), new BigDecimal("2.00"));
Row doubleCol = Row.of(99L, 4L, 0L, 1L, 1.0D, 2.0D);
Row booleanCol = Row.of(32L, 4L, 0L, null, false, true);
Row decimalCol = Row.of(85L, 4L, 1L, null, new BigDecimal("1.00"), new BigDecimal("2.00"));
Row doubleCol = Row.of(85L, 4L, 0L, 1L, 1.0D, 2.0D);
Row fixedCol =
Row.of(
55L,
44L,
4L,
2L,
null,
Base64.getDecoder().decode("1111"),
Base64.getDecoder().decode("2222"));
Row floatCol = Row.of(90L, 4L, 0L, 2L, 0f, 0f);
Row intCol = Row.of(91L, 4L, 0L, null, 1, 2);
Row longCol = Row.of(91L, 4L, 0L, null, 1L, 2L);
Row stringCol = Row.of(99L, 4L, 0L, null, "1", "2");
Row floatCol = Row.of(71L, 4L, 0L, 2L, 0f, 0f);
Row intCol = Row.of(71L, 4L, 0L, null, 1, 2);
Row longCol = Row.of(79L, 4L, 0L, null, 1L, 2L);
Row stringCol = Row.of(79L, 4L, 0L, null, "1", "2");

List<Row> expected =
Lists.newArrayList(
Expand Down Expand Up @@ -291,7 +291,7 @@ public void testSelectNestedValues() throws Exception {
public void testNestedValues() throws Exception {
createNestedTable();

Row leafDoubleCol = Row.of(53L, 3L, 1L, 1L, 0.0D, 0.0D);
Row leafDoubleCol = Row.of(46L, 3L, 1L, 1L, 0.0D, 0.0D);
Row leafLongCol = Row.of(54L, 3L, 1L, null, 0L, 1L);
Row metrics = Row.of(Row.of(leafDoubleCol, leafLongCol));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ public void testSelectNestedValues() throws Exception {
public void testNestedValues() throws Exception {
createNestedTable();

Object[] leafDoubleCol = row(53L, 3L, 1L, 1L, 0.0D, 0.0D);
Object[] leafDoubleCol = row(46L, 3L, 1L, 1L, 0.0D, 0.0D);
Object[] leafLongCol = row(54L, 3L, 1L, null, 0L, 1L);
Object[] metrics = row(leafDoubleCol, leafLongCol);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ public void testSelectNestedValues() throws Exception {
public void testNestedValues() throws Exception {
createNestedTable();

Object[] leafDoubleCol = row(53L, 3L, 1L, 1L, 0.0D, 0.0D);
Object[] leafDoubleCol = row(46L, 3L, 1L, 1L, 0.0D, 0.0D);
Object[] leafLongCol = row(54L, 3L, 1L, null, 0L, 1L);
Object[] metrics = row(leafDoubleCol, leafLongCol);

Expand Down