Delete the oldest tracked version metadata files after commit #23813

7hong · 2024-10-17T07:44:04Z

Description

When using the iceberg table, the xxx-metadata.json file is generated each time commit is executed. In the iceberg table, we can automatically clean the previous metadata file through configuration.

write.metadata.delete-after-commit.enabled = true
write.metadata.previous-versions-max = 10
However, the metadata file is not automatically cleaned in trino. A large number of metadata.json files are left on the hdfs.

Different from #20863 ，I have followed the configurations in iceberg (write.metadata.delete-after-commit.enabled and write.metadata.previous-version-max ). Instead of adding a new configuration, this keeps it compatible with iceberg.

Release notes

( x) Release notes are required, with the following suggested text:

## Iceberg connector

* Delete the oldest tracked version metadata files after commit. ({issue} #19582 ).

ebyhr · 2024-10-24T07:48:04Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

Why do we need a new test class? Please use existing test classes as much as possible to avoid redundant bootstrap time.

ebyhr · 2024-10-24T07:49:28Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

Iceberg-connector: Delete the oldest tracked version metadata files after commit

Please follow commit message guideline. https://cbea.ms/git-commit/

Reminder, I see you changed a PR title. Please also update a commit title.

Sorry, I thought the PR's title would be the title of the final merge commit. I will reopen a PR

You don't need to reopen a PR. Please amend the commit title and push it.

ebyhr · 2024-10-24T07:52:22Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

Can we use Sets.difference instead?

ebyhr · 2024-10-24T07:52:50Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

nit: Unwrap and static import METADATA_DELETE_AFTER_COMMIT_ENABLED & METADATA_DELETE_AFTER_COMMIT_ENABLED_DEFAULT.

ebyhr · 2024-10-24T07:53:22Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

nit: Use enhanced instanceof.

ebyhr · 2024-10-24T07:57:43Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

https://trino.io/docs/current/develop/tests.html

Test methods should be defined as package-private.

Same for tearDown

ebyhr · 2024-10-24T07:58:04Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

https://trino.io/docs/current/develop/tests.html#do-not-try-to-manage-memory

ebyhr · 2024-10-24T07:58:24Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

Parquet is the default format. Remove.

ebyhr · 2024-10-24T07:58:51Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

ebyhr · 2024-10-24T07:59:47Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

Are these config properties related to the test purpose? Please remove if it's unrelated.

ebyhr · 2024-10-24T08:06:04Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

trino/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

The file path is wrong. It should be plugin/trino-iceberg/..., remove a leading trino.

ebyhr · 2024-10-24T08:08:27Z

...erg/src/test/java/io/trino/plugin/iceberg/TestIcebergHiveMetastoreAutoCleanMetadataFile.java

We should verify that the oldest metadata file was exactly removed instead of just checking the count.

7hong · 2024-10-24T12:20:37Z

Thank you very much for your patient review. According to your suggestions, I have completed the revisions. Please review it again. Thank you very much. @ebyhr

ebyhr · 2024-10-24T23:20:02Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

Reminder, I see you changed a PR title. Please also update a commit title.

ebyhr · 2024-10-24T23:28:06Z

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergFileOperations.java

No need to put this constant in fields as this is used only from testInsertWithAutoCleanMetadataFile. Please change to a local variable.

ebyhr · 2024-10-24T23:28:55Z

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergFileOperations.java

This test class is for verifying file operation counts. Please move to TestIcebergV2 which has loadTable method or somewhere.

ebyhr · 2024-10-24T23:30:19Z

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergFileOperations.java

Please assert metadata files in this loop.

ebyhr · 2024-10-24T23:31:59Z

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergFileOperations.java

AssertJ provides hasSize method.

ebyhr · 2024-10-24T23:32:47Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

ebyhr · 2024-10-24T23:32:55Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

ebyhr · 2024-10-24T23:34:18Z

...no-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractIcebergTableOperations.java

Rename f to file.

Single file without a delete in a partition can't be optimized any further

When a memory input file is created for avro, rc, and line readers, we need to update the length that will be passed in to the reader since the length of the memory input file can possibly be less than the original input file length.

While HTTP/2 was supported before, now it is used by default with CLI as well as the JDBC driver.

Previous logic was setting all columns as non-nullable which is not valid when the data contains nulls

cla-bot bot added the cla-signed label Oct 17, 2024

github-actions bot added the iceberg Iceberg connector label Oct 17, 2024

7hong changed the title ~~Iceberg-connector: Delete the oldest tracked version metadata files a…~~ Iceberg-connector: Delete the oldest tracked version metadata files after commit Oct 17, 2024

7hong requested review from akenO8, alexjo2144, bitsondatadev, colebow, findepi, findinpath, krvikash, martint, mosabua, raunaqmorarka and wendigo October 17, 2024 08:07

mosabua removed their request for review October 18, 2024 20:13

ebyhr removed request for akenO8, bitsondatadev, colebow, martint and wendigo October 24, 2024 07:47

ebyhr reviewed Oct 24, 2024

View reviewed changes

7hong force-pushed the dev branch from c31fd04 to cdf9960 Compare October 24, 2024 12:10

7hong changed the title ~~Iceberg-connector: Delete the oldest tracked version metadata files after commit~~ Delete the oldest tracked version metadata files after commit Oct 24, 2024

ebyhr reviewed Oct 24, 2024

View reviewed changes

7hong added 3 commits October 28, 2024 17:56

optimize code

3a2531b

Use static import

a737f01

Remove count from the test

b76bd90

Jan Waś and others added 22 commits October 28, 2024 17:56

Add the Faker connector

2bd68ba

Remove unused FakerInsertTableHandle

7fda026

Delete the oldest tracked version metadata files after commit

439a2cf

Add the Faker connector

7b47cf6

Make the faker connector generate multiple splits per node

3a57b40

Add note about removed *.http-client.max-connections properties

97f94cf

Improve docs for at_timezone

800bf12

Use hive.metastore.partition-batch-size.max in sync_partition_metadata

c2fd58c

Rename Ssl to Tls in MongoDB connector

44a8f3e

Run testDeleteRowsConcurrently sequentially for better reliability

3448701

Pass in requestPayer for ListObjectsV2Request

54a2a81

Extract StructLikeWrapperWithFieldIdToIndex to separate class

6d64869

Add StructLikeWrapperWithFieldIdToIndex#createStructLikeWrapper

0de86ea

Skip single file in a partition from OPTIMIZE

93e06c0

Single file without a delete in a partition can't be optimized any further

Update length if memory input file is created

f08908a

When a memory input file is created for avro, rc, and line readers, we need to update the length that will be passed in to the reader since the length of the memory input file can possibly be less than the original input file length.

Add 463 release notes entries

d067aa4

While HTTP/2 was supported before, now it is used by default with CLI as well as the JDBC driver.

Move testing metadata manager to test classpath

28c9109

Ignore exceptions when computing stats after query finished

b6baab5

Re-enable Iceberg JDBC testDeleteRowsConcurrently

43ae63b

Fix iceberg migrate to use nullable columns

6211c64

Previous logic was setting all columns as non-nullable which is not valid when the data contains nulls

Delete the oldest tracked version metadata files after commit

de260a5

Delete the oldest tracked version metadata files after commit

212047f

7hong force-pushed the dev branch from eaf3391 to 212047f Compare October 28, 2024 10:56

github-actions bot added release-notes docs hive Hive connector labels Oct 28, 2024

7hong closed this Oct 28, 2024

7hong deleted the dev branch October 28, 2024 10:58

7hong mentioned this pull request Oct 28, 2024

Delete the oldest tracked version metadata files after commit #23940

Closed

Uh oh!

Delete the oldest tracked version metadata files after commit #23813

Delete the oldest tracked version metadata files after commit #23813

Uh oh!

Conversation

7hong commented Oct 17, 2024 • edited by mosabua Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release notes

Uh oh!

ebyhr Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

7hong commented Oct 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

8 participants

7hong commented Oct 17, 2024 •

edited by mosabua

Loading

ebyhr Oct 24, 2024 •

edited

Loading