Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: priorityLB reset failover timer when connecting if seen ready or idle … #9093

Merged
merged 1 commit into from Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 26 additions & 20 deletions xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java
Expand Up @@ -188,7 +188,8 @@ private final class ChildLbState {
final GracefulSwitchLoadBalancer lb;
// Timer to fail over to the next priority if not connected in 10 sec. Scheduled only once at
// child initialization.
final ScheduledHandle failOverTimer;
ScheduledHandle failOverTimer;
boolean seenReadyOrIdleSinceTransientFailure = false;
// Timer to delay shutdown and deletion of the priority. Scheduled whenever the child is
// deactivated.
@Nullable ScheduledHandle deletionTimer;
Expand All @@ -200,24 +201,23 @@ private final class ChildLbState {
this.priority = priority;
childHelper = new ChildHelper(ignoreReresolution);
lb = new GracefulSwitchLoadBalancer(childHelper);
failOverTimer = syncContext.schedule(new FailOverTask(), 10, TimeUnit.SECONDS, executor);
logger.log(XdsLogLevel.DEBUG, "Priority created: {0}", priority);
}

class FailOverTask implements Runnable {
@Override
public void run() {
if (deletionTimer != null && deletionTimer.isPending()) {
// The child is deactivated.
return;
}
picker = new ErrorPicker(
Status.UNAVAILABLE.withDescription("Connection timeout for priority " + priority));
logger.log(XdsLogLevel.DEBUG, "Priority {0} failed over to next", priority);
currentPriority = null; // reset currentPriority to guarantee failover happen
tryNextPriority(true);
final class FailOverTask implements Runnable {
@Override
public void run() {
if (deletionTimer != null && deletionTimer.isPending()) {
// The child is deactivated.
return;
}
picker = new ErrorPicker(
Status.UNAVAILABLE.withDescription("Connection timeout for priority " + priority));
logger.log(XdsLogLevel.DEBUG, "Priority {0} failed over to next", priority);
currentPriority = null; // reset currentPriority to guarantee failover happen
tryNextPriority(true);
}

failOverTimer = syncContext.schedule(new FailOverTask(), 10, TimeUnit.SECONDS, executor);
logger.log(XdsLogLevel.DEBUG, "Priority created: {0}", priority);
}

/**
Expand Down Expand Up @@ -312,11 +312,17 @@ public void run() {
if (deletionTimer != null && deletionTimer.isPending()) {
return;
}
if (failOverTimer.isPending()) {
if (newState.equals(READY) || newState.equals(IDLE)
|| newState.equals(TRANSIENT_FAILURE)) {
failOverTimer.cancel();
if (newState.equals(CONNECTING) ) {
if (!failOverTimer.isPending() && seenReadyOrIdleSinceTransientFailure) {
failOverTimer = syncContext.schedule(new FailOverTask(), 10, TimeUnit.SECONDS,
executor);
}
} else if (newState.equals(READY) || newState.equals(IDLE)) {
seenReadyOrIdleSinceTransientFailure = true;
failOverTimer.cancel();
} else if (newState.equals(TRANSIENT_FAILURE)) {
seenReadyOrIdleSinceTransientFailure = false;
failOverTimer.cancel();
}
tryNextPriority(true);
}
Expand Down
38 changes: 38 additions & 0 deletions xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java
Expand Up @@ -435,6 +435,44 @@ public void idleToConnectingDoesNotTriggerFailOver() {
assertThat(fooHelpers).hasSize(1);
}

@Test
public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() {
PriorityChildConfig priorityChildConfig0 =
new PriorityChildConfig(new PolicySelection(fooLbProvider, new Object()), true);
PriorityChildConfig priorityChildConfig1 =
new PriorityChildConfig(new PolicySelection(fooLbProvider, new Object()), true);
PriorityLbConfig priorityLbConfig =
new PriorityLbConfig(
ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1),
ImmutableList.of("p0", "p1"));
priorityLb.handleResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(priorityLbConfig)
.build());
assertThat(fooBalancers).hasSize(1);
assertThat(fooHelpers).hasSize(1);
Helper helper0 = Iterables.getOnlyElement(fooHelpers);

// p0 gets IDLE.
helper0.updateBalancingState(
IDLE,
BUFFER_PICKER);
assertCurrentPickerIsBufferPicker();

// p0 goes to CONNECTING, reset failover timer
fakeClock.forwardTime(5, TimeUnit.SECONDS);
helper0.updateBalancingState(
CONNECTING,
BUFFER_PICKER);
verify(helper, times(2)).updateBalancingState(eq(CONNECTING), eq(BUFFER_PICKER));

// failover happens
fakeClock.forwardTime(10, TimeUnit.SECONDS);
assertThat(fooBalancers).hasSize(2);
assertThat(fooHelpers).hasSize(2);
}

@Test
public void readyToConnectDoesNotFailOverButUpdatesPicker() {
PriorityChildConfig priorityChildConfig0 =
Expand Down