SEZ9 commented on PR #12755: URL: https://github.com/apache/seatunnel/pull/12755#issuecomment-6281635228
Thanks for the follow-up on def95cccf — the two points are mostly closed, with a couple of things I'd like to confirm before resolving them: **F2** – Resetting the master-address-to-join before `continue` is the right fix for the pinned-stale-master case. One question on the guard: since it's `if (clearedAny && !clusterService.isJoined())`, the reset is skipped when nothing was cleared in that pass (e.g. every candidate is on a permanent blacklist, or the list was empty). In that situation a master address learned earlier would still stay pinned. Could you either drop the `clearedAny` condition (resetting on every non-joined retry pass should be harmless) or explain why the pinned address is safe to keep there? **F1/F3** – Refreshing the candidate list via `getPossibleAddresses()` when it's empty resolves the "sleeps the whole join window without re-running discovery" part, and the new `discoveryWorkerStartedWithEmptyDiscoveryShouldEventuallyJoin` test is a good addition. Two remaining asks: - Please make that test assert that discovery is actually invoked again (e.g. an invocation counter on the discovery stub), not only that the worker eventually joins — otherwise it wouldn't catch a regression where the join succeeds for an unrelated reason. - F1 also covered the misleading "temporarily blacklisted, will keep retrying" log when the candidates are all *permanently* blacklisted (nothing to retry). Is that message now differentiated, or does the branch still fire with the same text in that case? If it's unchanged, a distinct log line (or skipping the retry branch when no temporary entries exist) would close it. Once those are addressed or clarified I'm happy to resolve F1–F3. <!-- streview-comment:1879 --> -- 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]
