cshuo commented on PR #19731: URL: https://github.com/apache/hudi/pull/19731#issuecomment-5538175372
> Thanks for taking this on -- removing the "write an empty commit just to trigger MDT maintenance" workaround is a real operational win, and the request-driven refactor is a sensible shape for it. The change is additive at the storage and API layer (no timeline/MDT/log-format changes, no config renames or removals), and inline behavior stays the default. Most of my comments are about behavior deltas introduced by the refactor rather than the feature itself. Thks for the detailed review. For 1, I updated the v6 guard so delegated compaction scheduling prevents falling through to log compaction. Compaction not requested remains an exception: logcompaction-only requests are supported, subject to the existing timeline validation. Added regression coverage for both cases and the external delegation bypass. For 2, I removed the completed-deltacommit dependency for archive and v8+ clean in the explicit APIs, while preserving the existing inline behavior. V6 clean and compaction/log-compaction scheduling still require a completed deltacommit and now log a WARN when skipped. Added regression coverage for both versions. For 3, I kept validation in copy(mode): a valid request is not necessarily valid after changing its mode. An instant-bearing request is execution-only, so switching it to a scheduling mode should fail rather than silently discard the instant or bypass validation. Documented this contract and added rejection tests, including rejection before any table-service work begins. For 4, I kept the existing execution-delegation key: clean/archive are opt-in additions, and existing valid configurations retain their behavior. The tool executes through MDT writer APIs directly and does not depend on an HTTP Table Service Manager server. Documented that delegating clean/archive skips their inline execution without automatically submitting or launching a replacement job; users must arrange external execution. Also extracted the shared archive action constant. For 5 ~ 10, fixed the configuration mapping, shared parsing, null guard, skipped-service warnings, and exception handling, and added the requested test coverage; all 128 targeted tests pass locally. @nsivabalan Please take another look when you have a chance. -- 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]
