From dfa722448a1cbf7bef31c4a9798e745b7ba394d0 Mon Sep 17 00:00:00 2001 From: Himanshu Date: Mon, 9 Mar 2020 12:13:59 -0700 Subject: [PATCH] remove ServerDiscoverySelector from DruidLeaderClient --- .../druid/discovery/DruidLeaderClient.java | 19 +---------------- .../guice/CoordinatorDiscoveryModule.java | 19 ++--------------- .../guice/IndexingServiceDiscoveryModule.java | 19 ++--------------- .../discovery/DruidLeaderClientTest.java | 21 ++++++------------- .../coordinator/duty/CompactSegmentsTest.java | 2 +- .../druid/sql/calcite/util/CalciteTests.java | 5 +---- 6 files changed, 13 insertions(+), 72 deletions(-) diff --git a/server/src/main/java/org/apache/druid/discovery/DruidLeaderClient.java b/server/src/main/java/org/apache/druid/discovery/DruidLeaderClient.java index 2b107bbac3a7..0679ac39e4ac 100644 --- a/server/src/main/java/org/apache/druid/discovery/DruidLeaderClient.java +++ b/server/src/main/java/org/apache/druid/discovery/DruidLeaderClient.java @@ -21,8 +21,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Throwables; -import org.apache.druid.client.selector.DiscoverySelector; -import org.apache.druid.client.selector.Server; import org.apache.druid.concurrent.LifecycleLock; import org.apache.druid.java.util.common.IOE; import org.apache.druid.java.util.common.ISE; @@ -71,9 +69,6 @@ public class DruidLeaderClient private final String leaderRequestPath; - //Note: This is kept for back compatibility with pre 0.11.0 releases and should be removed in future. - private final DiscoverySelector serverDiscoverySelector; - private LifecycleLock lifecycleLock = new LifecycleLock(); private DruidNodeDiscovery druidNodeDiscovery; private AtomicReference currentKnownLeader = new AtomicReference<>(); @@ -82,15 +77,13 @@ public DruidLeaderClient( HttpClient httpClient, DruidNodeDiscoveryProvider druidNodeDiscoveryProvider, NodeRole nodeRoleToWatch, - String leaderRequestPath, - DiscoverySelector serverDiscoverySelector + String leaderRequestPath ) { this.httpClient = httpClient; this.druidNodeDiscoveryProvider = druidNodeDiscoveryProvider; this.nodeRoleToWatch = nodeRoleToWatch; this.leaderRequestPath = leaderRequestPath; - this.serverDiscoverySelector = serverDiscoverySelector; } @LifecycleStart @@ -280,16 +273,6 @@ private String getCurrentKnownLeader(final boolean cached) throws IOException @Nullable private String pickOneHost() { - Server server = serverDiscoverySelector.pick(); - if (server != null) { - return StringUtils.format( - "%s://%s:%s", - server.getScheme(), - server.getAddress(), - server.getPort() - ); - } - Iterator iter = druidNodeDiscovery.getAllNodes().iterator(); if (iter.hasNext()) { DiscoveryDruidNode node = iter.next(); diff --git a/server/src/main/java/org/apache/druid/guice/CoordinatorDiscoveryModule.java b/server/src/main/java/org/apache/druid/guice/CoordinatorDiscoveryModule.java index b90a9b5e76ff..d9f083767df6 100644 --- a/server/src/main/java/org/apache/druid/guice/CoordinatorDiscoveryModule.java +++ b/server/src/main/java/org/apache/druid/guice/CoordinatorDiscoveryModule.java @@ -24,8 +24,6 @@ import com.google.inject.Provides; import org.apache.druid.client.coordinator.Coordinator; import org.apache.druid.client.coordinator.CoordinatorSelectorConfig; -import org.apache.druid.curator.discovery.ServerDiscoveryFactory; -import org.apache.druid.curator.discovery.ServerDiscoverySelector; import org.apache.druid.discovery.DruidLeaderClient; import org.apache.druid.discovery.DruidNodeDiscoveryProvider; import org.apache.druid.discovery.NodeRole; @@ -42,32 +40,19 @@ public void configure(Binder binder) JsonConfigProvider.bind(binder, "druid.selectors.coordinator", CoordinatorSelectorConfig.class); } - @Provides - @Coordinator - @ManageLifecycle - public ServerDiscoverySelector getServiceProvider( - CoordinatorSelectorConfig config, - ServerDiscoveryFactory serverDiscoveryFactory - ) - { - return serverDiscoveryFactory.createSelector(config.getServiceName()); - } - @Provides @Coordinator @ManageLifecycle public DruidLeaderClient getLeaderHttpClient( @EscalatedGlobal HttpClient httpClient, - DruidNodeDiscoveryProvider druidNodeDiscoveryProvider, - @Coordinator ServerDiscoverySelector serverDiscoverySelector + DruidNodeDiscoveryProvider druidNodeDiscoveryProvider ) { return new DruidLeaderClient( httpClient, druidNodeDiscoveryProvider, NodeRole.COORDINATOR, - "/druid/coordinator/v1/leader", - serverDiscoverySelector + "/druid/coordinator/v1/leader" ); } } diff --git a/server/src/main/java/org/apache/druid/guice/IndexingServiceDiscoveryModule.java b/server/src/main/java/org/apache/druid/guice/IndexingServiceDiscoveryModule.java index 3c4f63c5404f..3e4807813911 100644 --- a/server/src/main/java/org/apache/druid/guice/IndexingServiceDiscoveryModule.java +++ b/server/src/main/java/org/apache/druid/guice/IndexingServiceDiscoveryModule.java @@ -24,8 +24,6 @@ import com.google.inject.Provides; import org.apache.druid.client.indexing.IndexingService; import org.apache.druid.client.indexing.IndexingServiceSelectorConfig; -import org.apache.druid.curator.discovery.ServerDiscoveryFactory; -import org.apache.druid.curator.discovery.ServerDiscoverySelector; import org.apache.druid.discovery.DruidLeaderClient; import org.apache.druid.discovery.DruidNodeDiscoveryProvider; import org.apache.druid.discovery.NodeRole; @@ -42,32 +40,19 @@ public void configure(Binder binder) JsonConfigProvider.bind(binder, "druid.selectors.indexing", IndexingServiceSelectorConfig.class); } - @Provides - @IndexingService - @ManageLifecycle - public ServerDiscoverySelector getServiceProvider( - IndexingServiceSelectorConfig config, - ServerDiscoveryFactory serverDiscoveryFactory - ) - { - return serverDiscoveryFactory.createSelector(config.getServiceName()); - } - @Provides @IndexingService @ManageLifecycle public DruidLeaderClient getLeaderHttpClient( @EscalatedGlobal HttpClient httpClient, - DruidNodeDiscoveryProvider druidNodeDiscoveryProvider, - @IndexingService ServerDiscoverySelector serverDiscoverySelector + DruidNodeDiscoveryProvider druidNodeDiscoveryProvider ) { return new DruidLeaderClient( httpClient, druidNodeDiscoveryProvider, NodeRole.OVERLORD, - "/druid/indexer/v1/leader", - serverDiscoverySelector + "/druid/indexer/v1/leader" ); } } diff --git a/server/src/test/java/org/apache/druid/discovery/DruidLeaderClientTest.java b/server/src/test/java/org/apache/druid/discovery/DruidLeaderClientTest.java index 7c376ea20b68..570720db48ff 100644 --- a/server/src/test/java/org/apache/druid/discovery/DruidLeaderClientTest.java +++ b/server/src/test/java/org/apache/druid/discovery/DruidLeaderClientTest.java @@ -29,7 +29,6 @@ import com.google.inject.name.Named; import com.google.inject.name.Names; import com.google.inject.servlet.GuiceFilter; -import org.apache.druid.curator.discovery.ServerDiscoverySelector; import org.apache.druid.guice.GuiceInjectors; import org.apache.druid.guice.Jerseys; import org.apache.druid.guice.JsonConfigProvider; @@ -123,8 +122,7 @@ public void testSimple() throws Exception httpClient, druidNodeDiscoveryProvider, NodeRole.PEON, - "/simple/leader", - EasyMock.createNiceMock(ServerDiscoverySelector.class) + "/simple/leader" ); druidLeaderClient.start(); @@ -148,8 +146,7 @@ public void testNoLeaderFound() throws Exception httpClient, druidNodeDiscoveryProvider, NodeRole.PEON, - "/simple/leader", - EasyMock.createNiceMock(ServerDiscoverySelector.class) + "/simple/leader" ); druidLeaderClient.start(); @@ -175,8 +172,7 @@ public void testRedirection() throws Exception httpClient, druidNodeDiscoveryProvider, NodeRole.PEON, - "/simple/leader", - EasyMock.createNiceMock(ServerDiscoverySelector.class) + "/simple/leader" ); druidLeaderClient.start(); @@ -188,9 +184,6 @@ public void testRedirection() throws Exception @Test public void testServerFailureAndRedirect() throws Exception { - ServerDiscoverySelector serverDiscoverySelector = EasyMock.createMock(ServerDiscoverySelector.class); - EasyMock.expect(serverDiscoverySelector.pick()).andReturn(null).anyTimes(); - DruidNodeDiscovery druidNodeDiscovery = EasyMock.createMock(DruidNodeDiscovery.class); DiscoveryDruidNode dummyNode = new DiscoveryDruidNode( new DruidNode("test", "dummyhost", false, 64231, null, true, false), @@ -203,14 +196,13 @@ public void testServerFailureAndRedirect() throws Exception DruidNodeDiscoveryProvider druidNodeDiscoveryProvider = EasyMock.createMock(DruidNodeDiscoveryProvider.class); EasyMock.expect(druidNodeDiscoveryProvider.getForNodeRole(NodeRole.PEON)).andReturn(druidNodeDiscovery).anyTimes(); - EasyMock.replay(serverDiscoverySelector, druidNodeDiscovery, druidNodeDiscoveryProvider); + EasyMock.replay(druidNodeDiscovery, druidNodeDiscoveryProvider); DruidLeaderClient druidLeaderClient = new DruidLeaderClient( httpClient, druidNodeDiscoveryProvider, NodeRole.PEON, - "/simple/leader", - serverDiscoverySelector + "/simple/leader" ); druidLeaderClient.start(); @@ -236,8 +228,7 @@ public void testFindCurrentLeader() httpClient, druidNodeDiscoveryProvider, NodeRole.PEON, - "/simple/leader", - EasyMock.createNiceMock(ServerDiscoverySelector.class) + "/simple/leader" ); druidLeaderClient.start(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java index 0c17cf9436ae..c8ef83dac463 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java @@ -846,7 +846,7 @@ private class TestDruidLeaderClient extends DruidLeaderClient private TestDruidLeaderClient(ObjectMapper jsonMapper) { - super(null, new TestNodeDiscoveryProvider(), null, null, null); + super(null, new TestNodeDiscoveryProvider(), null, null); this.jsonMapper = jsonMapper; } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java index d8088ae7ed87..6ec53a0d426c 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java @@ -1009,10 +1009,7 @@ NodeRole.COORDINATOR, new FakeDruidNodeDiscovery(ImmutableMap.of(NodeRole.COORDI new FakeHttpClient(), provider, NodeRole.COORDINATOR, - "/simple/leader", - () -> { - throw new UnsupportedOperationException(); - } + "/simple/leader" ); return new SystemSchema(