SEZ9 commented on PR #11856: URL: https://github.com/apache/seatunnel/pull/11856#issuecomment-5724160863
Thanks for the follow-up. The latest commit `dc2c64103c` (restoring the `TimeUnit` import in `SeaTunnelServerShutdownTest.java`) is a one-line, test-only change that fixes the compile break from `f3308d8168`; no objection to that part. On the earlier findings, here is where I stand: - **F1 (marker published from `ManagedService.shutdown()` while the node is PASSIVE):** the thread now describes the marker being written from a synchronous `LifecycleListener.stateChanged(SHUTTING_DOWN)` callback while the node is still `ACTIVE`, which is the right approach in principle. Could you point me to where the listener is registered and where the IMap write happens, and confirm there is a test that exercises the real Hazelcast lifecycle path rather than only a mocked map call? - **F4 (no eviction on `engine_gracefulMemberRemoval`):** the thread mentions a "TTL-bounded marker". Is that a Hazelcast-side TTL on the map entry, or still a manual timestamp check that depends on cross-node clock agreement? If the former, F4 is resolved; if the latter, it remains open. - **F2 (marker consumed before failure processing / master failover):** the thread says `PhysicalVertex` classifies directly for the master-failover restore path. Is the marker still destructively removed before task-failure processing completes, and how does a failover mid-processing keep the graceful classification? - **F3 / F6 (`JobException` replaced by a plain String in `TaskExecutionState`):** no change described. Are these still as originally reported? If so, I'd like the graceful path to keep the `JobException` (or an equivalent throwable) so the persisted/exposed error payload and job history/REST fields are unchanged. - **F5 / F8 (free-text message matching, template duplicated between `CoordinatorService` and `PhysicalVertex`):** if `PhysicalVertex` now classifies using the marker rather than matching the offline message, please confirm the regex/template duplication has been removed. Otherwise both remain open. - **F7 (docs):** I still don't see documentation for the new IMap and the WARN downgrade behavior under `docs/`. Please add it or tell me where it lives. A short per-finding status would be great, and I'll do a final pass on the current head once I have it. <!-- streview-comment:1125 --> -- 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]
