Skip to content

Conversation

@SergeySereda0218
Copy link
Contributor

Description

Closes #14822
Allow Iceberg writes to use either append or fastAppend. Users in certain use cases may want to do compaction of Manifests async from writes. We should allow configuring fastAppend for that situation on a session level basis.

Additional context and related issues

New IcebergSessionProperty iceberg.merge_manifests_on_write . Default value - "true".
SET SESSION iceberg.merge_manifests_on_write = false to force fastAppend()
SET SESSION iceberg.merge_manifests_on_write = true to force append()

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

Iceberg

  • Allow to do compaction of Manifests asynchronously from writes by configuring property iceberg.merge_manifests_on_write on a session level. ({issue}14822)
# Section

Test_trino_cli
Test_IcebergQueryRunner

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. A few minor comments, otherwise looks good.

Copy link
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Please squash commits into one.

Copy link
Member

@alexjo2144 alexjo2144 left a comment

Choose a reason for hiding this comment

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

Nice, thanks for picking this up

@SergeySereda0218 SergeySereda0218 force-pushed the allow_iceberg_writes_to_use_either_append_or_fastappend branch 6 times, most recently from 63623f0 to 3f5f096 Compare November 26, 2022 20:35
@SergeySereda0218 SergeySereda0218 force-pushed the allow_iceberg_writes_to_use_either_append_or_fastappend branch from 3f5f096 to 995dc42 Compare November 27, 2022 10:41
@SergeySereda0218
Copy link
Contributor Author

Hi.
All code reviews are resolved and all tests are successfully passed.
Is there something else I should do in this PR for merge?

@alexjo2144 alexjo2144 requested a review from ebyhr December 13, 2022 14:39
@ebyhr ebyhr merged commit 5a34f27 into trinodb:master Feb 1, 2023
@github-actions github-actions bot added this to the 407 milestone Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Allow Iceberg writes to use either append or fastAppend

4 participants