flasn4nt
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

core: Remove RetryingNR.RESOLUTION_RESULT_LISTENER_KEY 

It was introduced in fcb5c54e4 because at the time we didn't change the
API to communicate the status. When onResult2() was introduced in
90d0fabb1 this hack stopped being necessary.
This commit is contained in:
Eric Anderson 2025-06-12 11:47:05 -07:00
parent 240f731e00
commit d88ef97a87
4 changed files with 6 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
View File

@ -94,7 +94,6 @@ import io.grpc.internal.ManagedChannelServiceConfig.MethodInfo;
import io.grpc.internal.ManagedChannelServiceConfig.ServiceConfigConvertedSelector;
import io.grpc.internal.RetriableStream.ChannelBufferMeter;
import io.grpc.internal.RetriableStream.Throttle;
import io.grpc.internal.RetryingNameResolver.ResolutionResultListener;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
@ -1653,18 +1652,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
@Override
public void onResult(final ResolutionResult resolutionResult) {
final class NamesResolved implements Runnable {
@Override
public void run() {
Status status = onResult2(resolutionResult);
ResolutionResultListener resolutionResultListener = resolutionResult.getAttributes()
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
resolutionResultListener.resolutionAttempted(status);
}
}
syncContext.execute(new NamesResolved());
syncContext.execute(() -> onResult2(resolutionResult));
}
@SuppressWarnings("ReferenceEquality")

32
core/src/main/java/io/grpc/internal/RetryingNameResolver.java
View File

@ -17,7 +17,6 @@
package io.grpc.internal;
import com.google.common.annotations.VisibleForTesting;
import io.grpc.Attributes;
import io.grpc.NameResolver;
import io.grpc.Status;
import io.grpc.SynchronizationContext;
@ -34,9 +33,6 @@ final class RetryingNameResolver extends ForwardingNameResolver {
private final RetryScheduler retryScheduler;
private final SynchronizationContext syncContext;
static final Attributes.Key<ResolutionResultListener> RESOLUTION_RESULT_LISTENER_KEY
= Attributes.Key.create(
"io.grpc.internal.RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY");
/**
* Creates a new {@link RetryingNameResolver}.
@ -88,18 +84,7 @@ final class RetryingNameResolver extends ForwardingNameResolver {
@Override
public void onResult(ResolutionResult resolutionResult) {
// If the resolution result listener is already an attribute it indicates that a name resolver
// has already been wrapped with this class. This indicates a misconfiguration.
if (resolutionResult.getAttributes().get(RESOLUTION_RESULT_LISTENER_KEY) != null) {
throw new IllegalStateException(
"RetryingNameResolver can only be used once to wrap a NameResolver");
}
// To have retry behavior for name resolvers that haven't migrated to onResult2.
delegateListener.onResult(resolutionResult.toBuilder().setAttributes(
resolutionResult.getAttributes().toBuilder()
.set(RESOLUTION_RESULT_LISTENER_KEY, new ResolutionResultListener()).build())
.build());
syncContext.execute(() -> onResult2(resolutionResult));
}
@Override
@ -119,19 +104,4 @@ final class RetryingNameResolver extends ForwardingNameResolver {
syncContext.execute(() -> retryScheduler.schedule(new DelayedNameResolverRefresh()));
}
}
/**
* Simple callback class to store in {@link ResolutionResult} attributes so that
* ManagedChannel can indicate if the resolved addresses were accepted. Temporary until
* the Listener2.onResult() API can be changed to return a boolean for this purpose.
*/
class ResolutionResultListener {
public void resolutionAttempted(Status successStatus) {
if (successStatus.isOk()) {
retryScheduler.reset();
} else {
retryScheduler.schedule(new DelayedNameResolverRefresh());
}
}
}
}

4
core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
View File

@ -3844,8 +3844,6 @@ public class ManagedChannelImplTest {
verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture());
ResolvedAddresses resolvedAddresses = resolvedAddressCaptor.getValue();
assertThat(resolvedAddresses.getAddresses()).isEqualTo(nameResolverFactory.servers);
assertThat(resolvedAddresses.getAttributes()
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY)).isNotNull();
// simulating request connection and then transport ready after resolved address
Subchannel subchannel =
@ -3951,8 +3949,6 @@ public class ManagedChannelImplTest {
verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture());
ResolvedAddresses resolvedAddresses = resolvedAddressCaptor.getValue();
assertThat(resolvedAddresses.getAddresses()).isEqualTo(nameResolverFactory.servers);
assertThat(resolvedAddresses.getAttributes()
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY)).isNotNull();
// simulating request connection and then transport ready after resolved address
Subchannel subchannel =

