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<ManifestFile> committed) { } } - this.cachedNewDataManifests = committedNewDataManifests; + this.cachedNewDataManifests = null; Review Comment: @nastra Wouldn't it be safer to set the list to empty at the end of this function? If this list isn't emptied it can lead to the same bug since it will think it doesn't need to produce manifests in the `newDataFilesAsManifests` method. I don't think this is an issue right now because I expect all commits to fail or succeed so all files to be deleted, but maybe for safety we should just clear it? @ConeyLiu If I understand correctly the files are only cleared once the transaction/operation fully fails so it should clear everything at the end right? -- 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.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org