Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-09-09 Thread via GitHub
amogh-jahagirdar merged PR #10983: URL: https://github.com/apache/iceberg/pull/10983 -- 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...@

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-31 Thread via GitHub
hantangwangd commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1739706862 ## core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java: ## @@ -370,7 +370,7 @@ public void testRetainLastWithExpireById() { } // Retain la

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-30 Thread via GitHub
amogh-jahagirdar commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1739152717 ## core/src/main/java/org/apache/iceberg/RemoveSnapshots.java: ## @@ -321,6 +323,15 @@ ExpireSnapshots withIncrementalCleanup(boolean useIncrementalCleanup)

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-29 Thread via GitHub
hantangwangd commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1737646125 ## core/src/main/java/org/apache/iceberg/RemoveSnapshots.java: ## @@ -116,6 +117,7 @@ public ExpireSnapshots cleanExpiredFiles(boolean clean) { public ExpireS

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-29 Thread via GitHub
amogh-jahagirdar commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1737259547 ## core/src/main/java/org/apache/iceberg/RemoveSnapshots.java: ## @@ -321,6 +323,14 @@ ExpireSnapshots withIncrementalCleanup(boolean useIncrementalCleanup)

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-29 Thread via GitHub
hantangwangd commented on PR #10983: URL: https://github.com/apache/iceberg/pull/10983#issuecomment-2317476054 Following the discussions above, I changed the approach to just ensure that users will not fall into a state where they cleanup files that are still referenced: - If `expir

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-28 Thread via GitHub
hantangwangd commented on PR #10983: URL: https://github.com/apache/iceberg/pull/10983#issuecomment-2316827563 > The incremental cleanup logic is already quite complex and I'm now thinking it's not really worth it to add handling this particular case. There's probably more cases and then if

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-28 Thread via GitHub
hantangwangd commented on PR #10983: URL: https://github.com/apache/iceberg/pull/10983#issuecomment-2316825435 Thank you very much for your guidance @rdblue, it makes sense to avoid users falling into such a state in advance. -- This is an automated message from the Apache Git Service. To

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-28 Thread via GitHub
amogh-jahagirdar commented on PR #10983: URL: https://github.com/apache/iceberg/pull/10983#issuecomment-2316763362 @hantangwangd I thought about this a bit more and while I still think updating the IncrementalFileCleanup to address this particular case is possible, I think the question more

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-28 Thread via GitHub
rdblue commented on PR #10983: URL: https://github.com/apache/iceberg/pull/10983#issuecomment-2316382335 @amogh-jahagirdar, @hantangwangd, I'm not sure that incremental cleanup is doing anything wrong here. Incremental cleanup deletes data files when the snapshot that removed them from the

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-27 Thread via GitHub
RussellSpitzer commented on PR #10983: URL: https://github.com/apache/iceberg/pull/10983#issuecomment-2314425466 So when I wrote the Spark procedure for this we were already aware that this code path has a lot of potential issues. We end up basically completely rewriting the logic of detect

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-27 Thread via GitHub
hantangwangd commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1733882501 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -327,4 +342,34 @@ private Set findFilesToDelete( return filesToDelete; } +

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-27 Thread via GitHub
amogh-jahagirdar commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1733319305 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -327,4 +342,34 @@ private Set findFilesToDelete( return filesToDelete;

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-27 Thread via GitHub
amogh-jahagirdar commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1733319305 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -327,4 +342,34 @@ private Set findFilesToDelete( return filesToDelete;

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-27 Thread via GitHub
amogh-jahagirdar commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1733319305 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -327,4 +342,34 @@ private Set findFilesToDelete( return filesToDelete;

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-27 Thread via GitHub
hantangwangd commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1733280303 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -327,4 +342,34 @@ private Set findFilesToDelete( return filesToDelete; } +

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-27 Thread via GitHub
hantangwangd commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1733265734 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -327,4 +342,34 @@ private Set findFilesToDelete( return filesToDelete; } +

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-26 Thread via GitHub
amogh-jahagirdar commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1731784757 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -327,4 +342,34 @@ private Set findFilesToDelete( return filesToDelete;

Re: [PR] Core: Fix the behavior of IncrementalFileCleanup when expire a snapshot [iceberg]

2024-08-26 Thread via GitHub
amogh-jahagirdar commented on code in PR #10983: URL: https://github.com/apache/iceberg/pull/10983#discussion_r1731788916 ## core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java: ## @@ -61,17 +63,21 @@ public void cleanFiles(TableMetadata beforeExpiration, TableMet