core: Remove LB refreshNameResolver check

It's been 17 months since the check was introduced, which is plenty for
the migration. Leaving ignoreRefreshNameResolutionCheck() in-place to
let users delete their call sites. We'll remove the method after a few
releases.

Fixes #9409

api/src/main/java/io/grpc/LoadBalancer.java

@@ -1073,8 +1073,10 @@ public abstract class LoadBalancer {
      * that need to be updated for the new expected behavior.
      *
      * @since 1.38.0
+     * @deprecated Warning has been removed
      */
     @ExperimentalApi("https://github.com/grpc/grpc-java/issues/8088")
+    @Deprecated
     public void ignoreRefreshNameResolutionCheck() {
       // no-op
     }
@@ -1356,13 +1358,19 @@ public abstract class LoadBalancer {
      * unnecessary delays of RPCs. Please refer to {@link PickResult#withSubchannel
      * PickResult.withSubchannel()}'s javadoc for more information.
      *
+     * <p>When a subchannel's state is IDLE or TRANSIENT_FAILURE and the address for the subchannel
+     * was received in {@link LoadBalancer#handleResolvedAddresses}, load balancers should call
+     * {@link Helper#refreshNameResolution} to inform polling name resolvers that it is an
+     * appropriate time to refresh the addresses. Without the refresh, changes to the addresses may
+     * never be detected.
+     *
      * <p>SHUTDOWN can only happen in two cases.  One is that LoadBalancer called {@link
      * Subchannel#shutdown} earlier, thus it should have already discarded this Subchannel.  The
      * other is that Channel is doing a {@link ManagedChannel#shutdownNow forced shutdown} or has
      * already terminated, thus there won't be further requests to LoadBalancer.  Therefore, the
      * LoadBalancer usually don't need to react to a SHUTDOWN state.
-     * @param newState the new state
      *
+     * @param newState the new state
      * @since 1.22.0
      */
     void onSubchannelState(ConnectivityStateInfo newState);

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

@@ -1453,8 +1453,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
 
   private final class LbHelperImpl extends LoadBalancer.Helper {
     AutoConfiguredLoadBalancer lb;
-    boolean nsRefreshedByLb;
-    boolean ignoreRefreshNsCheck;
 
     @Override
     public AbstractSubchannel createSubchannel(CreateSubchannelArgs args) {
@@ -1493,7 +1491,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
     @Override
     public void refreshNameResolution() {
       syncContext.throwIfNotInThisSynchronizationContext();
-      nsRefreshedByLb = true;
       final class LoadBalancerRefreshNameResolution implements Runnable {
         @Override
         public void run() {
@@ -1504,11 +1501,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
       syncContext.execute(new LoadBalancerRefreshNameResolution());
     }
 
-    @Override
-    public void ignoreRefreshNameResolutionCheck() {
-      ignoreRefreshNsCheck = true;
-    }
-
     @Override
     public ManagedChannel createOobChannel(EquivalentAddressGroup addressGroup, String authority) {
       return createOobChannel(Collections.singletonList(addressGroup), authority);
@@ -1979,16 +1971,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
         void onStateChange(InternalSubchannel is, ConnectivityStateInfo newState) {
           checkState(listener != null, "listener is null");
           listener.onSubchannelState(newState);
-          if (newState.getState() == TRANSIENT_FAILURE || newState.getState() == IDLE) {
-            if (!helper.ignoreRefreshNsCheck && !helper.nsRefreshedByLb) {
-              logger.log(Level.WARNING,
-                  "LoadBalancer should call Helper.refreshNameResolution() to refresh name "
-                      + "resolution if subchannel state becomes TRANSIENT_FAILURE or IDLE. "
-                      + "This will no longer happen automatically in the future releases");
-              refreshAndResetNameResolution();
-              helper.nsRefreshedByLb = true;
-            }
-          }
         }
 
         @Override

core/src/main/java/io/grpc/util/ForwardingLoadBalancerHelper.java

@@ -95,6 +95,7 @@ public abstract class ForwardingLoadBalancerHelper extends LoadBalancer.Helper {
   }
 
   @Override
+  @Deprecated
   public void ignoreRefreshNameResolutionCheck() {
     delegate().ignoreRefreshNameResolutionCheck();
   }

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

@@ -140,9 +140,6 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.logging.Handler;
-import java.util.logging.Level;
-import java.util.logging.LogRecord;
 import javax.annotation.Nullable;
 import org.junit.After;
 import org.junit.Assert;
@@ -284,7 +281,6 @@ public class ManagedChannelImplTest {
   private boolean requestConnection = true;
   private BlockingQueue<MockClientTransportInfo> transports;
   private boolean panicExpected;
-  private final List<LogRecord> logs = new ArrayList<>();
   @Captor
   private ArgumentCaptor<ResolvedAddresses> resolvedAddressCaptor;
 
@@ -336,22 +332,6 @@ public class ManagedChannelImplTest {
     when(executorPool.getObject()).thenReturn(executor.getScheduledExecutorService());
     when(balancerRpcExecutorPool.getObject())
         .thenReturn(balancerRpcExecutor.getScheduledExecutorService());
-    Handler handler = new Handler() {
-      @Override
-      public void publish(LogRecord record) {
-        logs.add(record);
-      }
-
-      @Override
-      public void flush() {
-      }
-
-      @Override
-      public void close() throws SecurityException {
-      }
-    };
-    ManagedChannelImpl.logger.addHandler(handler);
-    ManagedChannelImpl.logger.setLevel(Level.ALL);
 
     channelBuilder = new ManagedChannelImplBuilder(TARGET,
         new UnsupportedClientTransportFactoryBuilder(), new FixedPortProvider(DEFAULT_PORT));
@@ -1591,103 +1571,6 @@ public class ManagedChannelImplTest {
     timer.forwardTime(ManagedChannelImpl.SUBCHANNEL_SHUTDOWN_DELAY_SECONDS, TimeUnit.SECONDS);
   }
 
-  @Test
-  public void subchannelConnectionBroken_noLbRefreshingResolver_logWarningAndTriggeRefresh() {
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri)
-            .setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
-            .build();
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    createChannel();
-    FakeNameResolverFactory.FakeNameResolver resolver =
-        Iterables.getOnlyElement(nameResolverFactory.resolvers);
-    assertThat(resolver.refreshCalled).isEqualTo(0);
-
-    Subchannel subchannel =
-        createSubchannelSafely(helper, addressGroup, Attributes.EMPTY, subchannelStateListener);
-    InternalSubchannel internalSubchannel =
-        (InternalSubchannel) subchannel.getInternalSubchannel();
-    internalSubchannel.obtainActiveTransport();
-    MockClientTransportInfo transportInfo = transports.poll();
-
-    // Break subchannel connection
-    transportInfo.listener.transportShutdown(Status.UNAVAILABLE.withDescription("unreachable"));
-    LogRecord log = Iterables.getOnlyElement(logs);
-    assertThat(log.getLevel()).isEqualTo(Level.WARNING);
-    assertThat(log.getMessage()).isEqualTo(
-        "LoadBalancer should call Helper.refreshNameResolution() to refresh name resolution if "
-            + "subchannel state becomes TRANSIENT_FAILURE or IDLE. This will no longer happen "
-            + "automatically in the future releases");
-    assertThat(resolver.refreshCalled).isEqualTo(1);
-  }
-
-  @Test
-  public void subchannelConnectionBroken_ResolverRefreshedByLb() {
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri)
-            .setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
-            .build();
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    createChannel();
-    FakeNameResolverFactory.FakeNameResolver resolver =
-        Iterables.getOnlyElement(nameResolverFactory.resolvers);
-    assertThat(resolver.refreshCalled).isEqualTo(0);
-    ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
-    verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
-    helper = helperCaptor.getValue();
-
-    SubchannelStateListener listener = new SubchannelStateListener() {
-      @Override
-      public void onSubchannelState(ConnectivityStateInfo newState) {
-        // Normal LoadBalancer should refresh name resolution when some subchannel enters
-        // TRANSIENT_FAILURE or IDLE
-        if (newState.getState() == TRANSIENT_FAILURE || newState.getState() == IDLE) {
-          helper.refreshNameResolution();
-        }
-      }
-    };
-    Subchannel subchannel =
-        createSubchannelSafely(helper, addressGroup, Attributes.EMPTY, listener);
-    InternalSubchannel internalSubchannel =
-        (InternalSubchannel) subchannel.getInternalSubchannel();
-    internalSubchannel.obtainActiveTransport();
-    MockClientTransportInfo transportInfo = transports.poll();
-
-    // Break subchannel connection and simulate load balancer refreshing name resolution
-    transportInfo.listener.transportShutdown(Status.UNAVAILABLE.withDescription("unreachable"));
-    assertThat(logs).isEmpty();
-    assertThat(resolver.refreshCalled).isEqualTo(1);
-  }
-
-  @Test
-  public void subchannelConnectionBroken_ignoreRefreshNameResolutionCheck_noRefresh() {
-    FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory.Builder(expectedUri)
-            .setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
-            .build();
-    channelBuilder.nameResolverFactory(nameResolverFactory);
-    createChannel();
-    FakeNameResolverFactory.FakeNameResolver resolver =
-        Iterables.getOnlyElement(nameResolverFactory.resolvers);
-    assertThat(resolver.refreshCalled).isEqualTo(0);
-    ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
-    verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
-    helper = helperCaptor.getValue();
-    helper.ignoreRefreshNameResolutionCheck();
-
-    Subchannel subchannel =
-        createSubchannelSafely(helper, addressGroup, Attributes.EMPTY, subchannelStateListener);
-    InternalSubchannel internalSubchannel =
-        (InternalSubchannel) subchannel.getInternalSubchannel();
-    internalSubchannel.obtainActiveTransport();
-    MockClientTransportInfo transportInfo = transports.poll();
-
-    // Break subchannel connection
-    transportInfo.listener.transportShutdown(Status.UNAVAILABLE.withDescription("unreachable"));
-    assertThat(logs).isEmpty();
-    assertThat(resolver.refreshCalled).isEqualTo(0);
-  }
-
   @Test
   public void subchannelStringableBeforeStart() {
     createChannel();

xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java

@@ -285,10 +285,8 @@ final class ClusterResolverLoadBalancer extends LoadBalancer {
     private final class RefreshableHelper extends ForwardingLoadBalancerHelper {
       private final Helper delegate;
 
-      @SuppressWarnings("deprecation")
       private RefreshableHelper(Helper delegate) {
         this.delegate = checkNotNull(delegate, "delegate");
-        delegate.ignoreRefreshNameResolutionCheck();
       }
 
       @Override

xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java

@@ -679,7 +679,6 @@ public class ClusterResolverLoadBalancerTest {
     EquivalentAddressGroup endpoint2 = makeAddress("endpoint-addr-2");
     resolver.deliverEndpointAddresses(Arrays.asList(endpoint1, endpoint2));
     assertThat(resolver.refreshCount).isEqualTo(0);
-    verify(helper).ignoreRefreshNameResolutionCheck();
     FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers);
     childBalancer.helper.refreshNameResolution();
     assertThat(resolver.refreshCount).isEqualTo(1);
@@ -745,7 +744,6 @@ public class ClusterResolverLoadBalancerTest {
     FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers);
     assertAddressesEqual(Collections.singletonList(endpoint), childBalancer.addresses);
     assertThat(resolver.refreshCount).isEqualTo(0);
-    verify(helper).ignoreRefreshNameResolutionCheck();
 
     childBalancer.helper.refreshNameResolution();
     assertThat(resolver.refreshCount).isEqualTo(1);