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

Patch upstream PR #3717 and its dependencies with some refactor #116

Merged
merged 4 commits into from Jul 25, 2022

Conversation

rzhang10
Copy link
Member

@rzhang10 rzhang10 commented Jul 18, 2022

This PR patches apache/iceberg#3717 and its dependencies to make it work in LI-Iceberg 0.11. It will address our production issues where a commit succeeds but its metadata file is deleted (because Iceberg wrongly thinks it fails) and the table is corrupted.

A more detailed explanation could be found in the description of the upstream PR.

kbendick and others added 2 commits July 18, 2022 13:19
…s and enforce tableName is implemented at compile time (#2630)

(cherry picked from commit df93538)
Copy link
Contributor

@yiqiangin yiqiangin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -36,9 +36,18 @@ private TableProperties() {
public static final String COMMIT_TOTAL_RETRY_TIME_MS = "commit.retry.total-timeout-ms";
public static final int COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT = 1800000; // 30 minutes

public static final String COMMIT_NUM_STATUS_CHECKS = "commit.num-status-checks";
public static final String COMMIT_NUM_STATUS_CHECKS = "commit.status-check.num-retries";
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: renaming it as something like "COMMIT_NUM_RETRIES_CHECKS", as the value of the string is changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

The name is directly from OSS so I feel it's better to keep it aligned.

@rzhang10 rzhang10 merged commit 6a170b9 into linkedin:li-0.11.x Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants