DanielLeens commented on PR #10808: URL: https://github.com/apache/seatunnel/pull/10808#issuecomment-5805500770
Good follow-up questions, SEZ9 - I checked `ServerExecuteCommand.java` on `0d4d8624c` directly rather than going by description, so here is the source-level answer to both remaining items: **F1/F2 (fallback vs. server-confirmed election):** The "first non-lite member" fallback in `getActiveMasterAddress()` (`ServerExecuteCommand.java:182-194`) is not a client-side guess made independently of the server. It only fires when Hazelcast has already reported a master (`masterMember != null`) and that master happens to be a worker-only (lite) member - i.e. the server-confirmed mastership sits on a node that is not coordinator-capable, so the client infers which non-lite member is actually coordinating. When the server has not confirmed any master at all (`masterMember == null`), the method returns `null` immediately at line 183-184 and no fallback is attempted at all - covered by `testUnknownMasterDoesNotSelectFallbackCoordinator` (`ServerExecuteCommandTest.java:104-117`). The "why this is acceptable" explanation you asked for is already written down as a code comment: the Javadoc on `describeActiveMasterResolution()` (lines 196-209) states the label is "best effort" and " can lag behind the cluster during failover", and that exact wording is what gets printed to the operator (`... (best effort). Hazelcast master ... is a worker-only member, so the coordinator is inferred on the client ...`), covered by `testInferredCoordinatorIsMarkedBestEffort` (lines 138-160). **F6 (unresolved coordinator):** Confirmed. When no coordinator can be resolved (`activeMasterAddress == null`), `getRole()` (lines 225-235) still prints every non-lite member's row as plain `MASTER` (the configured role), not `ACTIVE MASTER`, because the `masterAddress != null && ...` guard at line 230 is false. What changes is that `describeActiveMasterResolution()` prints an explicit `Active master: UNKNOWN. No coordinator-capable member can be resolved from the current membership view, so MASTER rows only show the configured role.` note directly below the table (lines 211-213), so operators are not left reading plain `MASTER` rows as if an election had already completed. This exact case is unit-tested in `testUnknownCoordinatorIsReportedExplicitly` (lines 123-132), which asserts the note is non-null and contains `UNKNOWN`. Both check out from source and tests, so F1/F2/F6 are resolved on my side too, on top of the F3/F4/F5/F8 pointers from my previous comment. On the CI/merge-conflict follow-up you flagged: I just re-checked live status and it is still open - `mergeStateStatus` is `DIRTY` / `mergeable` is `CONFLICTING`, and the `Build` check on `0d4d8624c` is still `FAILURE`. So the rebase-and-rerun-CI step is still outstanding; I'd hold off on final sign-off until @davidzollo syncs with `dev` and we see a green run on the synced head. -- 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]
