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
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
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
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
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
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
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) {
}
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(
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) {
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) {
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) {
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
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) {
}
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) {
}
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
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
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
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(
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) {
}
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) {
}
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) {
}
}
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
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
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
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(
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())
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(
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(
28 matches
Mail list logo