You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "sunchao (via GitHub)" <gi...@apache.org> on 2026/05/17 02:42:08 UTC
[PR] Fix HA master bootstrap redirect handling [celeborn]
sunchao opened a new pull request, #3691:
URL: https://github.com/apache/celeborn/pull/3691
## Why are the changes needed?
The HA master client can lose useful leader redirects while bootstrapping or fail over poorly when a suggested leader is stale, unavailable, or forms a redirect chain. In those cases, the client can stop short of trying the remaining configured masters even though a usable master is still available.
## What changes were proposed in this PR?
This port brings the HA redirect handling fix from `openai/celeborn#70` onto upstream `main`:
- preserve `MasterNotLeaderException` signals through nested causes
- follow suggested-leader redirects during bootstrap and failover
- fall back to configured masters when redirect setup fails or cycles
- advance the configured-master index correctly when endpoint setup throws
- add focused regression coverage for bootstrap redirects, chained redirects, redirect cycles, and fallback behavior
## How was this PR tested?
- `build/mvn test -pl common -am -Dtest=MasterClientSuiteJ -DwildcardSuites=org.apache.celeborn.common.client.__NoSuchSuite__`
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2330] Fix HA master bootstrap redirect handling [celeborn]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3691:
URL: https://github.com/apache/celeborn/pull/3691#discussion_r3278309596
##########
common/src/main/java/org/apache/celeborn/common/client/MasterClient.java:
##########
@@ -241,7 +285,13 @@ private RpcEndpointRef getOrSetupRpcEndpointRef(AtomicInteger currentIndex) {
if (endpointRef == null) {
int index = currentIndex.get();
do {
- RpcEndpointRef tempEndpointRef = setupEndpointRef(activeMasterEndpoints.get(index));
+ RpcEndpointRef tempEndpointRef;
+ try {
+ tempEndpointRef = setupEndpointRef(activeMasterEndpoints.get(index));
+ } catch (RuntimeException e) {
+ currentIndex.set((index + 1) % activeMasterEndpoints.size());
+ throw e;
+ }
Review Comment:
`getOrSetupRpcEndpointRef` now rethrows `RuntimeException` from `setupEndpointRef` (for bootstrap-time redirect handling), so callers can observe unchecked exceptions from this method. Consider updating the method’s contract/Javadoc (or using a more specific exception type) to make this expected retry-path behavior clearer for future maintainers.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2330] Fix HA master bootstrap redirect handling [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas closed pull request #3691: [CELEBORN-2330] Fix HA master bootstrap redirect handling
URL: https://github.com/apache/celeborn/pull/3691
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2330] Fix HA master bootstrap redirect handling [celeborn]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #3691:
URL: https://github.com/apache/celeborn/pull/3691#discussion_r3285002168
##########
common/src/main/java/org/apache/celeborn/common/client/MasterClient.java:
##########
@@ -241,7 +285,13 @@ private RpcEndpointRef getOrSetupRpcEndpointRef(AtomicInteger currentIndex) {
if (endpointRef == null) {
int index = currentIndex.get();
do {
- RpcEndpointRef tempEndpointRef = setupEndpointRef(activeMasterEndpoints.get(index));
+ RpcEndpointRef tempEndpointRef;
+ try {
+ tempEndpointRef = setupEndpointRef(activeMasterEndpoints.get(index));
+ } catch (RuntimeException e) {
+ currentIndex.set((index + 1) % activeMasterEndpoints.size());
+ throw e;
+ }
Review Comment:
Updated the Java doc of the method
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2330] Fix HA master bootstrap redirect handling [celeborn]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #3691:
URL: https://github.com/apache/celeborn/pull/3691#issuecomment-4536346547
Thanks @SteNicholas and @RexXiong !
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2330] Fix HA master bootstrap redirect handling [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #3691:
URL: https://github.com/apache/celeborn/pull/3691#issuecomment-4531926339
Thanks. Merged to main(v0.7.0).
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2330] Fix HA master bootstrap redirect handling [celeborn]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #3691:
URL: https://github.com/apache/celeborn/pull/3691#discussion_r3295080777
##########
common/src/main/java/org/apache/celeborn/common/client/MasterClient.java:
##########
@@ -171,14 +171,16 @@ private <T> T sendMessageInner(Object message, Class<T> clz) throws Throwable {
private boolean shouldRetry(@Nullable RpcEndpointRef oldRef, Throwable e) {
// It will always throw celeborn exception , so we need to get the cause
// 'CelebornException: Exception thrown in awaitResult'
Review Comment:
Updated
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2330] Fix HA master bootstrap redirect handling [celeborn]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3691:
URL: https://github.com/apache/celeborn/pull/3691#discussion_r3293902120
##########
common/src/main/java/org/apache/celeborn/common/client/MasterClient.java:
##########
@@ -171,14 +171,16 @@ private <T> T sendMessageInner(Object message, Class<T> clz) throws Throwable {
private boolean shouldRetry(@Nullable RpcEndpointRef oldRef, Throwable e) {
// It will always throw celeborn exception , so we need to get the cause
// 'CelebornException: Exception thrown in awaitResult'
Review Comment:
The comment above `shouldRetry` says the client "will always throw celeborn exception" and therefore must inspect `e.getCause()`, but with the new bootstrap redirect behavior `setupEndpointRef` can now surface a `RuntimeException` path as well. Consider updating this comment to reflect that `findMasterNotLeaderException` walks the full cause chain (and why), rather than implying the throwable is always a `CelebornException` wrapper.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org