Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-18 Thread via GitHub
rdblue merged PR #9230: URL: https://github.com/apache/iceberg/pull/9230 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apac

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-18 Thread via GitHub
rdblue commented on PR #9230: URL: https://github.com/apache/iceberg/pull/9230#issuecomment-1861657377 Thanks, @jasonf20! The current version looks great. I'll commit it when tests are passing. For the issues caused by calling `commit` more than once, I think we should keep state and

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-18 Thread via GitHub
jasonf20 commented on PR #9230: URL: https://github.com/apache/iceberg/pull/9230#issuecomment-1861357164 > To which test case are you referring? I didn't think any of them were valid uses of the API because commit was called multiple times and the behavior is undefined. To the test c

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-18 Thread via GitHub
rdblue commented on PR #9230: URL: https://github.com/apache/iceberg/pull/9230#issuecomment-1861257079 > I think this fix is still relevant as the test fails without it. To which test case are you referring? I didn't think any of them were valid uses of the API because commit was call

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-18 Thread via GitHub
jasonf20 commented on PR #9230: URL: https://github.com/apache/iceberg/pull/9230#issuecomment-1859879328 Hi @rdblue. Thanks for info. Good to know about the manifest compaction conflict case. I was looking for a way the list could be partially cleared and this answers that. I

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-17 Thread via GitHub
rdblue commented on PR #9230: URL: https://github.com/apache/iceberg/pull/9230#issuecomment-1859376491 @jasonf20, sorry for being unclear in my reply earlier, but I don't think that the tests in this PR reproduce the error. The tests here reproduce similar errors, but do it by manually call

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-15 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1427260537 ## core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java: ## @@ -895,7 +895,7 @@ private void cleanUncommittedAppends(Set committed) { }

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-14 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1427386841 ## core/src/main/java/org/apache/iceberg/FastAppend.java: ## @@ -187,8 +187,7 @@ protected void cleanUncommitted(Set committed) { deleteFile(manifest.path(

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-14 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1427375881 ## core/src/main/java/org/apache/iceberg/SnapshotProducer.java: ## @@ -487,7 +487,11 @@ protected void cleanAll() { } protected void deleteFile(String path) {

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-14 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1427383977 ## core/src/main/java/org/apache/iceberg/SnapshotProducer.java: ## @@ -487,7 +487,11 @@ protected void cleanAll() { } protected void deleteFile(String path) {

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-14 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1427375881 ## core/src/main/java/org/apache/iceberg/SnapshotProducer.java: ## @@ -487,7 +487,11 @@ protected void cleanAll() { } protected void deleteFile(String path) {

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-14 Thread via GitHub
jasonf20 commented on PR #9230: URL: https://github.com/apache/iceberg/pull/9230#issuecomment-1856811895 Based on further Slack discussions I re-added the FastApply fix. Couple of notes: * I think it’s better to clear the lists instead of removing one item at a time. If the list i

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-14 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1427260537 ## core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java: ## @@ -895,7 +895,7 @@ private void cleanUncommittedAppends(Set committed) { }

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-14 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1427260537 ## core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java: ## @@ -895,7 +895,7 @@ private void cleanUncommittedAppends(Set committed) { }

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1419551515 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapsho

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1419551515 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapsho

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1419551515 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapsho

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
nastra commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1419439406 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapshot(

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1419433713 ## core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java: ## @@ -895,7 +895,7 @@ private void cleanUncommittedAppends(Set committed) { }

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
ConeyLiu commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1419062959 ## core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java: ## @@ -895,7 +895,7 @@ private void cleanUncommittedAppends(Set committed) { }

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
nastra commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1418954500 ## core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java: ## @@ -895,7 +895,7 @@ private void cleanUncommittedAppends(Set committed) { } }

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-07 Thread via GitHub
nastra commented on PR #9230: URL: https://github.com/apache/iceberg/pull/9230#issuecomment-1845262257 @ConeyLiu could you also please take a look at this? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-06 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1417962641 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapsho

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-06 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1417962641 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapsho

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-06 Thread via GitHub
jasonf20 commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1417960440 ## core/src/main/java/org/apache/iceberg/FastAppend.java: ## @@ -187,8 +187,7 @@ protected void cleanUncommitted(Set committed) { deleteFile(manifest.path(

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-06 Thread via GitHub
rdblue commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1417822086 ## core/src/main/java/org/apache/iceberg/FastAppend.java: ## @@ -187,8 +187,7 @@ protected void cleanUncommitted(Set committed) { deleteFile(manifest.path())

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-06 Thread via GitHub
rdblue commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1417820651 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapshot(

Re: [PR] Core: Fixed certain operations failing to add new data files during retries [iceberg]

2023-12-06 Thread via GitHub
rdblue commented on code in PR #9230: URL: https://github.com/apache/iceberg/pull/9230#discussion_r1417819364 ## core/src/test/java/org/apache/iceberg/TestFastAppend.java: ## @@ -313,6 +313,37 @@ public void testRecoveryWithoutManifestList() { metadata.currentSnapshot(