SEZ9 commented on PR #10808: URL: https://github.com/apache/seatunnel/pull/10808#issuecomment-5738716801
Thanks for the follow-up commit `0d4d8624c`. Based on the latest automated review of the head, the new `describeActiveMasterResolution(masterMember, activeMasterAddress)` in `ServerExecuteCommand.java` looks like the right shape for the CLI concerns I raised: - It directly addresses the "silent MASTER for every non-lite member" problem: printing `Active master: UNKNOWN...` when no coordinator-capable member is visible makes the ambiguous state explicit instead of leaving the user to guess. - Labelling the inferred address as `(best effort)` when the Hazelcast master is a lite member is a good way to be honest about the client-side fallback versus the server-side election. I'm fine with keeping the fallback as long as the output makes clear it is not authoritative, which this does. - Keeping the default output unchanged when the Hazelcast master is already coordinator-capable avoids noise for the common case. A few things I'd still like to confirm from your side, since I only have the summary of the diff in the thread and not the full change: 1. `docs/en/introduction/concepts/incompatible-changes.md`: please confirm the entry now (a) calls out the CLI member-list "ACTIVE MASTER" semantics change in separated clusters, and (b) describes the standby/failover window in which no node reports as the active coordinator (so the `UNKNOWN` CLI output is documented, not surprising). 2. `docs/en/engines/zeta/telemetry.md`: does the "bounded timeout" for the final cancel-time metrics flush now state the actual value and whether/how it is configurable? If the timeout is hard-coded, please just state the number. 3. `docs/en/engines/zeta/rest-api-v1.md`: a one-line note next to the new `nodeRole`/`coordinator`/`worker` fields reminding operators that the cluster-health endpoint is unauthenticated by default and now exposes topology would be enough to close that point. 4. `ServerExecuteCommandTest.java`: please replace the inline fully-qualified `java.util.Collections` with an import — trivial, but it's the only style nit left from my earlier pass. If all four are already covered in `0d4d8624c`, a short pointer to the relevant hunks is all I need and I'll re-check and approve. Thanks for the thorough work on the routing chain. <!-- streview-comment:1156 --> -- 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]
