SEZ9 commented on PR #10808:
URL: https://github.com/apache/seatunnel/pull/10808#issuecomment-5409355605

   Thanks @DanielLeens — and thanks for re-posting the review body verbatim. 
The rendering explanation makes sense; with the diff and full narrative in 
hand, your bootstrap analysis reads clearly: since `LiteNodeDropOutTcpIpJoiner` 
forbids a lite node from self-promoting, starting `secondServer` alone first 
means the cluster can never form until the non-lite `server` arrives, and the 
logs you describe (`Members {size:1}` never updating, endless `active master 
not yet known` retries from `NodeEngineUtil.getActiveMasterAddressOrThrow`) are 
consistent with that. Agreed this is a genuine bootstrap gap exposed by the 
reorder, not a routing/metrics side effect.
   
   One note: your comment appears to cut off mid-sentence right where you start 
addressing the earlier-round follow-ups ("...when the active coordi"). Could 
you re-post the rest? In the meantime, here are the concrete asks still open 
from that round:
   
   1. **CLI fallback selection** (`ServerExecuteCommand.java`): the "first 
non-lite member" heuristic can disagree with the server's actual active 
coordinator. Please either query the server-side election result or clearly 
document/annotate the fallback as a best-effort guess.
   2. **Unknown-coordinator output**: when no active coordinator resolves, the 
member list silently labels every non-lite member `MASTER`. Please add an 
explicit "coordinator unknown" indication instead.
   3. **incompatible-changes.md**: add the member-list "ACTIVE MASTER" 
semantics change for separated clusters, and state the standby/failover window 
during which no node reports as active coordinator.
   4. **telemetry.md**: state the actual value of the "bounded timeout" for the 
final cancel-time metrics flush and whether it is configurable.
   5. **rest-api-v1.md**: a short note that the new 
`nodeRole`/`coordinator`/`worker` fields expand topology disclosure on the 
default-unauthenticated cluster-health endpoint would be helpful.
   6. **Minor**: `ServerExecuteCommandTest.java` uses fully-qualified 
`java.util.Collections` inline — please switch to an import.
   
   Once you've re-posted the truncated portion and addressed (or pushed back 
on) the above, I'm happy to take another pass.
   
   <!-- streview-comment:553 -->


-- 
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]

Reply via email to