From 7bf0fa260296456ba7a9af1475d9bc6943d03aa4 Mon Sep 17 00:00:00 2001 From: Bohan Yang Date: Fri, 9 Aug 2024 15:32:27 -0700 Subject: [PATCH] remove some dual read related tests and deprecate dual read LB --- .../dualread/DualReadLoadBalancer.java | 1 + .../d2/balancer/util/WarmUpLoadBalancer.java | 7 ++++++- .../DualReadZkAndXdsLoadBalancerFactory.java | 1 + .../balancer/util/WarmUpLoadBalancerTest.java | 19 +++++++++++-------- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java b/d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java index 59bd12d149..c9a0c9a6e4 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/dualread/DualReadLoadBalancer.java @@ -58,6 +58,7 @@ * In DUAL_READ mode, it reads from both the old and the new load balancer, but relies on the data from old * load balancer only. */ +@Deprecated public class DualReadLoadBalancer implements LoadBalancerWithFacilities { private static final Logger LOG = LoggerFactory.getLogger(DualReadLoadBalancer.class); diff --git a/d2/src/main/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancer.java b/d2/src/main/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancer.java index 0c2e905fd1..19f82a7b2d 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancer.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancer.java @@ -398,7 +398,7 @@ private static boolean isModeToWarmUp(DualReadModeProvider.DualReadMode mode, bo public void shutdown(PropertyEventThread.PropertyEventShutdownCallback shutdown) { // avoid cleaning when you risk to have partial results since some of the services have not loaded yet - if (_outstandingRequests.size() == 0) + if (completedOutStandingRequests()) { // cleanup from unused services FileSystemDirectory fsDirectory = new FileSystemDirectory(_d2FsDirPath, _d2ServicePath); @@ -412,6 +412,11 @@ public void shutdown(PropertyEventThread.PropertyEventShutdownCallback shutdown) _loadBalancer.shutdown(shutdown); } + boolean completedOutStandingRequests() + { + return _outstandingRequests.isEmpty(); + } + private Set getUsedClusters() { Set usedClusters = new HashSet<>(); diff --git a/d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java b/d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java index 5f96357144..b72c5102fe 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java +++ b/d2/src/main/java/com/linkedin/d2/xds/balancer/DualReadZkAndXdsLoadBalancerFactory.java @@ -28,6 +28,7 @@ * discovery data sources: direct ZooKeeper data and xDS data. The {@link DualReadModeProvider} will * determine dynamically at run-time which read mode to use. */ +@Deprecated public class DualReadZkAndXdsLoadBalancerFactory implements LoadBalancerWithFacilitiesFactory { private final LoadBalancerWithFacilitiesFactory _zkLbFactory; diff --git a/d2/src/test/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancerTest.java b/d2/src/test/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancerTest.java index d763bac048..36d8aabb3b 100644 --- a/d2/src/test/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancerTest.java +++ b/d2/src/test/java/com/linkedin/d2/balancer/util/WarmUpLoadBalancerTest.java @@ -44,6 +44,7 @@ import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; +import org.testng.annotations.Ignore; import org.testng.annotations.Test; import static com.linkedin.d2.balancer.dualread.DualReadModeProvider.DualReadMode.*; @@ -127,7 +128,7 @@ public void testMakingWarmUpRequests() throws URISyntaxException, InterruptedExc Assert.assertEquals(VALID_FILES.size(), requestCount.get()); } - @Test(timeOut = 10000, retryAnalyzer = ThreeRetries.class) + @Test(timeOut = 10_000) public void testDeletingFilesAfterShutdown() throws InterruptedException, ExecutionException, TimeoutException { createDefaultServicesIniFiles(); @@ -138,7 +139,7 @@ public void testDeletingFilesAfterShutdown() throws InterruptedException, Execut DownstreamServicesFetcher returnPartialDownstreams = callback -> callback.onSuccess(partialServices); - LoadBalancer warmUpLoadBalancer = new WarmUpLoadBalancer(balancer, balancer, Executors.newSingleThreadScheduledExecutor(), + WarmUpLoadBalancer warmUpLoadBalancer = new WarmUpLoadBalancer(balancer, balancer, Executors.newSingleThreadScheduledExecutor(), _tmpdir.getAbsolutePath(), MY_SERVICES_FS, returnPartialDownstreams, WarmUpLoadBalancer.DEFAULT_SEND_REQUESTS_TIMEOUT_SECONDS, WarmUpLoadBalancer.DEFAULT_CONCURRENT_REQUESTS); @@ -157,9 +158,11 @@ public void testDeletingFilesAfterShutdown() throws InterruptedException, Execut "After shutdown the unused services should have been deleted. Expected lower number of:" + allServicesBeforeShutdown.size() + ", actual " + partialServices.size()); - Assert.assertTrue(partialServices.containsAll(allServicesAfterShutdown) - && allServicesAfterShutdown.containsAll(partialServices), - "There should be just the services that were passed by the partial fetcher"); + if (warmUpLoadBalancer.completedOutStandingRequests()) { + Assert.assertTrue(partialServices.containsAll(allServicesAfterShutdown) + && allServicesAfterShutdown.containsAll(partialServices), + "There should be just the services that were passed by the partial fetcher"); + } } /** @@ -373,7 +376,7 @@ public Object[][] modesToWarmUpDataProvider() }; } - @Test(dataProvider = "modesToWarmUpDataProvider", retryAnalyzer = ThreeRetries.class) + @Ignore("dual read is only used in INDIS migration phase and should be deprecated") public void testSuccessWithDualRead(DualReadModeProvider.DualReadMode mode, Boolean isIndis) throws InterruptedException, ExecutionException, TimeoutException { @@ -399,7 +402,7 @@ public void testSuccessWithDualRead(DualReadModeProvider.DualReadMode mode, Bool Assert.assertEquals(completedWarmUpCount.get(), VALID_FILES.size()); } - @Test(dataProvider = "modesToWarmUpDataProvider", retryAnalyzer = ThreeRetries.class) + @Ignore("dual read is only used in INDIS migration phase and should be deprecated") public void testDualReadHitTimeout(DualReadModeProvider.DualReadMode mode, Boolean isIndis) throws InterruptedException, ExecutionException, TimeoutException { @@ -425,7 +428,7 @@ public void testDualReadHitTimeout(DualReadModeProvider.DualReadMode mode, Boole Assert.assertEquals(completedWarmUpCount.get(), 0); } - @Test(dataProvider = "modesToWarmUpDataProvider", retryAnalyzer = ThreeRetries.class) + @Ignore("dual read is only used in INDIS migration phase and should be deprecated") public void testDualReadCompleteWarmUpHitTimeout(DualReadModeProvider.DualReadMode mode, Boolean isIndis) throws InterruptedException, ExecutionException, TimeoutException {