danny0405 commented on issue #20479: URL: https://github.com/apache/hudi/issues/20479#issuecomment-6515308149
I traced the relevant code paths and checked the current `master` sources. This appears to be a valid savepoint validation bug. I have not independently rerun the Spark reproduction, so the findings below are based on code inspection. There are two separate failures: 1. **Completed clean: the empty string bypasses the fallback.** `KEEP_LATEST_FILE_VERSIONS` does not populate an earliest retained instant. In `CleanActionExecutor`, a missing action instant is explicitly converted to `earliestCommitToRetain = ""`. `SavepointActionExecutor.getLastCommitRetained()` returns that string through `Option.map`, which treats it as present, so the HUDI-9675 fallback is never evaluated. The subsequent timestamp comparison against `""` accepts a normal commit timestamp even if the files needed for that snapshot have already been cleaned. 2. **Requested/inflight clean: the missing action instant is dereferenced.** The pending-clean branch calls `getEarliestInstantToRetain().getTimestamp()` without a null check. A file-version cleaner plan can therefore cause an NPE before the fallback executes, including when attempting to savepoint the latest completed commit. Relevant code: - [Completed-clean metadata construction](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanActionExecutor.java#L180-L185) - [Savepoint retention validation and fallback](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/savepoint/SavepointActionExecutor.java#L143-L170) The reported restore outcome is consistent with the code: savepoint creation collects files still visible at or before the target instant without verifying that the historical snapshot is complete. For a COW file group rewritten at `c1 -> c2 -> c3`, cleaning down to one version can leave only the `c3` file. An incorrectly accepted savepoint at `c2` cannot preserve its already-deleted file; restoring then rolls back `c3` and removes the remaining file. Depending on the file layout, this can produce partial data loss or an empty table. The existing fallback test constructs `HoodieCleanMetadata` with an unset/null retention field, so it misses the empty-string value produced by actual cleaning: [test fixture](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/action/savepoint/TestSavepointActionExecutor.java#L114-L130). A focused fix would normalize both null and empty retention timestamps to an absent value, safely extract the optional instant from pending cleaner plans, and apply the existing conservative fallback. Regression coverage should include completed-clean metadata with both null and empty boundaries, requested/inflight plans with a null retention instant, and a COW integration case verifying that savepoint creation rejects a cleaned historical snapshot while still allowing a valid latest snapshot. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
