SEZ9 commented on PR #10551: URL: https://github.com/apache/seatunnel/pull/10551#issuecomment-6407387892
Thanks for carrying the generation-safety work through the dev update. Before I take another full pass on 633b1e1ba8ba3baef4dfa7377d6cfe0822458c2a, could you help me map the current diff back to the earlier review points? A short pointer to the relevant hunk for each is enough. - F1 / F2: does `repairMissingJobStateForRestore` (and the second restore fence that reads `pendingJobCleanupIMap` by jobId) now validate ownership/generation rather than just presence, so a stale record from an older generation of the same jobId can no longer block or abort restore of the active generation? - F3: is the nested `pendingJobCleanupIMap.lock(jobId)` → `runningJobStateIMap.lock(key)` ordering documented at both sites, and does `DistributedStateTransition` always take them in the same order? - F4 / F6: how does `cleanupPendingJobStateMaps` behave now when a `remove` fails part-way, particularly after `runningJobInfoIMap.remove` has already succeeded? Does it continue past individual failures / reschedule, or is the cleanup-record removal gated on all paired-state deletions succeeding? - F5: is `jobMaster.init(...)` still called while holding the `pendingJobCleanupIMap` lock on the master-switch restore path, or has it moved outside the lock? - F7: when repair returns null and no owned cleanup record exists, what happens to the retained JobInfo (removed, enqueued for cleanup, or left as is), and is that case logged? - F8: is there a test covering the unowned stale-cleanup-record-during-restore scenario (old-generation record present, active generation restores; and no owned record with repair returning null)? Once I have those pointers I'll re-review the hunks directly. Thanks again for the thorough update. <!-- streview-comment:2043 --> -- 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]
