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.

Sign up for GitHub

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

Jump to bottom

Make open writers limit configurable in iceberg connector #17771

Merged
merged 2 commits into from
May 19, 2022
Merged

Make open writers limit configurable in iceberg connector #17771

merged 2 commits into from
May 19, 2022

Conversation

v-jizhang
Copy link
Contributor

@v-jizhang v-jizhang commented May 16, 2022 •
edited by NikhilCollooru
Loading

Cherry-pick of Trino trinodb/trino#6684

Addresses trinodb/trino#6650.

Co-authored-by: Akshay Rai akrai@linkedin.com

== RELEASE NOTES ==

Iceberg Changes
* Add `iceberg.max-partitions-per-writer` configuration property to allow configuring the limit on partitions per writer.

@v-jizhang v-jizhang requested a review from a team as a code owner May 16, 2022 21:28
@v-jizhang v-jizhang requested a review from presto-oss May 16, 2022 21:28
@v-jizhang v-jizhang marked this pull request as draft May 16, 2022 21:28
@v-jizhang v-jizhang marked this pull request as ready for review May 16, 2022 21:58
kewang1024
kewang1024 reviewed May 18, 2022
View reviewed changes
Copy link
Collaborator

@kewang1024 kewang1024 left a comment

Choose a reason for hiding this comment

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

For the second commit, can we improve the commit message to "Add Iceberg configuration properties in docs"

presto-docs/src/main/sphinx/connector/iceberg.rst Show resolved Hide resolved
@v-jizhang v-jizhang force-pushed the iceberg-open-writers-conf branch from 9f35605 to f85079d Compare May 18, 2022 17:57
NikhilCollooru
NikhilCollooru reviewed May 18, 2022
View reviewed changes
Copy link
Contributor

@NikhilCollooru NikhilCollooru left a comment

Choose a reason for hiding this comment

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

Please change the 2nd commit title to "Add Iceberg configuration property to docs"

presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergPageSinkProvider.java Show resolved Hide resolved
@v-jizhang v-jizhang force-pushed the iceberg-open-writers-conf branch from cf75aaa to 760ab72 Compare May 18, 2022 23:24
v-jizhang and others added 2 commits May 18, 2022 16:26
@v-jizhang @akshayrai
Make open writers limit configurable in iceberg connector
bf7a990 
Cherry-pick of Trino trinodb/trino#6684

Co-authored-by: Akshay Rai <akrai@linkedin.com>
@v-jizhang @amitds1997
Add Iceberg configuration property to docs
e8878aa 
Cherry-pick of trinodb/trino#7200

Co-authored-by: Amit Singh <amitds1997@gmail.com>
@v-jizhang v-jizhang force-pushed the iceberg-open-writers-conf branch from 54b2cd9 to e8878aa Compare May 18, 2022 23:27
@v-jizhang
Copy link
Contributor Author

For the second commit, can we improve the commit message to "Add Iceberg configuration properties in docs"

Changed. Thank you.

@NikhilCollooru NikhilCollooru self-assigned this May 19, 2022
@NikhilCollooru NikhilCollooru merged commit 2547605 into prestodb:master May 19, 2022
@highker highker mentioned this pull request Jul 6, 2022
Closed
7 tasks
@Akanksha-kedia
Copy link
Contributor

To wat value we can increase the parameter value ? For hive ? I see page of position count of 4096 is getting processed any relation of this and parameter defined?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kewang1024 kewang1024 kewang1024 approved these changes

@NikhilCollooru NikhilCollooru NikhilCollooru approved these changes

@presto-oss presto-oss Awaiting requested review from presto-oss presto-oss is a code owner automatically assigned from prestodb/committers

Assignees

@NikhilCollooru NikhilCollooru

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@v-jizhang @Akanksha-kedia @NikhilCollooru @kewang1024