wybaby168 commented on PR #11872: URL: https://github.com/apache/seatunnel/pull/11872#issuecomment-5339695269
Thank you for the careful review and for tracing the actual Java serialization path. You are right: my original explanation incorrectly relied on the `Serializable` constructor to initialize the newly introduced superclass state. The existing null check happened to keep the restore path working, but it was not the invariant the PR claimed. I appreciate you catching that. I have updated the PR in `18a4a3bfa`: - Added `readResolve()` to rebuild a fully initialized legacy state after deserialization, so `assignedSplits` is guaranteed to be non-null and empty. - Kept the replacement as `RabbitmqSourceState` rather than returning the base state, so the enumerator can identify the compatibility path and emit an INFO log. New checkpoints still use `RabbitmqSplitEnumeratorState`. - Strengthened the real 2.3.13 checkpoint fixture test to assert the restored map is non-null and empty before constructing the enumerator. - Added the recommended INFO log for legacy checkpoint restoration. - Corrected the PR description to explain the actual serialization behavior. - Rebased onto the latest `dev`. Local validation with Java 8 passes: the required 16-module reactor build including Spotless succeeds, and all 26 RabbitMQ connector tests pass. The refreshed GitHub workflow is still running; Code style, License header, Helm, and Sanity checks have passed so far. Thank you again for the precise feedback. It exposed a real weakness in both the implementation guarantee and the test, and the PR is better for it. When convenient, could you please take another look? -- 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]
