From ab9704560525468d28294c7c6c0e4266e79efab4 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Fri, 20 Sep 2024 14:25:17 +0000 Subject: [PATCH 01/21] SynchronizationContextTest changes for scheduleFixedDelay with Duration --- .../java/io/grpc/SynchronizationContext.java | 33 ++++ .../io/grpc/SynchronizationContextTest.java | 172 +++++++++++------- 2 files changed, 135 insertions(+), 70 deletions(-) diff --git a/api/src/main/java/io/grpc/SynchronizationContext.java b/api/src/main/java/io/grpc/SynchronizationContext.java index 5a7677ac15f..046988b4779 100644 --- a/api/src/main/java/io/grpc/SynchronizationContext.java +++ b/api/src/main/java/io/grpc/SynchronizationContext.java @@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkState; import java.lang.Thread.UncaughtExceptionHandler; +import java.time.Duration; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Executor; @@ -194,6 +195,38 @@ public String toString() { } + public final ScheduledHandle scheduleWithFixedDelay( + final Runnable task, Duration initialDelay, Duration delay, TimeUnit unit, + ScheduledExecutorService timerService) { + final ManagedRunnable runnable = new ManagedRunnable(task); + System.out.println("Inside Durationcall"); + ScheduledFuture future = timerService.scheduleWithFixedDelay(new Runnable() { + @Override + public void run() { + execute(runnable); + } + + @Override + public String toString() { + return task.toString() + "(scheduled in SynchronizationContext with delay of " + delay + + ")"; + } + }, toNanosSaturated(initialDelay), toNanosSaturated(delay), unit); + return new ScheduledHandle(runnable, future); + } + static long toNanosSaturated(Duration duration) { + // Using a try/catch seems lazy, but the catch block will rarely get invoked (except for + // durations longer than approximately +/- 292 years). + try { + //long delay = TimeUnit.MILLISECONDS.convert(500, TimeUnit.SECONDS); // Converts 500 seconds to milliseconds + return duration.toNanos(); + //return TimeUnit.NANOSECONDS.convert(duration); + + } catch (ArithmeticException tooBig) { + return duration.isNegative() ? Long.MIN_VALUE : Long.MAX_VALUE; + } + } + private static class ManagedRunnable implements Runnable { final Runnable task; boolean isCancelled; diff --git a/api/src/test/java/io/grpc/SynchronizationContextTest.java b/api/src/test/java/io/grpc/SynchronizationContextTest.java index 3d5e7fa42b9..a4d9e80837f 100644 --- a/api/src/test/java/io/grpc/SynchronizationContextTest.java +++ b/api/src/test/java/io/grpc/SynchronizationContextTest.java @@ -27,6 +27,7 @@ import com.google.common.util.concurrent.testing.TestingExecutors; import io.grpc.SynchronizationContext.ScheduledHandle; +import java.time.Duration; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; @@ -52,6 +53,7 @@ */ @RunWith(JUnit4.class) public class SynchronizationContextTest { + private final BlockingQueue uncaughtErrors = new LinkedBlockingQueue<>(); private final SynchronizationContext syncContext = new SynchronizationContext( new Thread.UncaughtExceptionHandler() { @@ -72,8 +74,9 @@ public void uncaughtException(Thread t, Throwable e) { @Mock private Runnable task3; - - @After public void tearDown() { + + @After + public void tearDown() { assertThat(uncaughtErrors).isEmpty(); } @@ -105,36 +108,36 @@ public void multiThread() throws Exception { final AtomicReference task2Thread = new AtomicReference<>(); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task1Thread.set(Thread.currentThread()); - task1Running.countDown(); - try { - assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - return null; + @Override + public Void answer(InvocationOnMock invocation) { + task1Thread.set(Thread.currentThread()); + task1Running.countDown(); + try { + assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + throw new RuntimeException(e); } - }).when(task1).run(); + return null; + } + }).when(task1).run(); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task2Thread.set(Thread.currentThread()); - return null; - } - }).when(task2).run(); + @Override + public Void answer(InvocationOnMock invocation) { + task2Thread.set(Thread.currentThread()); + return null; + } + }).when(task2).run(); Thread sideThread = new Thread() { - @Override - public void run() { - syncContext.executeLater(task1); - task1Added.countDown(); - syncContext.drain(); - sideThreadDone.countDown(); - } - }; + @Override + public void run() { + syncContext.executeLater(task1); + task1Added.countDown(); + syncContext.drain(); + sideThreadDone.countDown(); + } + }; sideThread.start(); assertTrue(task1Added.await(5, TimeUnit.SECONDS)); @@ -162,26 +165,26 @@ public void throwIfNotInThisSynchronizationContext() throws Exception { final CountDownLatch task1Proceed = new CountDownLatch(1); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task1Running.countDown(); - syncContext.throwIfNotInThisSynchronizationContext(); - try { - assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - taskSuccess.set(true); - return null; + @Override + public Void answer(InvocationOnMock invocation) { + task1Running.countDown(); + syncContext.throwIfNotInThisSynchronizationContext(); + try { + assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + throw new RuntimeException(e); } - }).when(task1).run(); + taskSuccess.set(true); + return null; + } + }).when(task1).run(); Thread sideThread = new Thread() { - @Override - public void run() { - syncContext.execute(task1); - } - }; + @Override + public void run() { + syncContext.execute(task1); + } + }; sideThread.start(); assertThat(task1Running.await(5, TimeUnit.SECONDS)).isTrue(); @@ -215,11 +218,11 @@ public void taskThrows() { InOrder inOrder = inOrder(task1, task2, task3); final RuntimeException e = new RuntimeException("Simulated"); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - throw e; - } - }).when(task2).run(); + @Override + public Void answer(InvocationOnMock invocation) { + throw e; + } + }).when(task2).run(); syncContext.executeLater(task1); syncContext.executeLater(task2); syncContext.executeLater(task3); @@ -246,6 +249,24 @@ public void schedule() { verify(task1).run(); } + @Test + public void testScheduleWithFixedDelay() { + MockScheduledExecutorService executorService = new MockScheduledExecutorService(); + + ScheduledHandle handle = + syncContext.scheduleWithFixedDelay(task1, Duration.ofNanos(110), Duration.ofNanos(110), + TimeUnit.NANOSECONDS, executorService); + + assertThat(executorService.delay) + .isEqualTo(executorService.unit.convert(110, TimeUnit.NANOSECONDS)); + assertThat(handle.isPending()).isTrue(); + verify(task1, never()).run(); + + executorService.command.run(); + assertThat(handle.isPending()).isFalse(); + verify(task1).run(); + } + @Test public void scheduleDueImmediately() { MockScheduledExecutorService executorService = new MockScheduledExecutorService(); @@ -288,28 +309,28 @@ public void scheduledHandle_cancelRacesWithTimerExpiration() throws Exception { final CountDownLatch sideThreadDone = new CountDownLatch(1); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task1Running.countDown(); - try { - ScheduledHandle task2Handle; - assertThat(task2Handle = task2HandleQueue.poll(5, TimeUnit.SECONDS)).isNotNull(); - task2Handle.cancel(); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - task1Done.set(true); - return null; + @Override + public Void answer(InvocationOnMock invocation) { + task1Running.countDown(); + try { + ScheduledHandle task2Handle; + assertThat(task2Handle = task2HandleQueue.poll(5, TimeUnit.SECONDS)).isNotNull(); + task2Handle.cancel(); + } catch (InterruptedException e) { + throw new RuntimeException(e); } - }).when(task1).run(); + task1Done.set(true); + return null; + } + }).when(task1).run(); Thread sideThread = new Thread() { - @Override - public void run() { - syncContext.execute(task1); - sideThreadDone.countDown(); - } - }; + @Override + public void run() { + syncContext.execute(task1); + sideThreadDone.countDown(); + } + }; ScheduledHandle handle = syncContext.schedule(task2, 10, TimeUnit.NANOSECONDS, executorService); // This will execute and block in task1 @@ -340,6 +361,7 @@ public void run() { } static class MockScheduledExecutorService extends ForwardingScheduledExecutorService { + private ScheduledExecutorService delegate = TestingExecutors.noOpScheduledExecutor(); Runnable command; @@ -347,15 +369,25 @@ static class MockScheduledExecutorService extends ForwardingScheduledExecutorSer TimeUnit unit; ScheduledFuture future; - @Override public ScheduledExecutorService delegate() { + @Override + public ScheduledExecutorService delegate() { return delegate; } - @Override public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { + @Override + public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { this.command = command; this.delay = delay; this.unit = unit; return future = super.schedule(command, delay, unit); } + @Override + public ScheduledFuture scheduleWithFixedDelay(Runnable command, long intialDelay, long delay, + TimeUnit unit) { + this.command = command; + this.delay = delay; + this.unit = unit; + return future = super.scheduleWithFixedDelay(command, intialDelay, delay, unit); + } } } From fef4c923f04864bbb8ef9420da910f687bb0da6d Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Fri, 20 Sep 2024 14:47:45 +0000 Subject: [PATCH 02/21] Revert "SynchronizationContextTest changes for scheduleFixedDelay with Duration" This reverts commit ab9704560525468d28294c7c6c0e4266e79efab4. --- .../java/io/grpc/SynchronizationContext.java | 33 ---- .../io/grpc/SynchronizationContextTest.java | 172 +++++++----------- 2 files changed, 70 insertions(+), 135 deletions(-) diff --git a/api/src/main/java/io/grpc/SynchronizationContext.java b/api/src/main/java/io/grpc/SynchronizationContext.java index 046988b4779..5a7677ac15f 100644 --- a/api/src/main/java/io/grpc/SynchronizationContext.java +++ b/api/src/main/java/io/grpc/SynchronizationContext.java @@ -20,7 +20,6 @@ import static com.google.common.base.Preconditions.checkState; import java.lang.Thread.UncaughtExceptionHandler; -import java.time.Duration; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Executor; @@ -195,38 +194,6 @@ public String toString() { } - public final ScheduledHandle scheduleWithFixedDelay( - final Runnable task, Duration initialDelay, Duration delay, TimeUnit unit, - ScheduledExecutorService timerService) { - final ManagedRunnable runnable = new ManagedRunnable(task); - System.out.println("Inside Durationcall"); - ScheduledFuture future = timerService.scheduleWithFixedDelay(new Runnable() { - @Override - public void run() { - execute(runnable); - } - - @Override - public String toString() { - return task.toString() + "(scheduled in SynchronizationContext with delay of " + delay - + ")"; - } - }, toNanosSaturated(initialDelay), toNanosSaturated(delay), unit); - return new ScheduledHandle(runnable, future); - } - static long toNanosSaturated(Duration duration) { - // Using a try/catch seems lazy, but the catch block will rarely get invoked (except for - // durations longer than approximately +/- 292 years). - try { - //long delay = TimeUnit.MILLISECONDS.convert(500, TimeUnit.SECONDS); // Converts 500 seconds to milliseconds - return duration.toNanos(); - //return TimeUnit.NANOSECONDS.convert(duration); - - } catch (ArithmeticException tooBig) { - return duration.isNegative() ? Long.MIN_VALUE : Long.MAX_VALUE; - } - } - private static class ManagedRunnable implements Runnable { final Runnable task; boolean isCancelled; diff --git a/api/src/test/java/io/grpc/SynchronizationContextTest.java b/api/src/test/java/io/grpc/SynchronizationContextTest.java index a4d9e80837f..3d5e7fa42b9 100644 --- a/api/src/test/java/io/grpc/SynchronizationContextTest.java +++ b/api/src/test/java/io/grpc/SynchronizationContextTest.java @@ -27,7 +27,6 @@ import com.google.common.util.concurrent.testing.TestingExecutors; import io.grpc.SynchronizationContext.ScheduledHandle; -import java.time.Duration; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; @@ -53,7 +52,6 @@ */ @RunWith(JUnit4.class) public class SynchronizationContextTest { - private final BlockingQueue uncaughtErrors = new LinkedBlockingQueue<>(); private final SynchronizationContext syncContext = new SynchronizationContext( new Thread.UncaughtExceptionHandler() { @@ -74,9 +72,8 @@ public void uncaughtException(Thread t, Throwable e) { @Mock private Runnable task3; - - @After - public void tearDown() { + + @After public void tearDown() { assertThat(uncaughtErrors).isEmpty(); } @@ -108,36 +105,36 @@ public void multiThread() throws Exception { final AtomicReference task2Thread = new AtomicReference<>(); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task1Thread.set(Thread.currentThread()); - task1Running.countDown(); - try { - assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); - } catch (InterruptedException e) { - throw new RuntimeException(e); + @Override + public Void answer(InvocationOnMock invocation) { + task1Thread.set(Thread.currentThread()); + task1Running.countDown(); + try { + assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return null; } - return null; - } - }).when(task1).run(); + }).when(task1).run(); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task2Thread.set(Thread.currentThread()); - return null; - } - }).when(task2).run(); + @Override + public Void answer(InvocationOnMock invocation) { + task2Thread.set(Thread.currentThread()); + return null; + } + }).when(task2).run(); Thread sideThread = new Thread() { - @Override - public void run() { - syncContext.executeLater(task1); - task1Added.countDown(); - syncContext.drain(); - sideThreadDone.countDown(); - } - }; + @Override + public void run() { + syncContext.executeLater(task1); + task1Added.countDown(); + syncContext.drain(); + sideThreadDone.countDown(); + } + }; sideThread.start(); assertTrue(task1Added.await(5, TimeUnit.SECONDS)); @@ -165,26 +162,26 @@ public void throwIfNotInThisSynchronizationContext() throws Exception { final CountDownLatch task1Proceed = new CountDownLatch(1); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task1Running.countDown(); - syncContext.throwIfNotInThisSynchronizationContext(); - try { - assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); - } catch (InterruptedException e) { - throw new RuntimeException(e); + @Override + public Void answer(InvocationOnMock invocation) { + task1Running.countDown(); + syncContext.throwIfNotInThisSynchronizationContext(); + try { + assertTrue(task1Proceed.await(5, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + taskSuccess.set(true); + return null; } - taskSuccess.set(true); - return null; - } - }).when(task1).run(); + }).when(task1).run(); Thread sideThread = new Thread() { - @Override - public void run() { - syncContext.execute(task1); - } - }; + @Override + public void run() { + syncContext.execute(task1); + } + }; sideThread.start(); assertThat(task1Running.await(5, TimeUnit.SECONDS)).isTrue(); @@ -218,11 +215,11 @@ public void taskThrows() { InOrder inOrder = inOrder(task1, task2, task3); final RuntimeException e = new RuntimeException("Simulated"); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - throw e; - } - }).when(task2).run(); + @Override + public Void answer(InvocationOnMock invocation) { + throw e; + } + }).when(task2).run(); syncContext.executeLater(task1); syncContext.executeLater(task2); syncContext.executeLater(task3); @@ -249,24 +246,6 @@ public void schedule() { verify(task1).run(); } - @Test - public void testScheduleWithFixedDelay() { - MockScheduledExecutorService executorService = new MockScheduledExecutorService(); - - ScheduledHandle handle = - syncContext.scheduleWithFixedDelay(task1, Duration.ofNanos(110), Duration.ofNanos(110), - TimeUnit.NANOSECONDS, executorService); - - assertThat(executorService.delay) - .isEqualTo(executorService.unit.convert(110, TimeUnit.NANOSECONDS)); - assertThat(handle.isPending()).isTrue(); - verify(task1, never()).run(); - - executorService.command.run(); - assertThat(handle.isPending()).isFalse(); - verify(task1).run(); - } - @Test public void scheduleDueImmediately() { MockScheduledExecutorService executorService = new MockScheduledExecutorService(); @@ -309,28 +288,28 @@ public void scheduledHandle_cancelRacesWithTimerExpiration() throws Exception { final CountDownLatch sideThreadDone = new CountDownLatch(1); doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) { - task1Running.countDown(); - try { - ScheduledHandle task2Handle; - assertThat(task2Handle = task2HandleQueue.poll(5, TimeUnit.SECONDS)).isNotNull(); - task2Handle.cancel(); - } catch (InterruptedException e) { - throw new RuntimeException(e); + @Override + public Void answer(InvocationOnMock invocation) { + task1Running.countDown(); + try { + ScheduledHandle task2Handle; + assertThat(task2Handle = task2HandleQueue.poll(5, TimeUnit.SECONDS)).isNotNull(); + task2Handle.cancel(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + task1Done.set(true); + return null; } - task1Done.set(true); - return null; - } - }).when(task1).run(); + }).when(task1).run(); Thread sideThread = new Thread() { - @Override - public void run() { - syncContext.execute(task1); - sideThreadDone.countDown(); - } - }; + @Override + public void run() { + syncContext.execute(task1); + sideThreadDone.countDown(); + } + }; ScheduledHandle handle = syncContext.schedule(task2, 10, TimeUnit.NANOSECONDS, executorService); // This will execute and block in task1 @@ -361,7 +340,6 @@ public void run() { } static class MockScheduledExecutorService extends ForwardingScheduledExecutorService { - private ScheduledExecutorService delegate = TestingExecutors.noOpScheduledExecutor(); Runnable command; @@ -369,25 +347,15 @@ static class MockScheduledExecutorService extends ForwardingScheduledExecutorSer TimeUnit unit; ScheduledFuture future; - @Override - public ScheduledExecutorService delegate() { + @Override public ScheduledExecutorService delegate() { return delegate; } - @Override - public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { + @Override public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { this.command = command; this.delay = delay; this.unit = unit; return future = super.schedule(command, delay, unit); } - @Override - public ScheduledFuture scheduleWithFixedDelay(Runnable command, long intialDelay, long delay, - TimeUnit unit) { - this.command = command; - this.delay = delay; - this.unit = unit; - return future = super.scheduleWithFixedDelay(command, intialDelay, delay, unit); - } } } From 4c31880f4d475ad96d09329b7617e2c02d57bfd6 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Thu, 17 Oct 2024 15:48:52 +0000 Subject: [PATCH 03/21] api: Javadoc changes for LoadBalancer method signatures --- api/src/main/java/io/grpc/LoadBalancer.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index 6d74006b396..ada91db0b77 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -156,11 +156,11 @@ public String toString() { private int recursionCount; /** - * Handles newly resolved server groups and metadata attributes from name resolution system. - * {@code servers} contained in {@link EquivalentAddressGroup} should be considered equivalent - * but may be flattened into a single list if needed. + * Handles newly resolved addresses and metadata attributes from name resolution system. + * {@link EquivalentAddressGroup} addresses should be considered equivalent but may be flattened + * into a single list if needed. * - *

Implementations should not modify the given {@code servers}. + *

Implementations should not modify the given {@code resolvedAddresses}. * * @param resolvedAddresses the resolved server addresses, attributes, and config. * @since 1.21.0 @@ -179,12 +179,12 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { * EquivalentAddressGroup} addresses should be considered equivalent but may be flattened into a * single list if needed. * - *

Implementations can choose to reject the given addresses by returning {@code false}. + *

Implementations can choose to reject the given addresses by returning {@code UNAVAILABLE}. * - *

Implementations should not modify the given {@code addresses}. + *

Implementations should not modify the given {@code resolvedAddresses}. * * @param resolvedAddresses the resolved server addresses, attributes, and config. - * @return {@code true} if the resolved addresses were accepted. {@code false} if rejected. + * @return {@code OK} if the resolved addresses were accepted. {@code UNAVAILABLE} if rejected. * @since 1.49.0 */ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { From 9ddcb09bfd30d932f8a5b04d9d617f72610d3238 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 21 Oct 2024 09:40:21 +0000 Subject: [PATCH 04/21] api: Changes for UNAVAILABLE to Status.UNAVAILABLE and Status.OK --- api/src/main/java/io/grpc/LoadBalancer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index ada91db0b77..b8ae831554c 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -179,12 +179,12 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { * EquivalentAddressGroup} addresses should be considered equivalent but may be flattened into a * single list if needed. * - *

Implementations can choose to reject the given addresses by returning {@code UNAVAILABLE}. + *

Implementations can choose to reject the given addresses by returning {@code Status.UNAVAILABLE}. * *

Implementations should not modify the given {@code resolvedAddresses}. * * @param resolvedAddresses the resolved server addresses, attributes, and config. - * @return {@code OK} if the resolved addresses were accepted. {@code UNAVAILABLE} if rejected. + * @return {@code Status.OK} if the resolved addresses were accepted. {@code Status.UNAVAILABLE} if rejected. * @since 1.49.0 */ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { From 596d11db5b3e9d4da0b39e7ea534eae45f2938b7 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 21 Oct 2024 10:31:52 +0000 Subject: [PATCH 05/21] api: Check style fix for UNAVAILABLE to Status.UNAVAILABLE and Status.OK --- api/src/main/java/io/grpc/LoadBalancer.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index b8ae831554c..e5c70d22f7f 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -179,12 +179,14 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { * EquivalentAddressGroup} addresses should be considered equivalent but may be flattened into a * single list if needed. * - *

Implementations can choose to reject the given addresses by returning {@code Status.UNAVAILABLE}. + *

Implementations can choose to reject the given addresses by returning + * {@code Status.UNAVAILABLE}. * *

Implementations should not modify the given {@code resolvedAddresses}. * * @param resolvedAddresses the resolved server addresses, attributes, and config. - * @return {@code Status.OK} if the resolved addresses were accepted. {@code Status.UNAVAILABLE} if rejected. + * @return {@code Status.OK} if the resolved addresses were accepted. + * {@code Status.UNAVAILABLE} if rejected. * @since 1.49.0 */ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { From d9bf85ec8c56a57e9c8c2deb904752ea03c41fed Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 21 Oct 2024 10:44:53 +0000 Subject: [PATCH 06/21] api: Check style fix for Status.UNAVAILABLE and Status.OK --- api/src/main/java/io/grpc/LoadBalancer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index e5c70d22f7f..d945f962d33 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -187,6 +187,7 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { * @param resolvedAddresses the resolved server addresses, attributes, and config. * @return {@code Status.OK} if the resolved addresses were accepted. * {@code Status.UNAVAILABLE} if rejected. + * * @since 1.49.0 */ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { From 401a7d274d22186c3c0e2a3e5c71bb8756ef0a62 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 21 Oct 2024 10:54:37 +0000 Subject: [PATCH 07/21] api: Check style fix for Status.UNAVAILABLE --- api/src/main/java/io/grpc/LoadBalancer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index d945f962d33..061be07ba28 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -185,8 +185,8 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { *

Implementations should not modify the given {@code resolvedAddresses}. * * @param resolvedAddresses the resolved server addresses, attributes, and config. - * @return {@code Status.OK} if the resolved addresses were accepted. - * {@code Status.UNAVAILABLE} if rejected. + * @return {@code Status.OK} if the resolved addresses were accepted. {@code Status.UNAVAILABLE} + * if rejected. * * @since 1.49.0 */ From f388ac204dd7229c7ed8f2c135ca8e7dcad81c71 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 21 Oct 2024 11:03:43 +0000 Subject: [PATCH 08/21] api: Check style fix for Status.UNAVAILABLE --- api/src/main/java/io/grpc/LoadBalancer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index 061be07ba28..cf4704388b1 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -186,7 +186,7 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { * * @param resolvedAddresses the resolved server addresses, attributes, and config. * @return {@code Status.OK} if the resolved addresses were accepted. {@code Status.UNAVAILABLE} - * if rejected. + * if rejected. * * @since 1.49.0 */ From 846eb007bf713eb3a6a6ea1b865e1ce8a6758eff Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 28 Oct 2024 12:20:15 +0000 Subject: [PATCH 09/21] api/util/services: Deprected handleResolvedAddresses method and related changes --- api/src/main/java/io/grpc/LoadBalancer.java | 3 +- .../test/java/io/grpc/LoadBalancerTest.java | 14 +-- .../HealthCheckingLoadBalancerFactory.java | 4 +- ...HealthCheckingLoadBalancerFactoryTest.java | 97 ++++++++++--------- .../io/grpc/util/ForwardingLoadBalancer.java | 4 +- .../grpc/util/GracefulSwitchLoadBalancer.java | 27 ------ .../io/grpc/util/MultiChildLoadBalancer.java | 2 +- .../util/OutlierDetectionLoadBalancer.java | 2 +- .../grpc/util/ForwardingLoadBalancerTest.java | 4 +- .../util/GracefulSwitchLoadBalancerTest.java | 36 +++---- .../OutlierDetectionLoadBalancerTest.java | 2 +- 11 files changed, 87 insertions(+), 108 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index cf4704388b1..a7fd58f0cc3 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -165,6 +165,7 @@ public String toString() { * @param resolvedAddresses the resolved server addresses, attributes, and config. * @since 1.21.0 */ + @Deprecated public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (recursionCount++ == 0) { // Note that the information about the addresses actually being accepted will be lost @@ -200,7 +201,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { return unavailableStatus; } else { if (recursionCount++ == 0) { - handleResolvedAddresses(resolvedAddresses); + //handleResolvedAddresses(resolvedAddresses); } recursionCount = 0; diff --git a/api/src/test/java/io/grpc/LoadBalancerTest.java b/api/src/test/java/io/grpc/LoadBalancerTest.java index 5e9e5cbe816..091a332f014 100644 --- a/api/src/test/java/io/grpc/LoadBalancerTest.java +++ b/api/src/test/java/io/grpc/LoadBalancerTest.java @@ -263,7 +263,7 @@ public void shutdown() { new EquivalentAddressGroup(new SocketAddress(){})); ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) .setAttributes(attrs).build(); - balancer.handleResolvedAddresses(addresses); + balancer.acceptResolvedAddresses(addresses); assertThat(resultCapture.get()).isEqualTo( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build()); } @@ -275,8 +275,9 @@ public void acceptResolvedAddresses_delegatesToHandleResolvedAddressGroups() { LoadBalancer balancer = new LoadBalancer() { @Override - public void handleResolvedAddresses(ResolvedAddresses addresses) { + public Status acceptResolvedAddresses(ResolvedAddresses addresses) { addressesCapture.set(addresses); + return Status.OK; } @Override @@ -297,7 +298,7 @@ public void shutdown() { new EquivalentAddressGroup(new SocketAddress(){})); ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) .setAttributes(attrs).build(); - balancer.handleResolvedAddresses(addresses); + balancer.acceptResolvedAddresses(addresses); assertThat(addressesCapture.get().getAddresses()).isEqualTo(servers); assertThat(addressesCapture.get().getAttributes()).isEqualTo(attrs); } @@ -309,9 +310,10 @@ public void acceptResolvedAddresses_noInfiniteLoop() { LoadBalancer balancer = new LoadBalancer() { @Override - public void handleResolvedAddresses(ResolvedAddresses addresses) { + public Status acceptResolvedAddresses(ResolvedAddresses addresses) { addressesCapture.add(addresses); - super.handleResolvedAddresses(addresses); + super.acceptResolvedAddresses(addresses); + return Status.OK; } @Override @@ -328,7 +330,7 @@ public void shutdown() { new EquivalentAddressGroup(new SocketAddress(){})); ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) .setAttributes(attrs).build(); - balancer.handleResolvedAddresses(addresses); + balancer.acceptResolvedAddresses(addresses); assertThat(addressesCapture).hasSize(1); assertThat(addressesCapture.get(0).getAddresses()).isEqualTo(servers); assertThat(addressesCapture.get(0).getAttributes()).isEqualTo(attrs); diff --git a/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java b/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java index cac522caf9e..91e8cc14882 100644 --- a/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java +++ b/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java @@ -187,14 +187,14 @@ protected LoadBalancer delegate() { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { Map healthCheckingConfig = resolvedAddresses .getAttributes() .get(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG); String serviceName = ServiceConfigUtil.getHealthCheckedServiceName(healthCheckingConfig); helper.setHealthCheckedService(serviceName); - super.handleResolvedAddresses(resolvedAddresses); + return super.acceptResolvedAddresses(resolvedAddresses); } @Override diff --git a/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java b/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java index 08a33106fb9..a49c426f7e1 100644 --- a/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java +++ b/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java @@ -206,15 +206,16 @@ public void setup() throws Exception { boolean shutdown; @Override - public void handleResolvedAddresses(final ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(final ResolvedAddresses resolvedAddresses) { syncContext.execute(new Runnable() { @Override public void run() { if (!shutdown) { - hcLb.handleResolvedAddresses(resolvedAddresses); + hcLb.acceptResolvedAddresses(resolvedAddresses); } } }); + return Status.OK; } @Override @@ -264,9 +265,9 @@ public void typicalWorkflow() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verify(origHelper, atLeast(0)).getSynchronizationContext(); verify(origHelper, atLeast(0)).getScheduledExecutorService(); verifyNoMoreInteractions(origHelper); @@ -404,9 +405,9 @@ public void healthCheckDisabledWhenServiceNotImplemented() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); // We create 2 Subchannels. One of them connects to a server that doesn't implement health check @@ -489,9 +490,9 @@ public void backoffRetriesWhenServerErroneouslyClosesRpcBeforeAnyResponse() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); SubchannelStateListener mockHealthListener = mockHealthListeners[0]; @@ -567,9 +568,9 @@ public void serverRespondResetsBackoff() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); SubchannelStateListener mockStateListener = mockStateListeners[0]; @@ -667,9 +668,9 @@ public void serviceConfigHasNoHealthCheckingInitiallyButDoesLater() { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); // First, create Subchannels 0 @@ -688,8 +689,8 @@ public void serviceConfigHasNoHealthCheckingInitiallyButDoesLater() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); - verify(origLb).handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); + verify(origLb).acceptResolvedAddresses(result2); // Health check started on existing Subchannel assertThat(healthImpls[0].calls).hasSize(1); @@ -711,9 +712,9 @@ public void serviceConfigDisablesHealthCheckWhenRpcActive() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY, maybeGetMockListener()); @@ -738,7 +739,7 @@ public void serviceConfigDisablesHealthCheckWhenRpcActive() { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); // Health check RPC cancelled. assertThat(serverCall.cancelled).isTrue(); @@ -746,7 +747,7 @@ public void serviceConfigDisablesHealthCheckWhenRpcActive() { inOrder.verify(getMockListener()).onSubchannelState( eq(ConnectivityStateInfo.forNonError(READY))); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); verifyNoMoreInteractions(origLb, mockStateListeners[0]); assertThat(healthImpl.calls).isEmpty(); @@ -759,9 +760,9 @@ public void serviceConfigDisablesHealthCheckWhenRetryPending() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); SubchannelStateListener mockHealthListener = mockHealthListeners[0]; @@ -793,7 +794,7 @@ public void serviceConfigDisablesHealthCheckWhenRetryPending() { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); // Retry timer is cancelled assertThat(clock.getPendingTasks()).isEmpty(); @@ -805,7 +806,7 @@ public void serviceConfigDisablesHealthCheckWhenRetryPending() { inOrder.verify(getMockListener()).onSubchannelState( eq(ConnectivityStateInfo.forNonError(READY))); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); verifyNoMoreInteractions(origLb, mockStateListeners[0]); } @@ -817,9 +818,9 @@ public void serviceConfigDisablesHealthCheckWhenRpcInactive() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY, maybeGetMockListener()); @@ -842,9 +843,9 @@ public void serviceConfigDisablesHealthCheckWhenRpcInactive() { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Underlying subchannel is now ready deliverSubchannelState(0, ConnectivityStateInfo.forNonError(READY)); @@ -870,9 +871,9 @@ public void serviceConfigChangesServiceNameWhenRpcActive() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); SubchannelStateListener mockHealthListener = mockHealthListeners[0]; @@ -900,9 +901,9 @@ public void serviceConfigChangesServiceNameWhenRpcActive() { eq(ConnectivityStateInfo.forNonError(READY))); // Service config returns with the same health check name. - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); // It's delivered to origLb, but nothing else happens - inOrder.verify(origLb).handleResolvedAddresses(result1); + inOrder.verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb, mockListener); // Service config returns a different health check name. @@ -911,8 +912,8 @@ public void serviceConfigChangesServiceNameWhenRpcActive() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); - inOrder.verify(origLb).handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Current health check RPC cancelled. assertThat(serverCall.cancelled).isTrue(); @@ -934,9 +935,9 @@ public void serviceConfigChangesServiceNameWhenRetryPending() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); SubchannelStateListener mockHealthListener = mockHealthListeners[0]; @@ -969,9 +970,9 @@ public void serviceConfigChangesServiceNameWhenRetryPending() { // Service config returns with the same health check name. - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); // It's delivered to origLb, but nothing else happens - inOrder.verify(origLb).handleResolvedAddresses(result1); + inOrder.verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb, mockListener); assertThat(clock.getPendingTasks()).hasSize(1); assertThat(healthImpl.calls).isEmpty(); @@ -982,12 +983,12 @@ public void serviceConfigChangesServiceNameWhenRetryPending() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); // Concluded CONNECTING state inOrder.verify(getMockListener()).onSubchannelState( eq(ConnectivityStateInfo.forNonError(CONNECTING))); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Current retry timer cancelled assertThat(clock.getPendingTasks()).isEmpty(); @@ -1008,9 +1009,9 @@ public void serviceConfigChangesServiceNameWhenRpcInactive() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY, maybeGetMockListener()); @@ -1031,9 +1032,9 @@ public void serviceConfigChangesServiceNameWhenRpcInactive() { inOrder.verifyNoMoreInteractions(); // Service config returns with the same health check name. - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); // It's delivered to origLb, but nothing else happens - inOrder.verify(origLb).handleResolvedAddresses(result1); + inOrder.verify(origLb).acceptResolvedAddresses(result1); assertThat(healthImpl.calls).isEmpty(); verifyNoMoreInteractions(origLb); @@ -1043,9 +1044,9 @@ public void serviceConfigChangesServiceNameWhenRpcInactive() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Underlying subchannel is now ready deliverSubchannelState(0, ConnectivityStateInfo.forNonError(READY)); @@ -1092,9 +1093,9 @@ public void balancerShutdown() { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); ServerSideCall[] serverCalls = new ServerSideCall[NUM_SUBCHANNELS]; @@ -1172,8 +1173,8 @@ public LoadBalancer newLoadBalancer(Helper helper) { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); createSubchannel(0, Attributes.EMPTY); assertThat(healthImpls[0].calls).isEmpty(); deliverSubchannelState(0, ConnectivityStateInfo.forNonError(READY)); diff --git a/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java b/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java index cefcbf344ea..8112e55fbbc 100644 --- a/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java @@ -30,8 +30,8 @@ public abstract class ForwardingLoadBalancer extends LoadBalancer { protected abstract LoadBalancer delegate(); @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - delegate().handleResolvedAddresses(resolvedAddresses); + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + return delegate().acceptResolvedAddresses(resolvedAddresses); } @Override diff --git a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index a63a641b037..614e1d6d504 100644 --- a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -55,19 +55,6 @@ @NotThreadSafe // Must be accessed in SynchronizationContext public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private final LoadBalancer defaultBalancer = new LoadBalancer() { - @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - // Most LB policies using this class will receive child policy configuration within the - // service config, so they are naturally calling switchTo() just before - // handleResolvedAddresses(), within their own handleResolvedAddresses(). If switchTo() is - // not called immediately after construction that does open up potential for bugs in the - // parent policies, where they fail to call switchTo(). So we will use the exception to try - // to notice those bugs quickly, as it will fail very loudly. - throw new IllegalStateException( - "GracefulSwitchLoadBalancer must switch to a load balancing policy before handling" - + " ResolvedAddresses"); - } - @Override public void handleNameResolutionError(final Status error) { helper.updateBalancingState( @@ -112,20 +99,6 @@ public GracefulSwitchLoadBalancer(Helper helper) { this.helper = checkNotNull(helper, "helper"); } - @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - if (switchToCalled) { - delegate().handleResolvedAddresses(resolvedAddresses); - return; - } - Config config = (Config) resolvedAddresses.getLoadBalancingPolicyConfig(); - switchToInternal(config.childFactory); - delegate().handleResolvedAddresses( - resolvedAddresses.toBuilder() - .setLoadBalancingPolicyConfig(config.childConfig) - .build()); - } - @Override public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (switchToCalled) { diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java index 3e56f41d038..389350bc346 100644 --- a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java @@ -193,7 +193,7 @@ private List updateChildrenWithResolvedAddresses( newChildLbStates.add(childLbState); if (entry.getValue() != null) { childLbState.setResolvedAddresses(entry.getValue()); // update child - childLbState.lb.handleResolvedAddresses(entry.getValue()); // update child LB + childLbState.lb.acceptResolvedAddresses(entry.getValue()); // update child LB } } diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 1f0290e76d7..080faba578a 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -171,7 +171,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { endpointTrackerMap.cancelTracking(); } - switchLb.handleResolvedAddresses( + switchLb.acceptResolvedAddresses( resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(config.childConfig).build()); return Status.OK; } diff --git a/util/src/test/java/io/grpc/util/ForwardingLoadBalancerTest.java b/util/src/test/java/io/grpc/util/ForwardingLoadBalancerTest.java index f9b53400cea..afdbfd77dad 100644 --- a/util/src/test/java/io/grpc/util/ForwardingLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/ForwardingLoadBalancerTest.java @@ -45,6 +45,8 @@ public void allMethodsForwarded() throws Exception { mockDelegate, new TestBalancer(), Arrays.asList( - LoadBalancer.class.getMethod("acceptResolvedAddresses", ResolvedAddresses.class))); + LoadBalancer.class.getMethod("acceptResolvedAddresses", ResolvedAddresses.class), + LoadBalancer.class.getMethod("handleResolvedAddresses", ResolvedAddresses.class)) + ); } } diff --git a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index f31443ace7b..1c87091a192 100644 --- a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -125,17 +125,17 @@ public void switchTo_handleResolvedAddressesAndNameResolutionErrorForwardedToLat helper0.updateBalancingState(READY, picker); ResolvedAddresses addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0).handleResolvedAddresses(addresses); + gracefulSwitchLb.acceptResolvedAddresses(addresses); + verify(lb0).acceptResolvedAddresses(addresses); gracefulSwitchLb.handleNameResolutionError(Status.DATA_LOSS); verify(lb0).handleNameResolutionError(Status.DATA_LOSS); gracefulSwitchLb.switchTo(lbPolicies[1]); LoadBalancer lb1 = balancers.get(lbPolicies[1]); addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0, never()).handleResolvedAddresses(addresses); - verify(lb1).handleResolvedAddresses(addresses); + gracefulSwitchLb.acceptResolvedAddresses(addresses); + verify(lb0, never()).acceptResolvedAddresses(addresses); + verify(lb1).acceptResolvedAddresses(addresses); gracefulSwitchLb.handleNameResolutionError(Status.ALREADY_EXISTS); verify(lb0, never()).handleNameResolutionError(Status.ALREADY_EXISTS); verify(lb1).handleNameResolutionError(Status.ALREADY_EXISTS); @@ -144,10 +144,10 @@ public void switchTo_handleResolvedAddressesAndNameResolutionErrorForwardedToLat verify(lb1).shutdown(); LoadBalancer lb2 = balancers.get(lbPolicies[2]); addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0, never()).handleResolvedAddresses(addresses); - verify(lb1, never()).handleResolvedAddresses(addresses); - verify(lb2).handleResolvedAddresses(addresses); + gracefulSwitchLb.acceptResolvedAddresses(addresses); + verify(lb0, never()).acceptResolvedAddresses(addresses); + verify(lb1, never()).acceptResolvedAddresses(addresses); + verify(lb2).acceptResolvedAddresses(addresses); gracefulSwitchLb.handleNameResolutionError(Status.CANCELLED); verify(lb0, never()).handleNameResolutionError(Status.CANCELLED); verify(lb1, never()).handleNameResolutionError(Status.CANCELLED); @@ -591,11 +591,11 @@ public void canHandleEmptyAddressListFromNameResolutionForwardedToLatestPolicy() public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { ResolvedAddresses addresses = newFakeAddresses(); Object child0Config = new Object(); - gracefulSwitchLb.handleResolvedAddresses(addresses.toBuilder() + gracefulSwitchLb.acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(createConfig(lbPolicies[0], child0Config)) .build()); LoadBalancer lb0 = balancers.get(lbPolicies[0]); - verify(lb0).handleResolvedAddresses(addresses.toBuilder() + verify(lb0).acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child0Config) .build()); Helper helper0 = helpers.get(lb0); @@ -606,14 +606,14 @@ public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy Object child1Config = new Object(); addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses.toBuilder() + gracefulSwitchLb.acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(createConfig(lbPolicies[1], child1Config)) .build()); LoadBalancer lb1 = balancers.get(lbPolicies[1]); - verify(lb0, never()).handleResolvedAddresses(addresses.toBuilder() + verify(lb0, never()).acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child1Config) .build()); - verify(lb1).handleResolvedAddresses(addresses.toBuilder() + verify(lb1).acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child1Config) .build()); gracefulSwitchLb.handleNameResolutionError(Status.ALREADY_EXISTS); @@ -622,18 +622,18 @@ public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy Object child2Config = new Object(); addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses.toBuilder() + gracefulSwitchLb.acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(createConfig(lbPolicies[2], child2Config)) .build()); verify(lb1).shutdown(); LoadBalancer lb2 = balancers.get(lbPolicies[2]); - verify(lb0, never()).handleResolvedAddresses(addresses.toBuilder() + verify(lb0, never()).acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child2Config) .build()); - verify(lb1, never()).handleResolvedAddresses(addresses.toBuilder() + verify(lb1, never()).acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child2Config) .build()); - verify(lb2).handleResolvedAddresses(addresses.toBuilder() + verify(lb2).acceptResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child2Config) .build()); gracefulSwitchLb.handleNameResolutionError(Status.CANCELLED); diff --git a/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java b/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java index 1b0139affef..d81740e116a 100644 --- a/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/OutlierDetectionLoadBalancerTest.java @@ -280,7 +280,7 @@ public void acceptResolvedAddresses() { loadBalancer.acceptResolvedAddresses(resolvedAddresses); // Handling of resolved addresses is delegated - verify(mockChildLb).handleResolvedAddresses( + verify(mockChildLb).acceptResolvedAddresses( resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(childConfig).build()); // There is a single pending task to run the outlier detection algorithm From ee36c4e0130bc6f13d48cbf4df82457917345db6 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Tue, 29 Oct 2024 04:45:28 +0000 Subject: [PATCH 10/21] interop-testing: Changes for deprecated handleResolvedAddresses method and related changes --- .../testing/integration/RpcBehaviorLoadBalancerProvider.java | 4 ++-- .../integration/RpcBehaviorLoadBalancerProviderTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java b/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java index 83c416765ec..7d78725a82d 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java @@ -111,10 +111,10 @@ protected LoadBalancer delegate() { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { helper.setRpcBehavior( ((RpcBehaviorConfig) resolvedAddresses.getLoadBalancingPolicyConfig()).rpcBehavior); - delegateLb.handleResolvedAddresses(resolvedAddresses); + return delegateLb.acceptResolvedAddresses(resolvedAddresses); } } diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java index 02ede46bcdd..9ae2c3f70ec 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java @@ -83,8 +83,8 @@ public void handleResolvedAddressesDelegated() { RpcBehaviorLoadBalancer lb = new RpcBehaviorLoadBalancer(new RpcBehaviorHelper(mockHelper), mockDelegateLb); ResolvedAddresses resolvedAddresses = buildResolvedAddresses(buildConfig()); - lb.handleResolvedAddresses(resolvedAddresses); - verify(mockDelegateLb).handleResolvedAddresses(resolvedAddresses); + lb.acceptResolvedAddresses(resolvedAddresses); + verify(mockDelegateLb).acceptResolvedAddresses(resolvedAddresses); } @Test From b1f857f38011df84e745322f6a44adad9989fa63 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Tue, 29 Oct 2024 07:15:42 +0000 Subject: [PATCH 11/21] xds: Changes for deprecated handleResolvedAddresses method and related changes --- .../io/grpc/xds/CdsLoadBalancer2Test.java | 3 +- .../xds/MetadataLoadBalancerProvider.java | 4 +- .../io/grpc/xds/PriorityLoadBalancerTest.java | 41 ++++++++++--------- .../xds/WeightedTargetLoadBalancerTest.java | 21 +++++----- .../grpc/xds/WrrLocalityLoadBalancerTest.java | 8 ++-- 5 files changed, 40 insertions(+), 37 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java index da32332a2a5..7357817c665 100644 --- a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java +++ b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java @@ -797,8 +797,9 @@ private final class FakeLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { config = resolvedAddresses.getLoadBalancingPolicyConfig(); + return Status.OK; } @Override diff --git a/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java b/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java index ecc0112a2e0..45f2337b746 100644 --- a/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java +++ b/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java @@ -108,11 +108,11 @@ protected LoadBalancer delegate() { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { MetadataLoadBalancerConfig config = (MetadataLoadBalancerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); helper.setMetadata(config.metadataKey, config.metadataValue); - delegateLb.handleResolvedAddresses(resolvedAddresses); + return delegateLb.acceptResolvedAddresses(resolvedAddresses); } } diff --git a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java index fafcd4d674a..a79af4cb66c 100644 --- a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java @@ -141,7 +141,7 @@ public void tearDown() { } @Test - public void handleResolvedAddresses() { + public void acceptResolvedAddresses() { SocketAddress socketAddress = new InetSocketAddress(8080); EquivalentAddressGroup eag = new EquivalentAddressGroup(socketAddress); eag = AddressFilter.setPathFilter(eag, ImmutableList.of("p1")); @@ -162,7 +162,7 @@ public void handleResolvedAddresses() { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2), ImmutableList.of("p0", "p1", "p2")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(addresses) .setAttributes(attributes) @@ -171,7 +171,7 @@ public void handleResolvedAddresses() { assertThat(fooBalancers).hasSize(1); assertThat(barBalancers).isEmpty(); LoadBalancer fooBalancer0 = Iterables.getOnlyElement(fooBalancers); - verify(fooBalancer0).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(fooBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); ResolvedAddresses addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(addressesReceived.getAddresses()).isEmpty(); assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); @@ -182,7 +182,7 @@ public void handleResolvedAddresses() { assertThat(fooBalancers).hasSize(1); assertThat(barBalancers).hasSize(1); LoadBalancer barBalancer0 = Iterables.getOnlyElement(barBalancers); - verify(barBalancer0).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(barBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) .containsExactly(socketAddress); @@ -194,7 +194,7 @@ public void handleResolvedAddresses() { assertThat(fooBalancers).hasSize(2); assertThat(barBalancers).hasSize(1); LoadBalancer fooBalancer1 = Iterables.getLast(fooBalancers); - verify(fooBalancer1).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(fooBalancer1).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(addressesReceived.getAddresses()).isEmpty(); assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); @@ -211,14 +211,14 @@ public void handleResolvedAddresses() { ImmutableMap.of("p1", new PriorityChildConfig(newChildConfig(barLbProvider, newBarConfig), true)), ImmutableList.of("p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(newAddresses) .setLoadBalancingPolicyConfig(newPriorityLbConfig) .build()); assertThat(fooBalancers).hasSize(2); assertThat(barBalancers).hasSize(1); - verify(barBalancer0, times(2)).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(barBalancer0, times(2)).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) .containsExactly(newSocketAddress); @@ -243,7 +243,7 @@ public void handleNameResolutionError() { PriorityLbConfig priorityLbConfig = new PriorityLbConfig(ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -255,7 +255,7 @@ public void handleNameResolutionError() { priorityLbConfig = new PriorityLbConfig(ImmutableMap.of("p1", priorityChildConfig1), ImmutableList.of("p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -286,7 +286,7 @@ public void typicalPriorityFailOverFlow() { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2, "p3", priorityChildConfig3), ImmutableList.of("p0", "p1", "p2", "p3")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -419,7 +419,7 @@ public void idleToConnectingDoesNotTriggerFailOver() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -455,7 +455,7 @@ public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -497,7 +497,7 @@ public void readyToConnectDoesNotFailOverButUpdatesPicker() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -530,7 +530,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) { // resolution update without priority change does not trigger failover Attributes.Key fooKey = Attributes.Key.create("fooKey"); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -559,7 +559,7 @@ public void typicalPriorityFailOverFlowWithIdleUpdate() { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2, "p3", priorityChildConfig3), ImmutableList.of("p0", "p1", "p2", "p3")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -662,7 +662,7 @@ public void bypassReresolutionRequestsIfConfiged() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -690,7 +690,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -717,7 +717,7 @@ public void noDuplicateOverallBalancingStateUpdate() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -727,7 +727,7 @@ public void noDuplicateOverallBalancingStateUpdate() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -801,9 +801,10 @@ static class FakeLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { helper.updateBalancingState( TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(Status.INTERNAL))); + return Status.OK; } @Override diff --git a/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java index cc6cb98412c..46fe3881167 100644 --- a/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java @@ -180,7 +180,7 @@ public void tearDown() { } @Test - public void handleResolvedAddresses() { + public void acceptResolvedAddresses() { ArgumentCaptor resolvedAddressesCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); Attributes.Key fakeKey = Attributes.Key.create("fake_key"); @@ -203,7 +203,7 @@ public void handleResolvedAddresses() { eag2 = AddressFilter.setPathFilter(eag2, ImmutableList.of("target2")); EquivalentAddressGroup eag3 = new EquivalentAddressGroup(socketAddresses[3]); eag3 = AddressFilter.setPathFilter(eag3, ImmutableList.of("target3")); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of(eag0, eag1, eag2, eag3)) .setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build()) @@ -216,7 +216,7 @@ public void handleResolvedAddresses() { assertThat(barLbCreated).isEqualTo(2); for (int i = 0; i < childBalancers.size(); i++) { - verify(childBalancers.get(i)).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(childBalancers.get(i)).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); ResolvedAddresses resolvedAddresses = resolvedAddressesCaptor.getValue(); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo(configs[i]); assertThat(resolvedAddresses.getAttributes().get(fakeKey)).isEqualTo(fakeValue); @@ -243,7 +243,7 @@ public void handleResolvedAddresses() { "target4", new WeightedPolicySelection( newWeights[3], newChildConfig(fooLbProvider, newConfigs[3]))); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets)) @@ -258,7 +258,7 @@ public void handleResolvedAddresses() { verify(childBalancers.get(0)).shutdown(); for (int i = 1; i < childBalancers.size(); i++) { verify(childBalancers.get(i), atLeastOnce()) - .handleResolvedAddresses(resolvedAddressesCaptor.capture()); + .acceptResolvedAddresses(resolvedAddressesCaptor.capture()); assertThat(resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig()) .isEqualTo(newConfigs[i - 1]); } @@ -286,7 +286,7 @@ public void handleNameResolutionError() { "target2", weightedLbConfig2, // {foo, 40, config3} "target3", weightedLbConfig3); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -313,7 +313,7 @@ public void balancingStateUpdatedFromChildBalancers() { "target2", weightedLbConfig2, // {foo, 40, config3} "target3", weightedLbConfig3); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -395,7 +395,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() { Map targets = ImmutableMap.of( "target0", weightedLbConfig0, "target1", weightedLbConfig1); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -421,7 +421,7 @@ public void noDuplicateOverallBalancingStateUpdate() { weights[0], newChildConfig(fakeLbProvider, configs[0])), "target3", new WeightedPolicySelection( weights[3], newChildConfig(fakeLbProvider, configs[3]))); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -470,9 +470,10 @@ static class FakeLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { helper.updateBalancingState( TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(Status.INTERNAL))); + return Status.OK; } @Override diff --git a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java index fcf8c826d86..1d8bd52cc52 100644 --- a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java @@ -108,7 +108,7 @@ public void setUp() { } @Test - public void handleResolvedAddresses() { + public void acceptResolvedAddresses() { // A two locality cluster with a mock child LB policy. String localityOne = "localityOne"; String localityTwo = "localityTwo"; @@ -124,7 +124,7 @@ public void handleResolvedAddresses() { // Assert that the child policy and the locality weights were correctly mapped to a // WeightedTargetConfig. - verify(mockWeightedTargetLb).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(mockWeightedTargetLb).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); Object config = resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig(); assertThat(config).isInstanceOf(WeightedTargetConfig.class); WeightedTargetConfig wtConfig = (WeightedTargetConfig) config; @@ -182,7 +182,7 @@ public void localityWeightAttributeNotPropagated() { // Assert that the child policy and the locality weights were correctly mapped to a // WeightedTargetConfig. - verify(mockWeightedTargetLb).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(mockWeightedTargetLb).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); //assertThat(resolvedAddressesCaptor.getValue().getAttributes() // .get(InternalXdsAttributes.ATTR_LOCALITY_WEIGHTS)).isNull(); @@ -213,7 +213,7 @@ private Object newChildConfig(LoadBalancerProvider provider, Object config) { } private void deliverAddresses(WrrLocalityConfig config, List addresses) { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(addresses).setLoadBalancingPolicyConfig(config) .build()); } From 6d3abf0fc76704907c3a5fb626759306cfa7014a Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 4 Nov 2024 11:27:58 +0000 Subject: [PATCH 12/21] api: Reverted changes for LoadBalancer and LoadBalancerTest --- api/src/main/java/io/grpc/LoadBalancer.java | 2 +- api/src/test/java/io/grpc/LoadBalancerTest.java | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index a7fd58f0cc3..067f5fc2fa1 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -201,7 +201,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { return unavailableStatus; } else { if (recursionCount++ == 0) { - //handleResolvedAddresses(resolvedAddresses); + handleResolvedAddresses(resolvedAddresses); } recursionCount = 0; diff --git a/api/src/test/java/io/grpc/LoadBalancerTest.java b/api/src/test/java/io/grpc/LoadBalancerTest.java index 091a332f014..485b4cb7aeb 100644 --- a/api/src/test/java/io/grpc/LoadBalancerTest.java +++ b/api/src/test/java/io/grpc/LoadBalancerTest.java @@ -240,9 +240,8 @@ public void handleResolvedAddresses_delegatesToAcceptResolvedAddresses() { LoadBalancer balancer = new LoadBalancer() { @Override - public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { resultCapture.set(resolvedAddresses); - return Status.OK; } @Override @@ -263,7 +262,7 @@ public void shutdown() { new EquivalentAddressGroup(new SocketAddress(){})); ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) .setAttributes(attrs).build(); - balancer.acceptResolvedAddresses(addresses); + balancer.handleResolvedAddresses(addresses); assertThat(resultCapture.get()).isEqualTo( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build()); } @@ -275,9 +274,8 @@ public void acceptResolvedAddresses_delegatesToHandleResolvedAddressGroups() { LoadBalancer balancer = new LoadBalancer() { @Override - public Status acceptResolvedAddresses(ResolvedAddresses addresses) { + public void handleResolvedAddresses(ResolvedAddresses addresses) { addressesCapture.set(addresses); - return Status.OK; } @Override @@ -298,7 +296,7 @@ public void shutdown() { new EquivalentAddressGroup(new SocketAddress(){})); ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) .setAttributes(attrs).build(); - balancer.acceptResolvedAddresses(addresses); + balancer.handleResolvedAddresses(addresses); assertThat(addressesCapture.get().getAddresses()).isEqualTo(servers); assertThat(addressesCapture.get().getAttributes()).isEqualTo(attrs); } @@ -310,10 +308,9 @@ public void acceptResolvedAddresses_noInfiniteLoop() { LoadBalancer balancer = new LoadBalancer() { @Override - public Status acceptResolvedAddresses(ResolvedAddresses addresses) { + public void handleResolvedAddresses(ResolvedAddresses addresses) { addressesCapture.add(addresses); - super.acceptResolvedAddresses(addresses); - return Status.OK; + super.handleResolvedAddresses(addresses); } @Override @@ -330,7 +327,7 @@ public void shutdown() { new EquivalentAddressGroup(new SocketAddress(){})); ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) .setAttributes(attrs).build(); - balancer.acceptResolvedAddresses(addresses); + balancer.handleResolvedAddresses(addresses); assertThat(addressesCapture).hasSize(1); assertThat(addressesCapture.get(0).getAddresses()).isEqualTo(servers); assertThat(addressesCapture.get(0).getAttributes()).isEqualTo(attrs); From 47002fea2ef1d840adecf5aac3d2368abcbd1bd3 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 4 Nov 2024 17:05:50 +0000 Subject: [PATCH 13/21] util: Replaced handleResolvedAddress to acceptResolvedAddresses. --- .../io/grpc/util/GracefulSwitchLoadBalancer.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index 614e1d6d504..c1dd87109f8 100644 --- a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -55,6 +55,19 @@ @NotThreadSafe // Must be accessed in SynchronizationContext public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private final LoadBalancer defaultBalancer = new LoadBalancer() { + @Override + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + // Most LB policies using this class will receive child policy configuration within the + // service config, so they are naturally calling switchTo() just before + // handleResolvedAddresses(), within their own handleResolvedAddresses(). If switchTo() is + // not called immediately after construction that does open up potential for bugs in the + // parent policies, where they fail to call switchTo(). So we will use the exception to try + // to notice those bugs quickly, as it will fail very loudly. + throw new IllegalStateException( + "GracefulSwitchLoadBalancer must switch to a load balancing policy before handling" + + " ResolvedAddresses"); + } + @Override public void handleNameResolutionError(final Status error) { helper.updateBalancingState( From cb85563938c41dac0e21d6dc68f92d105e4bf81d Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Tue, 5 Nov 2024 11:34:16 +0000 Subject: [PATCH 14/21] api: Reverted back to acceptResolvedAddress --- api/src/test/java/io/grpc/LoadBalancerTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/test/java/io/grpc/LoadBalancerTest.java b/api/src/test/java/io/grpc/LoadBalancerTest.java index 485b4cb7aeb..5e9e5cbe816 100644 --- a/api/src/test/java/io/grpc/LoadBalancerTest.java +++ b/api/src/test/java/io/grpc/LoadBalancerTest.java @@ -240,8 +240,9 @@ public void handleResolvedAddresses_delegatesToAcceptResolvedAddresses() { LoadBalancer balancer = new LoadBalancer() { @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { resultCapture.set(resolvedAddresses); + return Status.OK; } @Override From 1e6fef9983bd8496f26e607c1618eb89529e330e Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 25 Nov 2024 10:04:44 +0000 Subject: [PATCH 15/21] api/util: Addressed the re-review points --- api/src/main/java/io/grpc/LoadBalancer.java | 8 +++-- .../io/grpc/util/ForwardingLoadBalancer.java | 10 ++++++ .../grpc/util/GracefulSwitchLoadBalancer.java | 26 +++++++++++++- .../io/grpc/util/MultiChildLoadBalancer.java | 2 -- .../util/OutlierDetectionLoadBalancer.java | 3 +- .../util/GracefulSwitchLoadBalancerTest.java | 36 +++++++++---------- 6 files changed, 59 insertions(+), 26 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index 88c1d9fcd6f..96813e9edf0 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -157,13 +157,15 @@ public String toString() { /** * Handles newly resolved addresses and metadata attributes from name resolution system. - * {@link EquivalentAddressGroup} addresses should be considered equivalent but may be flattened + * Addresses in {@link EquivalentAddressGroup} should be considered equivalent but may be flattened * into a single list if needed. * - *

Implementations should not modify the given {@code resolvedAddresses}. - * * @param resolvedAddresses the resolved server addresses, attributes, and config. * @since 1.21.0 + * + * @deprecated As of release 1.69.0, + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + * */ @Deprecated public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { diff --git a/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java b/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java index 8112e55fbbc..84fc109cad3 100644 --- a/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java @@ -29,6 +29,16 @@ public abstract class ForwardingLoadBalancer extends LoadBalancer { */ protected abstract LoadBalancer delegate(); + /** + * @deprecated As of release 1.69.0, + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + */ + @Deprecated + @Override + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + delegate().handleResolvedAddresses(resolvedAddresses); + } + @Override public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { return delegate().acceptResolvedAddresses(resolvedAddresses); diff --git a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index c1dd87109f8..271c78f45bf 100644 --- a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -56,7 +56,7 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private final LoadBalancer defaultBalancer = new LoadBalancer() { @Override - public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { // Most LB policies using this class will receive child policy configuration within the // service config, so they are naturally calling switchTo() just before // handleResolvedAddresses(), within their own handleResolvedAddresses(). If switchTo() is @@ -112,6 +112,30 @@ public GracefulSwitchLoadBalancer(Helper helper) { this.helper = checkNotNull(helper, "helper"); } + /** + * @deprecated As of release 1.69.0, + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + */ + @Deprecated + @Override + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + if (switchToCalled) { + delegate().handleResolvedAddresses(resolvedAddresses); + return; + } + Config config = (Config) resolvedAddresses.getLoadBalancingPolicyConfig(); + switchToInternal(config.childFactory); + delegate().handleResolvedAddresses( + resolvedAddresses.toBuilder() + .setLoadBalancingPolicyConfig(config.childConfig) + .build()); + } + + /** + * + * @param resolvedAddresses the resolved server addresses, attributes, and config. + * @return + */ @Override public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (switchToCalled) { diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java index 36cf2f0542c..b51d2772d3e 100644 --- a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java @@ -192,8 +192,6 @@ private List updateChildrenWithResolvedAddresses( newChildLbStates.add(childLbState); if (entry.getValue() != null) { childLbState.lb.handleResolvedAddresses(entry.getValue()); // update child LB - childLbState.setResolvedAddresses(entry.getValue()); // update child - childLbState.lb.acceptResolvedAddresses(entry.getValue()); // update child LB } } diff --git a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java index 080faba578a..2eff70282f3 100644 --- a/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java @@ -171,9 +171,8 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { endpointTrackerMap.cancelTracking(); } - switchLb.acceptResolvedAddresses( + return switchLb.acceptResolvedAddresses( resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(config.childConfig).build()); - return Status.OK; } @Override diff --git a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index 1c87091a192..f31443ace7b 100644 --- a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -125,17 +125,17 @@ public void switchTo_handleResolvedAddressesAndNameResolutionErrorForwardedToLat helper0.updateBalancingState(READY, picker); ResolvedAddresses addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses); - verify(lb0).acceptResolvedAddresses(addresses); + gracefulSwitchLb.handleResolvedAddresses(addresses); + verify(lb0).handleResolvedAddresses(addresses); gracefulSwitchLb.handleNameResolutionError(Status.DATA_LOSS); verify(lb0).handleNameResolutionError(Status.DATA_LOSS); gracefulSwitchLb.switchTo(lbPolicies[1]); LoadBalancer lb1 = balancers.get(lbPolicies[1]); addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses); - verify(lb0, never()).acceptResolvedAddresses(addresses); - verify(lb1).acceptResolvedAddresses(addresses); + gracefulSwitchLb.handleResolvedAddresses(addresses); + verify(lb0, never()).handleResolvedAddresses(addresses); + verify(lb1).handleResolvedAddresses(addresses); gracefulSwitchLb.handleNameResolutionError(Status.ALREADY_EXISTS); verify(lb0, never()).handleNameResolutionError(Status.ALREADY_EXISTS); verify(lb1).handleNameResolutionError(Status.ALREADY_EXISTS); @@ -144,10 +144,10 @@ public void switchTo_handleResolvedAddressesAndNameResolutionErrorForwardedToLat verify(lb1).shutdown(); LoadBalancer lb2 = balancers.get(lbPolicies[2]); addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses); - verify(lb0, never()).acceptResolvedAddresses(addresses); - verify(lb1, never()).acceptResolvedAddresses(addresses); - verify(lb2).acceptResolvedAddresses(addresses); + gracefulSwitchLb.handleResolvedAddresses(addresses); + verify(lb0, never()).handleResolvedAddresses(addresses); + verify(lb1, never()).handleResolvedAddresses(addresses); + verify(lb2).handleResolvedAddresses(addresses); gracefulSwitchLb.handleNameResolutionError(Status.CANCELLED); verify(lb0, never()).handleNameResolutionError(Status.CANCELLED); verify(lb1, never()).handleNameResolutionError(Status.CANCELLED); @@ -591,11 +591,11 @@ public void canHandleEmptyAddressListFromNameResolutionForwardedToLatestPolicy() public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { ResolvedAddresses addresses = newFakeAddresses(); Object child0Config = new Object(); - gracefulSwitchLb.acceptResolvedAddresses(addresses.toBuilder() + gracefulSwitchLb.handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(createConfig(lbPolicies[0], child0Config)) .build()); LoadBalancer lb0 = balancers.get(lbPolicies[0]); - verify(lb0).acceptResolvedAddresses(addresses.toBuilder() + verify(lb0).handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child0Config) .build()); Helper helper0 = helpers.get(lb0); @@ -606,14 +606,14 @@ public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy Object child1Config = new Object(); addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses.toBuilder() + gracefulSwitchLb.handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(createConfig(lbPolicies[1], child1Config)) .build()); LoadBalancer lb1 = balancers.get(lbPolicies[1]); - verify(lb0, never()).acceptResolvedAddresses(addresses.toBuilder() + verify(lb0, never()).handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child1Config) .build()); - verify(lb1).acceptResolvedAddresses(addresses.toBuilder() + verify(lb1).handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child1Config) .build()); gracefulSwitchLb.handleNameResolutionError(Status.ALREADY_EXISTS); @@ -622,18 +622,18 @@ public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy Object child2Config = new Object(); addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses.toBuilder() + gracefulSwitchLb.handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(createConfig(lbPolicies[2], child2Config)) .build()); verify(lb1).shutdown(); LoadBalancer lb2 = balancers.get(lbPolicies[2]); - verify(lb0, never()).acceptResolvedAddresses(addresses.toBuilder() + verify(lb0, never()).handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child2Config) .build()); - verify(lb1, never()).acceptResolvedAddresses(addresses.toBuilder() + verify(lb1, never()).handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child2Config) .build()); - verify(lb2).acceptResolvedAddresses(addresses.toBuilder() + verify(lb2).handleResolvedAddresses(addresses.toBuilder() .setLoadBalancingPolicyConfig(child2Config) .build()); gracefulSwitchLb.handleNameResolutionError(Status.CANCELLED); From 1674b6dd73763810f331c45d604522e7a4420aff Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 25 Nov 2024 12:03:32 +0000 Subject: [PATCH 16/21] api/util: Resolved Deprecation warnings --- .../java/io/grpc/util/GracefulSwitchLoadBalancer.java | 10 +++++----- .../main/java/io/grpc/util/MultiChildLoadBalancer.java | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index 271c78f45bf..248e595abeb 100644 --- a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -55,6 +55,11 @@ @NotThreadSafe // Must be accessed in SynchronizationContext public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private final LoadBalancer defaultBalancer = new LoadBalancer() { + /** + * @deprecated As of release 1.69.0, + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + */ + @Deprecated @Override public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { // Most LB policies using this class will receive child policy configuration within the @@ -131,11 +136,6 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { .build()); } - /** - * - * @param resolvedAddresses the resolved server addresses, attributes, and config. - * @return - */ @Override public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (switchToCalled) { diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java index b51d2772d3e..8b80440f994 100644 --- a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java @@ -191,7 +191,7 @@ private List updateChildrenWithResolvedAddresses( } newChildLbStates.add(childLbState); if (entry.getValue() != null) { - childLbState.lb.handleResolvedAddresses(entry.getValue()); // update child LB + childLbState.lb.acceptResolvedAddresses(entry.getValue()); // update child LB } } From 60b4a391c2b21b829a6b5469f02e28ff0c108322 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Mon, 25 Nov 2024 14:43:25 +0000 Subject: [PATCH 17/21] api/util: Resolved checkstyle issues --- api/src/main/java/io/grpc/LoadBalancer.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index 96813e9edf0..3e1141b36f6 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -157,15 +157,14 @@ public String toString() { /** * Handles newly resolved addresses and metadata attributes from name resolution system. - * Addresses in {@link EquivalentAddressGroup} should be considered equivalent but may be flattened - * into a single list if needed. + * Addresses in {@link EquivalentAddressGroup} should be considered equivalent but may be + * flattened into a single list if needed. * * @param resolvedAddresses the resolved server addresses, attributes, and config. * @since 1.21.0 * - * @deprecated As of release 1.69.0, - * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} - * + * @deprecated As of release 1.69.0, use instead + * {@link #acceptResolvedAddresses(ResolvedAddresses)} */ @Deprecated public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { From 8b02cd4630a36cea09055c5062f2a20ccbd4a355 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Tue, 26 Nov 2024 07:35:01 +0000 Subject: [PATCH 18/21] api/util: Resolved deprecation warnings --- .../main/java/io/grpc/util/ForwardingLoadBalancer.java | 4 +++- .../java/io/grpc/util/GracefulSwitchLoadBalancer.java | 8 ++++++-- .../java/io/grpc/util/GracefulSwitchLoadBalancerTest.java | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java b/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java index 84fc109cad3..8a823ae5470 100644 --- a/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/ForwardingLoadBalancer.java @@ -30,8 +30,10 @@ public abstract class ForwardingLoadBalancer extends LoadBalancer { protected abstract LoadBalancer delegate(); /** + * Handles newly resolved addresses and metadata attributes from name resolution system. + * * @deprecated As of release 1.69.0, - * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} */ @Deprecated @Override diff --git a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index 248e595abeb..ba51aa92512 100644 --- a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -56,8 +56,10 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private final LoadBalancer defaultBalancer = new LoadBalancer() { /** + * Handles newly resolved addresses and metadata attributes from name resolution system. + * * @deprecated As of release 1.69.0, - * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} */ @Deprecated @Override @@ -118,8 +120,10 @@ public GracefulSwitchLoadBalancer(Helper helper) { } /** + * Handles newly resolved addresses and metadata attributes from name resolution system. + * * @deprecated As of release 1.69.0, - * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} */ @Deprecated @Override diff --git a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index f31443ace7b..0e1b1cc849b 100644 --- a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -587,6 +587,7 @@ public void canHandleEmptyAddressListFromNameResolutionForwardedToLatestPolicy() assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); } + @Deprecated @Test public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { ResolvedAddresses addresses = newFakeAddresses(); From 516e75bcb014f30ff7516205f42f360b62a168ed Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Tue, 26 Nov 2024 10:49:44 +0000 Subject: [PATCH 19/21] util/xds: Resolved deprecation warnings --- .../util/GracefulSwitchLoadBalancerTest.java | 1 - .../xds/MetadataLoadBalancerProvider.java | 11 +++- .../io/grpc/xds/PriorityLoadBalancerTest.java | 52 +++++++++++-------- .../xds/WeightedTargetLoadBalancerTest.java | 27 ++++++---- .../grpc/xds/WrrLocalityLoadBalancerTest.java | 11 ++-- 5 files changed, 63 insertions(+), 39 deletions(-) diff --git a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index 0e1b1cc849b..f31443ace7b 100644 --- a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -587,7 +587,6 @@ public void canHandleEmptyAddressListFromNameResolutionForwardedToLatestPolicy() assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); } - @Deprecated @Test public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { ResolvedAddresses addresses = newFakeAddresses(); diff --git a/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java b/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java index 45f2337b746..f148cb86000 100644 --- a/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java +++ b/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java @@ -107,12 +107,19 @@ protected LoadBalancer delegate() { return delegateLb; } + /** + * Handles newly resolved addresses and metadata attributes from name resolution system. + * + * @deprecated As of release 1.69.0, + * use instead {@link #acceptResolvedAddresses(ResolvedAddresses)} + */ + @Deprecated @Override - public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { MetadataLoadBalancerConfig config = (MetadataLoadBalancerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); helper.setMetadata(config.metadataKey, config.metadataValue); - return delegateLb.acceptResolvedAddresses(resolvedAddresses); + delegateLb.handleResolvedAddresses(resolvedAddresses); } } diff --git a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java index a79af4cb66c..4a5970126d5 100644 --- a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java @@ -140,8 +140,9 @@ public void tearDown() { assertThat(fakeClock.getPendingTasks()).isEmpty(); } + @Deprecated @Test - public void acceptResolvedAddresses() { + public void handleResolvedAddresses() { SocketAddress socketAddress = new InetSocketAddress(8080); EquivalentAddressGroup eag = new EquivalentAddressGroup(socketAddress); eag = AddressFilter.setPathFilter(eag, ImmutableList.of("p1")); @@ -162,7 +163,7 @@ public void acceptResolvedAddresses() { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2), ImmutableList.of("p0", "p1", "p2")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(addresses) .setAttributes(attributes) @@ -171,7 +172,7 @@ public void acceptResolvedAddresses() { assertThat(fooBalancers).hasSize(1); assertThat(barBalancers).isEmpty(); LoadBalancer fooBalancer0 = Iterables.getOnlyElement(fooBalancers); - verify(fooBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(fooBalancer0).handleResolvedAddresses(resolvedAddressesCaptor.capture()); ResolvedAddresses addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(addressesReceived.getAddresses()).isEmpty(); assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); @@ -182,7 +183,7 @@ public void acceptResolvedAddresses() { assertThat(fooBalancers).hasSize(1); assertThat(barBalancers).hasSize(1); LoadBalancer barBalancer0 = Iterables.getOnlyElement(barBalancers); - verify(barBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(barBalancer0).handleResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) .containsExactly(socketAddress); @@ -194,7 +195,7 @@ public void acceptResolvedAddresses() { assertThat(fooBalancers).hasSize(2); assertThat(barBalancers).hasSize(1); LoadBalancer fooBalancer1 = Iterables.getLast(fooBalancers); - verify(fooBalancer1).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(fooBalancer1).handleResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(addressesReceived.getAddresses()).isEmpty(); assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); @@ -211,14 +212,14 @@ public void acceptResolvedAddresses() { ImmutableMap.of("p1", new PriorityChildConfig(newChildConfig(barLbProvider, newBarConfig), true)), ImmutableList.of("p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(newAddresses) .setLoadBalancingPolicyConfig(newPriorityLbConfig) .build()); assertThat(fooBalancers).hasSize(2); assertThat(barBalancers).hasSize(1); - verify(barBalancer0, times(2)).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(barBalancer0, times(2)).handleResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) .containsExactly(newSocketAddress); @@ -232,6 +233,7 @@ public void acceptResolvedAddresses() { verify(barBalancer0, never()).shutdown(); } + @Deprecated @Test public void handleNameResolutionError() { Object fooConfig0 = new Object(); @@ -243,7 +245,7 @@ public void handleNameResolutionError() { PriorityLbConfig priorityLbConfig = new PriorityLbConfig(ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -255,7 +257,7 @@ public void handleNameResolutionError() { priorityLbConfig = new PriorityLbConfig(ImmutableMap.of("p1", priorityChildConfig1), ImmutableList.of("p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -271,6 +273,7 @@ public void handleNameResolutionError() { verify(fooLb1).handleNameResolutionError(status); } + @Deprecated @Test public void typicalPriorityFailOverFlow() { PriorityChildConfig priorityChildConfig0 = @@ -286,7 +289,7 @@ public void typicalPriorityFailOverFlow() { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2, "p3", priorityChildConfig3), ImmutableList.of("p0", "p1", "p2", "p3")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -409,6 +412,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) { verify(balancer3).shutdown(); } + @Deprecated @Test public void idleToConnectingDoesNotTriggerFailOver() { PriorityChildConfig priorityChildConfig0 = @@ -419,7 +423,7 @@ public void idleToConnectingDoesNotTriggerFailOver() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -445,6 +449,7 @@ public void idleToConnectingDoesNotTriggerFailOver() { assertThat(fooHelpers).hasSize(1); } + @Deprecated @Test public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() { PriorityChildConfig priorityChildConfig0 = @@ -455,7 +460,7 @@ public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -487,6 +492,7 @@ public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() { assertThat(fooHelpers).hasSize(2); } + @Deprecated @Test public void readyToConnectDoesNotFailOverButUpdatesPicker() { PriorityChildConfig priorityChildConfig0 = @@ -497,7 +503,7 @@ public void readyToConnectDoesNotFailOverButUpdatesPicker() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -530,7 +536,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) { // resolution update without priority change does not trigger failover Attributes.Key fooKey = Attributes.Key.create("fooKey"); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -544,6 +550,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) { assertThat(fooHelpers).hasSize(1); } + @Deprecated @Test public void typicalPriorityFailOverFlowWithIdleUpdate() { PriorityChildConfig priorityChildConfig0 = @@ -559,7 +566,7 @@ public void typicalPriorityFailOverFlowWithIdleUpdate() { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2, "p3", priorityChildConfig3), ImmutableList.of("p0", "p1", "p2", "p3")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -652,6 +659,7 @@ public void typicalPriorityFailOverFlowWithIdleUpdate() { verify(balancer3).shutdown(); } + @Deprecated @Test public void bypassReresolutionRequestsIfConfiged() { PriorityChildConfig priorityChildConfig0 = @@ -662,7 +670,7 @@ public void bypassReresolutionRequestsIfConfiged() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -680,6 +688,7 @@ public void bypassReresolutionRequestsIfConfiged() { verify(helper).refreshNameResolution(); } + @Deprecated @Test public void raceBetweenShutdownAndChildLbBalancingStateUpdate() { PriorityChildConfig priorityChildConfig0 = @@ -690,7 +699,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -705,6 +714,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() { verifyNoMoreInteractions(helper); } + @Deprecated @Test public void noDuplicateOverallBalancingStateUpdate() { FakeLoadBalancerProvider fakeLbProvider = new FakeLoadBalancerProvider(); @@ -717,7 +727,7 @@ public void noDuplicateOverallBalancingStateUpdate() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -727,7 +737,7 @@ public void noDuplicateOverallBalancingStateUpdate() { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.acceptResolvedAddresses( + priorityLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -800,11 +810,11 @@ static class FakeLoadBalancer extends LoadBalancer { this.helper = helper; } + @Deprecated @Override - public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { helper.updateBalancingState( TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(Status.INTERNAL))); - return Status.OK; } @Override diff --git a/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java index 46fe3881167..67cc6d3bc9f 100644 --- a/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java @@ -179,8 +179,9 @@ public void tearDown() { } } + @Deprecated @Test - public void acceptResolvedAddresses() { + public void handleResolvedAddresses() { ArgumentCaptor resolvedAddressesCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); Attributes.Key fakeKey = Attributes.Key.create("fake_key"); @@ -203,7 +204,7 @@ public void acceptResolvedAddresses() { eag2 = AddressFilter.setPathFilter(eag2, ImmutableList.of("target2")); EquivalentAddressGroup eag3 = new EquivalentAddressGroup(socketAddresses[3]); eag3 = AddressFilter.setPathFilter(eag3, ImmutableList.of("target3")); - weightedTargetLb.acceptResolvedAddresses( + weightedTargetLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of(eag0, eag1, eag2, eag3)) .setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build()) @@ -216,7 +217,7 @@ public void acceptResolvedAddresses() { assertThat(barLbCreated).isEqualTo(2); for (int i = 0; i < childBalancers.size(); i++) { - verify(childBalancers.get(i)).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(childBalancers.get(i)).handleResolvedAddresses(resolvedAddressesCaptor.capture()); ResolvedAddresses resolvedAddresses = resolvedAddressesCaptor.getValue(); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo(configs[i]); assertThat(resolvedAddresses.getAttributes().get(fakeKey)).isEqualTo(fakeValue); @@ -243,7 +244,7 @@ public void acceptResolvedAddresses() { "target4", new WeightedPolicySelection( newWeights[3], newChildConfig(fooLbProvider, newConfigs[3]))); - weightedTargetLb.acceptResolvedAddresses( + weightedTargetLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets)) @@ -258,12 +259,13 @@ public void acceptResolvedAddresses() { verify(childBalancers.get(0)).shutdown(); for (int i = 1; i < childBalancers.size(); i++) { verify(childBalancers.get(i), atLeastOnce()) - .acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + .handleResolvedAddresses(resolvedAddressesCaptor.capture()); assertThat(resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig()) .isEqualTo(newConfigs[i - 1]); } } + @Deprecated @Test public void handleNameResolutionError() { ArgumentCaptor pickerCaptor = ArgumentCaptor.forClass(SubchannelPicker.class); @@ -286,7 +288,7 @@ public void handleNameResolutionError() { "target2", weightedLbConfig2, // {foo, 40, config3} "target3", weightedLbConfig3); - weightedTargetLb.acceptResolvedAddresses( + weightedTargetLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -302,6 +304,7 @@ public void handleNameResolutionError() { } } + @Deprecated @Test public void balancingStateUpdatedFromChildBalancers() { Map targets = ImmutableMap.of( @@ -313,7 +316,7 @@ public void balancingStateUpdatedFromChildBalancers() { "target2", weightedLbConfig2, // {foo, 40, config3} "target3", weightedLbConfig3); - weightedTargetLb.acceptResolvedAddresses( + weightedTargetLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -390,12 +393,13 @@ public void balancingStateUpdatedFromChildBalancers() { new WeightedChildPicker(weights[3], failurePickers[3])); } + @Deprecated @Test public void raceBetweenShutdownAndChildLbBalancingStateUpdate() { Map targets = ImmutableMap.of( "target0", weightedLbConfig0, "target1", weightedLbConfig1); - weightedTargetLb.acceptResolvedAddresses( + weightedTargetLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -412,6 +416,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() { // When the ChildHelper is asked to update the overall balancing state, it should not do that if // the update was triggered by the parent LB that will handle triggering the overall state update. + @Deprecated @Test public void noDuplicateOverallBalancingStateUpdate() { FakeLoadBalancerProvider fakeLbProvider = new FakeLoadBalancerProvider(); @@ -421,7 +426,7 @@ public void noDuplicateOverallBalancingStateUpdate() { weights[0], newChildConfig(fakeLbProvider, configs[0])), "target3", new WeightedPolicySelection( weights[3], newChildConfig(fakeLbProvider, configs[3]))); - weightedTargetLb.acceptResolvedAddresses( + weightedTargetLb.handleResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -469,11 +474,11 @@ static class FakeLoadBalancer extends LoadBalancer { this.helper = helper; } + @Deprecated @Override - public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { helper.updateBalancingState( TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(Status.INTERNAL))); - return Status.OK; } @Override diff --git a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java index 1d8bd52cc52..43746a75373 100644 --- a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java @@ -107,8 +107,9 @@ public void setUp() { loadBalancer = new WrrLocalityLoadBalancer(mockHelper, lbRegistry); } + @Deprecated @Test - public void acceptResolvedAddresses() { + public void handleResolvedAddresses() { // A two locality cluster with a mock child LB policy. String localityOne = "localityOne"; String localityTwo = "localityTwo"; @@ -124,7 +125,7 @@ public void acceptResolvedAddresses() { // Assert that the child policy and the locality weights were correctly mapped to a // WeightedTargetConfig. - verify(mockWeightedTargetLb).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(mockWeightedTargetLb).handleResolvedAddresses(resolvedAddressesCaptor.capture()); Object config = resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig(); assertThat(config).isInstanceOf(WeightedTargetConfig.class); WeightedTargetConfig wtConfig = (WeightedTargetConfig) config; @@ -173,6 +174,7 @@ public void handleNameResolutionError_withChildLb() { verify(mockWeightedTargetLb).handleNameResolutionError(status); } + @Deprecated @Test public void localityWeightAttributeNotPropagated() { Object childPolicy = newChildConfig(mockChildProvider, null); @@ -182,7 +184,7 @@ public void localityWeightAttributeNotPropagated() { // Assert that the child policy and the locality weights were correctly mapped to a // WeightedTargetConfig. - verify(mockWeightedTargetLb).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(mockWeightedTargetLb).handleResolvedAddresses(resolvedAddressesCaptor.capture()); //assertThat(resolvedAddressesCaptor.getValue().getAttributes() // .get(InternalXdsAttributes.ATTR_LOCALITY_WEIGHTS)).isNull(); @@ -212,8 +214,9 @@ private Object newChildConfig(LoadBalancerProvider provider, Object config) { return GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(provider, config); } + @Deprecated private void deliverAddresses(WrrLocalityConfig config, List addresses) { - loadBalancer.acceptResolvedAddresses( + loadBalancer.handleResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(addresses).setLoadBalancingPolicyConfig(config) .build()); } From 36ecadbc52ecc9147006d59f3c5b9c590d3b3628 Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Tue, 26 Nov 2024 11:09:58 +0000 Subject: [PATCH 20/21] util: Resolved deprecation warnings --- .../test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index f31443ace7b..0e1b1cc849b 100644 --- a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -587,6 +587,7 @@ public void canHandleEmptyAddressListFromNameResolutionForwardedToLatestPolicy() assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); } + @Deprecated @Test public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { ResolvedAddresses addresses = newFakeAddresses(); From d2e6f04d9e7290b19333cf39ebd87906cd2627dd Mon Sep 17 00:00:00 2001 From: SreeramdasLavanya Date: Tue, 3 Dec 2024 17:06:55 +0000 Subject: [PATCH 21/21] util: Included suppress warnings for updateChildrenWithResolvedAddresses method --- util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java index 8b80440f994..67b48d6d913 100644 --- a/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java @@ -173,6 +173,7 @@ protected final AcceptResolvedAddrRetVal acceptResolvedAddressesInternal( } /** Returns removed children. */ + @SuppressWarnings("deprecation") private List updateChildrenWithResolvedAddresses( Map newChildAddresses) { // Create a map with the old values @@ -191,7 +192,8 @@ private List updateChildrenWithResolvedAddresses( } newChildLbStates.add(childLbState); if (entry.getValue() != null) { - childLbState.lb.acceptResolvedAddresses(entry.getValue()); // update child LB + //TODO - https://github.com/grpc/grpc-java/issues/11194 + childLbState.lb.handleResolvedAddresses(entry.getValue()); // update child LB } }