SEZ9 commented on PR #11856: URL: https://github.com/apache/seatunnel/pull/11856#issuecomment-5807035361
Thanks for re-tracing the diff and runtime path against `dc2c64103c` rather than only extending the earlier conclusions. One note: the comment as it appears on the thread ends mid-sentence at "the TTL/clock-skew rule", so nothing after section 1.1 (including the "Issue 4" you refer to) is visible here. If it is a rendering artifact again, a short inline restatement would be enough. Going point by point against the open findings: **F1 (marker published during shutdown while the node is already PASSIVE)** — moving the publish into `stateChanged()` on `SHUTTING_DOWN`, with the stale-marker clear on `STARTED`, is the right shape and addresses the core concern. Two remaining asks: (1) please make sure the code comment about the `SHUTTING_DOWN`/Hazelcast-version dependency lands in the diff, since the feature relies on IMap writes still being accepted in that state; (2) the class-level `Tests run: 10, Failures: 0, Errors: 0` is good, but please paste or link the per-method line for the new graceful-shutdown test so we can see it ran rather than being skipped. **F2 (destructive consume before processing / failover loses classification)** — reading the marker once per removal event and sharing one boolean across affected tasks helps consistency, but it doesn't tell me whether the entry is still removed before task-failure processing completes. Please confirm whether the marker is now read non-destructively and left to expire, or describe how a master failover mid-processing keeps the graceful classification. **F4 (no eviction on `engine_gracefulMemberRemoval`; manual TTL depends on clock agreement)** — the relevant text is not visible on this thread. Please restate briefly: is expiry enforced by map-level TTL/eviction configuration, or still by a manual timestamp comparison on the reader side? **F3 / F6 (JobException replaced by a bare String in the task state and in the persisted/REST error payload)** — not addressed in the visible portion. Please confirm whether the graceful path still carries a throwable with the original stack trace, or whether the "state handling unchanged" claim in the description needs revising. **F5 / F8 (free-text message matching for the WARN downgrade; template defined twice)** — also not covered in what's visible. If classification is now derived from the marker lookup rather than from matching message text, please say so and point to where the two definitions were collapsed into one. **F7 (docs)** — please confirm whether a `docs/` entry describing the marker map and the WARN downgrade of node-offline failures is part of the current head. Once the missing sections are re-posted or answered inline, I can close out the remaining items. <!-- streview-comment:1283 --> -- 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]
