Stephen0421 commented on PR #10062: URL: https://github.com/apache/paimon/pull/10062#issuecomment-5772160329
> Requirement fit: SUPPORTED: Spark users need a procedure for separated changelog cleanup. Implementation: FINDINGS. > > [P1] Please preserve the existing `ExpireChangelogImpl.expireAll(): void` JVM method. Changing its return type to `int` changes the bytecode descriptor, so an already-compiled Flink procedure or external caller that invokes `()V` will fail with `NoSuchMethodError` when paired with the new core jar. Keep the void method and add a separate count-returning method for Spark, or otherwise provide a binary-compatible bridge. > > [P2] `delete_all` reports the inclusive ID-range length as `deleted_changelogs_count`, even when the loop skips a missing changelog or skips one after a file-skipper failure. Please count successful deletions, or label the result as a range size. The current tests only use contiguous IDs. I reviewed the patch and existing core/Flink callers; I did not run the Spark suite locally. Thanks for the review. Both points are addressed. **P1:** Restored `ExpireChangelogImpl.expireAll()` to `void`, so existing `()V` callers (including the Flink procedure) stay compatible. Spark `delete_all` calls a new `expireAllDeletedCount()`. **P2:** `expireAllDeletedCount()` counts changelogs that are actually deleted. A missing changelog, or one skipped after a file-skipper failure, is not included. Added `ChangelogExpireTest#testExpireAllDeletedCountSkipsMissingChangelog` and a Spark procedure test that removes a middle changelog and expects `5` instead of the id-range size `6`. -- 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]