43
core/src/test/java/io/grpc/internal/RetryingNameResolverTest.java
View File

@ -17,7 +17,6 @@
package io.grpc.internal;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@ -28,7 +27,6 @@ import io.grpc.NameResolver.Listener2;
import io.grpc.NameResolver.ResolutionResult;
import io.grpc.Status;
import io.grpc.SynchronizationContext;
import io.grpc.internal.RetryingNameResolver.ResolutionResultListener;
import java.lang.Thread.UncaughtExceptionHandler;
import org.junit.Before;
import org.junit.Rule;
@ -58,8 +56,6 @@ public class RetryingNameResolverTest {
private RetryScheduler mockRetryScheduler;
@Captor
private ArgumentCaptor<Listener2> listenerCaptor;
@Captor
private ArgumentCaptor<ResolutionResult> onResultCaptor;
private final SynchronizationContext syncContext = new SynchronizationContext(
mock(UncaughtExceptionHandler.class));
@ -77,21 +73,14 @@ public class RetryingNameResolverTest {
retryingNameResolver.shutdown();
}
// Make sure the ResolutionResultListener callback is added to the ResolutionResult attributes,
// and the retry scheduler is reset since the name resolution was successful.
@Test
public void onResult_success() {
when(mockListener.onResult2(isA(ResolutionResult.class))).thenReturn(Status.OK);
retryingNameResolver.start(mockListener);
verify(mockNameResolver).start(listenerCaptor.capture());
listenerCaptor.getValue().onResult(ResolutionResult.newBuilder().build());
verify(mockListener).onResult(onResultCaptor.capture());
ResolutionResultListener resolutionResultListener = onResultCaptor.getValue()
.getAttributes()
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
assertThat(resolutionResultListener).isNotNull();
resolutionResultListener.resolutionAttempted(Status.OK);
verify(mockRetryScheduler).reset();
}
@ -107,21 +96,15 @@ public class RetryingNameResolverTest {
verify(mockRetryScheduler).reset();
}
// Make sure the ResolutionResultListener callback is added to the ResolutionResult attributes,
// and that a retry gets scheduled when the resolution results are rejected.
// Make sure that a retry gets scheduled when the resolution results are rejected.
@Test
public void onResult_failure() {
when(mockListener.onResult2(isA(ResolutionResult.class))).thenReturn(Status.UNAVAILABLE);
retryingNameResolver.start(mockListener);
verify(mockNameResolver).start(listenerCaptor.capture());
listenerCaptor.getValue().onResult(ResolutionResult.newBuilder().build());
verify(mockListener).onResult(onResultCaptor.capture());
ResolutionResultListener resolutionResultListener = onResultCaptor.getValue()
.getAttributes()
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
assertThat(resolutionResultListener).isNotNull();
resolutionResultListener.resolutionAttempted(Status.UNAVAILABLE);
verify(mockRetryScheduler).schedule(isA(Runnable.class));
}
@ -138,24 +121,6 @@ public class RetryingNameResolverTest {
verify(mockRetryScheduler).schedule(isA(Runnable.class));
}
// Wrapping a NameResolver more than once is a misconfiguration.
@Test
public void onResult_failure_doubleWrapped() {
NameResolver doubleWrappedResolver = new RetryingNameResolver(retryingNameResolver,
mockRetryScheduler, syncContext);
doubleWrappedResolver.start(mockListener);
verify(mockNameResolver).start(listenerCaptor.capture());
try {
listenerCaptor.getValue().onResult(ResolutionResult.newBuilder().build());
} catch (IllegalStateException e) {
assertThat(e).hasMessageThat().contains("can only be used once");
return;
}
fail("An exception should have been thrown for a double wrapped NAmeResolver");
}
// A retry should get scheduled when name resolution fails.
@Test
public void onError() {
@ -165,4 +130,4 @@ public class RetryingNameResolverTest {
verify(mockListener).onError(Status.DEADLINE_EXCEEDED);
verify(mockRetryScheduler).schedule(isA(Runnable.class));
}
}
}