From b38d0611efccbcdd5a2c4af5437e145b28c0d9ee Mon Sep 17 00:00:00 2001 From: Wei Date: Tue, 25 Jul 2017 22:09:01 +0300 Subject: [PATCH 01/33] Priority on loading for primary replica --- .../server/coordinator/rules/LoadRule.java | 143 ++++-- .../DruidCoordinatorRuleRunnerTest.java | 2 +- .../coordinator/rules/LoadRuleTest.java | 434 ++++++++---------- 3 files changed, 307 insertions(+), 272 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index f14a4f3581e0..0113013795a6 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -22,10 +22,12 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.MinMaxPriorityQueue; +import com.google.common.primitives.Ints; import com.metamx.emitter.EmittingLogger; import io.druid.java.util.common.IAE; import io.druid.server.coordinator.BalancerStrategy; import io.druid.server.coordinator.CoordinatorStats; +import io.druid.server.coordinator.DruidCluster; import io.druid.server.coordinator.DruidCoordinator; import io.druid.server.coordinator.DruidCoordinatorRuntimeParams; import io.druid.server.coordinator.LoadPeonCallback; @@ -36,6 +38,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -57,12 +60,59 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim final Map loadStatus = Maps.newHashMap(); + final Map tieredReplicants = getTieredReplicants(); + for (final String tier : tieredReplicants.keySet()) { + stats.addToTieredStat(ASSIGNED_COUNT, tier, 0); + } + + final BalancerStrategy strategy = params.getBalancerStrategy(); + + final int maxSegmentsInNodeLoadingQueue = params.getCoordinatorDynamicConfig() + .getMaxSegmentsInNodeLoadingQueue(); + + final Predicate serverHolderPredicate; + if (maxSegmentsInNodeLoadingQueue > 0) { + serverHolderPredicate = s -> (s != null && s.getNumberOfSegmentsInQueue() < maxSegmentsInNodeLoadingQueue); + } else { + serverHolderPredicate = Objects::nonNull; + } + int totalReplicantsInCluster = params.getSegmentReplicantLookup().getTotalReplicants(segment.getIdentifier()); - for (Map.Entry entry : getTieredReplicants().entrySet()) { + + final Optional primaryHolderToLoad; + if (totalReplicantsInCluster <= 0) { + log.trace("No replicants for %s", segment.getIdentifier()); + primaryHolderToLoad = getPrimaryHolder( + params.getDruidCluster(), + tieredReplicants, + strategy, + segment, + serverHolderPredicate + ); + + if (primaryHolderToLoad.isPresent()) { + final ServerHolder holder = primaryHolderToLoad.get(); + + holder.getPeon().loadSegment(segment, null); + stats.addToTieredStat(ASSIGNED_COUNT, holder.getServer().getTier(), 1); + ++totalReplicantsInCluster; + } else { + log.trace("No primary holder found for %s", segment.getIdentifier()); + return stats; + } + } else { + primaryHolderToLoad = Optional.empty(); + } + + for (Map.Entry entry : tieredReplicants.entrySet()) { final String tier = entry.getKey(); final int expectedReplicantsInTier = entry.getValue(); + final int totalReplicantsInTier = params.getSegmentReplicantLookup() - .getTotalReplicants(segment.getIdentifier(), tier); + .getTotalReplicants(segment.getIdentifier(), tier) + + (primaryHolderToLoad.map(holder -> holder.getServer().getTier().equals(tier)) + .orElse(false) ? 1 : 0); + final int loadedReplicantsInTier = params.getSegmentReplicantLookup() .getLoadedReplicants(segment.getIdentifier(), tier); @@ -73,26 +123,17 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim continue; } - final int maxSegmentsInNodeLoadingQueue = params.getCoordinatorDynamicConfig() - .getMaxSegmentsInNodeLoadingQueue(); - - Predicate serverHolderPredicate; - if (maxSegmentsInNodeLoadingQueue > 0) { - serverHolderPredicate = s -> (s != null && s.getNumberOfSegmentsInQueue() < maxSegmentsInNodeLoadingQueue); - } else { - serverHolderPredicate = Objects::nonNull; - } - - final List serverHolderList = serverQueue.stream() - .filter(serverHolderPredicate) - .collect(Collectors.toList()); + final List serverHolderList = + serverQueue + .stream() + .filter(serverHolderPredicate) + .filter(holder -> !primaryHolderToLoad.map(holder::equals).orElse(false)) + .collect(Collectors.toList()); - final BalancerStrategy strategy = params.getBalancerStrategy(); if (availableSegments.contains(segment)) { - int assignedCount = assign( + final int assignedCount = assignReplicas( params.getReplicationManager(), tier, - totalReplicantsInCluster, expectedReplicantsInTier, totalReplicantsInTier, strategy, @@ -108,14 +149,57 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim // Remove over-replication stats.accumulate(drop(loadStatus, segment, params)); - return stats; } - private int assign( + private static Optional getPrimaryHolder( + final DruidCluster cluster, + final Map tieredReplicants, + final BalancerStrategy strategy, + final DataSegment segment, + final Predicate serverHolderPredicate + ) + { + final List candidates = Lists.newLinkedList(); + + for (final Map.Entry entry : tieredReplicants.entrySet()) { + final int expectedReplicantsInTier = entry.getValue(); + if (expectedReplicantsInTier <= 0) { + continue; + } + + final String tier = entry.getKey(); + + final MinMaxPriorityQueue serverQueue = cluster.getHistoricalsByTier(tier); + if (serverQueue == null) { + log.makeAlert("Tier[%s] has no servers! Check your cluster configuration!", tier).emit(); + continue; + } + + final ServerHolder candidate = strategy.findNewSegmentHomeReplicator( + segment, + serverQueue.stream().filter(serverHolderPredicate).collect(Collectors.toList()) + ); + if (candidate == null) { + log.warn( + "Not enough [%s] servers or node capacity to assign primary segment[%s]! Expected Replicants[%d]", + tier, + segment.getIdentifier(), + expectedReplicantsInTier + ); + } else { + candidates.add(candidate); + } + } + + return candidates + .stream() + .max((s1, s2) -> Ints.compare(s1.getServer().getPriority(), s2.getServer().getPriority())); + } + + private int assignReplicas( final ReplicationThrottler replicationManager, final String tier, - final int totalReplicantsInCluster, final int expectedReplicantsInTier, final int totalReplicantsInTier, final BalancerStrategy strategy, @@ -125,11 +209,8 @@ private int assign( { int assignedCount = 0; int currReplicantsInTier = totalReplicantsInTier; - int currTotalReplicantsInCluster = totalReplicantsInCluster; while (currReplicantsInTier < expectedReplicantsInTier) { - boolean replicate = currTotalReplicantsInCluster > 0; - - if (replicate && !replicationManager.canCreateReplicant(tier)) { + if (!replicationManager.canCreateReplicant(tier)) { break; } @@ -144,12 +225,13 @@ private int assign( ); break; } + serverHolderList.remove(holder); - if (replicate) { - replicationManager.registerReplicantCreation( - tier, segment.getIdentifier(), holder.getServer().getHost() - ); - } + replicationManager.registerReplicantCreation( + tier, + segment.getIdentifier(), + holder.getServer().getHost() + ); holder.getPeon().loadSegment( segment, @@ -169,7 +251,6 @@ public void execute() ++assignedCount; ++currReplicantsInTier; - ++currTotalReplicantsInCluster; } return assignedCount; diff --git a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java index a53924c7f47d..0cf1be38381a 100644 --- a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java +++ b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java @@ -1188,7 +1188,7 @@ public void testReplicantThrottleAcrossTiers() throws Exception 1000, ServerType.HISTORICAL, "hot", - 0 + 1 ).toImmutableDruidServer(), mockPeon ) diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index f62574c7ec0c..785d710ee573 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -61,6 +61,7 @@ import org.junit.Test; import java.util.Arrays; +import java.util.Collections; import java.util.Map; import java.util.concurrent.Executors; @@ -81,79 +82,42 @@ public class LoadRuleTest ) ); - private LoadQueuePeon mockPeon; private ReplicationThrottler throttler; - private DataSegment segment; + private ListeningExecutorService exec; + private BalancerStrategy balancerStrategy; @Before public void setUp() throws Exception { EmittingLogger.registerEmitter(emitter); emitter.start(); - mockPeon = EasyMock.createMock(LoadQueuePeon.class); - throttler = new ReplicationThrottler(2, 1); - for (String tier : Arrays.asList("hot", DruidServer.DEFAULT_TIER)) { - throttler.updateReplicationState(tier); - } - segment = createDataSegment("foo"); + throttler = EasyMock.createMock(ReplicationThrottler.class); + + exec = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(1)); + balancerStrategy = new CostBalancerStrategyFactory().createBalancerStrategy(exec); } @After public void tearDown() throws Exception { - EasyMock.verify(mockPeon); + exec.shutdown(); emitter.close(); } @Test public void testLoad() throws Exception { + EasyMock.expect(throttler.canCreateReplicant(EasyMock.anyString())).andReturn(true).anyTimes(); + + final LoadQueuePeon mockPeon = createEmptyPeon(); mockPeon.loadSegment(EasyMock.anyObject(), EasyMock.anyObject()); EasyMock.expectLastCall().atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsToLoad()).andReturn(Sets.newHashSet()).atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsMarkedToDrop()).andReturn(Sets.newHashSet()).anyTimes(); - EasyMock.expect(mockPeon.getLoadQueueSize()).andReturn(0L).atLeastOnce(); - EasyMock.expect(mockPeon.getNumberOfSegmentsInQueue()).andReturn(0).anyTimes(); - EasyMock.replay(mockPeon); - - LoadRule rule = new LoadRule() - { - private final Map tiers = ImmutableMap.of( - "hot", 1, - DruidServer.DEFAULT_TIER, 2 - ); - - @Override - public Map getTieredReplicants() - { - return tiers; - } - - @Override - public int getNumReplicants(String tier) - { - return tiers.get(tier); - } - @Override - public String getType() - { - return "test"; - } - - @Override - public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) - { - return true; - } - - @Override - public boolean appliesTo(Interval interval, DateTime referenceTimestamp) - { - return true; - } - }; + LoadRule rule = createLoadRule(ImmutableMap.of( + "hot", 1, + DruidServer.DEFAULT_TIER, 2 + )); DruidCluster druidCluster = new DruidCluster( null, @@ -195,10 +159,12 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ) ); - ListeningExecutorService exec = MoreExecutors.listeningDecorator( - Executors.newFixedThreadPool(1)); - BalancerStrategy balancerStrategy = - new CostBalancerStrategyFactory().createBalancerStrategy(exec); + final DataSegment segment = createDataSegment("foo"); + + throttler.registerReplicantCreation(DruidServer.DEFAULT_TIER, segment.getIdentifier(), "hostNorm"); + EasyMock.expectLastCall().once(); + + EasyMock.replay(throttler, mockPeon); CoordinatorStats stats = rule.run( null, @@ -213,58 +179,115 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ); Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); - Assert.assertEquals(2L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, DruidServer.DEFAULT_TIER)); - exec.shutdown(); + Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, DruidServer.DEFAULT_TIER)); + + EasyMock.verify(throttler, mockPeon); } @Test - public void testDrop() throws Exception + public void testLoadPriority() throws Exception { - mockPeon.dropSegment(EasyMock.anyObject(), EasyMock.anyObject()); - EasyMock.expectLastCall().atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsToLoad()).andReturn(Sets.newHashSet()).atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsMarkedToDrop()).andReturn(Sets.newHashSet()).anyTimes(); - EasyMock.expect(mockPeon.getLoadQueueSize()).andReturn(0L).anyTimes(); - EasyMock.expect(mockPeon.getNumberOfSegmentsInQueue()).andReturn(0).anyTimes(); - EasyMock.replay(mockPeon); + EasyMock.expect(throttler.canCreateReplicant(EasyMock.anyString())).andReturn(false).anyTimes(); - LoadRule rule = new LoadRule() - { - private final Map tiers = ImmutableMap.of( - "hot", 0, - DruidServer.DEFAULT_TIER, 0 - ); + final LoadQueuePeon mockPeon1 = createEmptyPeon(); + final LoadQueuePeon mockPeon2 = createEmptyPeon(); - @Override - public Map getTieredReplicants() - { - return tiers; - } + mockPeon2.loadSegment(EasyMock.anyObject(), EasyMock.isNull()); + EasyMock.expectLastCall().once(); - @Override - public int getNumReplicants(String tier) - { - return tiers.get(tier); - } + EasyMock.replay(throttler, mockPeon1, mockPeon2); - @Override - public String getType() - { - return "test"; - } + final LoadRule rule = createLoadRule(ImmutableMap.of( + "tier1", 10, + "tier2", 10 + )); - @Override - public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) - { - return true; - } + final DruidCluster druidCluster = new DruidCluster( + null, + ImmutableMap.of( + "tier1", + MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( + Arrays.asList( + new ServerHolder( + new DruidServer( + "server1", + "host1", + null, + 1000, + ServerType.HISTORICAL, + "tier1", + 0 + ).toImmutableDruidServer(), + mockPeon1 + ) + ) + ), + "tier2", + MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( + Arrays.asList( + new ServerHolder( + new DruidServer( + "server2", + "host2", + null, + 1000, + ServerType.HISTORICAL, + "tier2", + 1 + ).toImmutableDruidServer(), + mockPeon2 + ), + new ServerHolder( + new DruidServer( + "server3", + "host3", + null, + 1000, + ServerType.HISTORICAL, + "tier2", + 1 + ).toImmutableDruidServer(), + mockPeon2 + ) + ) + ) + ) + ); - @Override - public boolean appliesTo(Interval interval, DateTime referenceTimestamp) - { - return true; - } - }; + final DataSegment segment = createDataSegment("foo"); + + final CoordinatorStats stats = rule.run( + null, + DruidCoordinatorRuntimeParams.newBuilder() + .withDruidCluster(druidCluster) + .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) + .withReplicationManager(throttler) + .withBalancerStrategy(balancerStrategy) + .withBalancerReferenceTimestamp(new DateTime("2013-01-01")) + .withAvailableSegments(Collections.singletonList(segment)).build(), + segment + ); + + Assert.assertEquals(0L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "tier1")); + Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "tier2")); + + EasyMock.verify(throttler, mockPeon1, mockPeon2); + } + + @Test + public void testDrop() throws Exception + { + final LoadQueuePeon mockPeon = createEmptyPeon(); + mockPeon.dropSegment(EasyMock.anyObject(), EasyMock.anyObject()); + EasyMock.expectLastCall().atLeastOnce(); + EasyMock.replay(throttler, mockPeon); + + LoadRule rule = createLoadRule(ImmutableMap.of( + "hot", 0, + DruidServer.DEFAULT_TIER, 0 + )); + + final DataSegment segment = createDataSegment("foo"); DruidServer server1 = new DruidServer( "serverHot", @@ -310,11 +333,6 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ) ); - ListeningExecutorService exec = MoreExecutors.listeningDecorator( - Executors.newFixedThreadPool(1)); - BalancerStrategy balancerStrategy = - new CostBalancerStrategyFactory().createBalancerStrategy(exec); - CoordinatorStats stats = rule.run( null, DruidCoordinatorRuntimeParams.newBuilder() @@ -329,57 +347,22 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) Assert.assertEquals(1L, stats.getTieredStat("droppedCount", "hot")); Assert.assertEquals(1L, stats.getTieredStat("droppedCount", DruidServer.DEFAULT_TIER)); - exec.shutdown(); + + EasyMock.verify(throttler, mockPeon); } @Test public void testLoadWithNonExistentTier() throws Exception { + final LoadQueuePeon mockPeon = createEmptyPeon(); mockPeon.loadSegment(EasyMock.anyObject(), EasyMock.anyObject()); EasyMock.expectLastCall().atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsToLoad()).andReturn(Sets.newHashSet()).atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsMarkedToDrop()).andReturn(Sets.newHashSet()).anyTimes(); - EasyMock.expect(mockPeon.getLoadQueueSize()).andReturn(0L).atLeastOnce(); - EasyMock.expect(mockPeon.getNumberOfSegmentsInQueue()).andReturn(0).anyTimes(); - EasyMock.replay(mockPeon); + EasyMock.replay(throttler, mockPeon); - LoadRule rule = new LoadRule() - { - private final Map tiers = ImmutableMap.of( - "nonExistentTier", 1, - "hot", 1 - ); - - @Override - public Map getTieredReplicants() - { - return tiers; - } - - @Override - public int getNumReplicants(String tier) - { - return tiers.get(tier); - } - - @Override - public String getType() - { - return "test"; - } - - @Override - public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) - { - return true; - } - - @Override - public boolean appliesTo(Interval interval, DateTime referenceTimestamp) - { - return true; - } - }; + LoadRule rule = createLoadRule(ImmutableMap.of( + "nonExistentTier", 1, + "hot", 1 + )); DruidCluster druidCluster = new DruidCluster( null, @@ -404,10 +387,7 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ) ); - ListeningExecutorService exec = MoreExecutors.listeningDecorator( - Executors.newFixedThreadPool(1)); - BalancerStrategy balancerStrategy = - new CostBalancerStrategyFactory().createBalancerStrategy(exec); + final DataSegment segment = createDataSegment("foo"); CoordinatorStats stats = rule.run( null, @@ -422,57 +402,24 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ); Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); - exec.shutdown(); + + EasyMock.verify(throttler, mockPeon); } @Test public void testDropWithNonExistentTier() throws Exception { + final LoadQueuePeon mockPeon = createEmptyPeon(); mockPeon.dropSegment(EasyMock.anyObject(), EasyMock.anyObject()); EasyMock.expectLastCall().atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsToLoad()).andReturn(Sets.newHashSet()).atLeastOnce(); - EasyMock.expect(mockPeon.getSegmentsMarkedToDrop()).andReturn(Sets.newHashSet()).anyTimes(); - EasyMock.expect(mockPeon.getLoadQueueSize()).andReturn(0L).anyTimes(); - EasyMock.expect(mockPeon.getNumberOfSegmentsInQueue()).andReturn(0).anyTimes(); - EasyMock.replay(mockPeon); + EasyMock.replay(throttler, mockPeon); - LoadRule rule = new LoadRule() - { - private final Map tiers = ImmutableMap.of( - "nonExistentTier", 1, - "hot", 1 - ); + LoadRule rule = createLoadRule(ImmutableMap.of( + "nonExistentTier", 1, + "hot", 1 + )); - @Override - public Map getTieredReplicants() - { - return tiers; - } - - @Override - public int getNumReplicants(String tier) - { - return tiers.get(tier); - } - - @Override - public String getType() - { - return "test"; - } - - @Override - public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) - { - return true; - } - - @Override - public boolean appliesTo(Interval interval, DateTime referenceTimestamp) - { - return true; - } - }; + final DataSegment segment = createDataSegment("foo"); DruidServer server1 = new DruidServer( "serverHot", @@ -514,11 +461,6 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ) ); - ListeningExecutorService exec = MoreExecutors.listeningDecorator( - Executors.newFixedThreadPool(1)); - BalancerStrategy balancerStrategy = - new CostBalancerStrategyFactory().createBalancerStrategy(exec); - CoordinatorStats stats = rule.run( null, DruidCoordinatorRuntimeParams.newBuilder() @@ -532,51 +474,20 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ); Assert.assertEquals(1L, stats.getTieredStat("droppedCount", "hot")); - exec.shutdown(); + + EasyMock.verify(throttler, mockPeon); } @Test public void testMaxLoadingQueueSize() throws Exception { - EasyMock.replay(mockPeon); - LoadQueuePeonTester peon = new LoadQueuePeonTester(); + EasyMock.replay(throttler); - LoadRule rule = new LoadRule() - { - private final Map tiers = ImmutableMap.of( - "hot", 1 - ); + final LoadQueuePeonTester peon = new LoadQueuePeonTester(); - @Override - public Map getTieredReplicants() - { - return tiers; - } - - @Override - public int getNumReplicants(String tier) - { - return tiers.get(tier); - } - - @Override - public String getType() - { - return "test"; - } - - @Override - public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) - { - return true; - } - - @Override - public boolean appliesTo(Interval interval, DateTime referenceTimestamp) - { - return true; - } - }; + LoadRule rule = createLoadRule(ImmutableMap.of( + "hot", 1 + )); DruidCluster druidCluster = new DruidCluster( null, @@ -601,11 +512,6 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) ) ); - ListeningExecutorService exec = MoreExecutors.listeningDecorator( - Executors.newFixedThreadPool(1)); - BalancerStrategy balancerStrategy = - new CostBalancerStrategyFactory().createBalancerStrategy(exec); - DataSegment dataSegment1 = createDataSegment("ds1"); DataSegment dataSegment2 = createDataSegment("ds2"); DataSegment dataSegment3 = createDataSegment("ds3"); @@ -629,7 +535,8 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) Assert.assertEquals(1L, stats1.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); Assert.assertEquals(1L, stats2.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); Assert.assertEquals(0L, stats3.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); - exec.shutdown(); + + EasyMock.verify(throttler); } private DataSegment createDataSegment(String dataSource) @@ -646,4 +553,51 @@ private DataSegment createDataSegment(String dataSource) 0 ); } + + private static LoadRule createLoadRule(final Map tieredReplicants) + { + return new LoadRule() + { + @Override + public Map getTieredReplicants() + { + return tieredReplicants; + } + + @Override + public int getNumReplicants(String tier) + { + return tieredReplicants.get(tier); + } + + @Override + public String getType() + { + return "test"; + } + + @Override + public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) + { + return true; + } + + @Override + public boolean appliesTo(Interval interval, DateTime referenceTimestamp) + { + return true; + } + }; + } + + private static LoadQueuePeon createEmptyPeon() + { + final LoadQueuePeon mockPeon = EasyMock.createMock(LoadQueuePeon.class); + EasyMock.expect(mockPeon.getSegmentsToLoad()).andReturn(Sets.newHashSet()).anyTimes(); + EasyMock.expect(mockPeon.getSegmentsMarkedToDrop()).andReturn(Sets.newHashSet()).anyTimes(); + EasyMock.expect(mockPeon.getLoadQueueSize()).andReturn(0L).anyTimes(); + EasyMock.expect(mockPeon.getNumberOfSegmentsInQueue()).andReturn(0).anyTimes(); + + return mockPeon; + } } From e273af1378796fd1ec7f05e681c447d8a84415a0 Mon Sep 17 00:00:00 2001 From: leventov Date: Wed, 6 Sep 2017 15:45:20 -0500 Subject: [PATCH 02/33] Simplicity fixes --- .../server/coordinator/rules/LoadRule.java | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 0113013795a6..a2e52beacaaf 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -22,7 +22,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.MinMaxPriorityQueue; -import com.google.common.primitives.Ints; import com.metamx.emitter.EmittingLogger; import io.druid.java.util.common.IAE; import io.druid.server.coordinator.BalancerStrategy; @@ -35,10 +34,10 @@ import io.druid.server.coordinator.ServerHolder; import io.druid.timeline.DataSegment; +import javax.annotation.Nullable; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -79,7 +78,7 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim int totalReplicantsInCluster = params.getSegmentReplicantLookup().getTotalReplicants(segment.getIdentifier()); - final Optional primaryHolderToLoad; + final ServerHolder primaryHolderToLoad; if (totalReplicantsInCluster <= 0) { log.trace("No replicants for %s", segment.getIdentifier()); primaryHolderToLoad = getPrimaryHolder( @@ -90,28 +89,27 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim serverHolderPredicate ); - if (primaryHolderToLoad.isPresent()) { - final ServerHolder holder = primaryHolderToLoad.get(); + if (primaryHolderToLoad != null) { - holder.getPeon().loadSegment(segment, null); - stats.addToTieredStat(ASSIGNED_COUNT, holder.getServer().getTier(), 1); + primaryHolderToLoad.getPeon().loadSegment(segment, null); + stats.addToTieredStat(ASSIGNED_COUNT, primaryHolderToLoad.getServer().getTier(), 1); ++totalReplicantsInCluster; } else { log.trace("No primary holder found for %s", segment.getIdentifier()); return stats; } } else { - primaryHolderToLoad = Optional.empty(); + primaryHolderToLoad = null; } for (Map.Entry entry : tieredReplicants.entrySet()) { final String tier = entry.getKey(); final int expectedReplicantsInTier = entry.getValue(); - final int totalReplicantsInTier = params.getSegmentReplicantLookup() - .getTotalReplicants(segment.getIdentifier(), tier) + - (primaryHolderToLoad.map(holder -> holder.getServer().getTier().equals(tier)) - .orElse(false) ? 1 : 0); + int totalReplicantsInTier = params.getSegmentReplicantLookup().getTotalReplicants(segment.getIdentifier(), tier); + if (primaryHolderToLoad != null && primaryHolderToLoad.getServer().getTier().equals(tier)) { + totalReplicantsInTier += 1; + } final int loadedReplicantsInTier = params.getSegmentReplicantLookup() .getLoadedReplicants(segment.getIdentifier(), tier); @@ -127,7 +125,7 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim serverQueue .stream() .filter(serverHolderPredicate) - .filter(holder -> !primaryHolderToLoad.map(holder::equals).orElse(false)) + .filter(holder -> primaryHolderToLoad != null && !holder.equals(primaryHolderToLoad)) .collect(Collectors.toList()); if (availableSegments.contains(segment)) { @@ -152,7 +150,8 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim return stats; } - private static Optional getPrimaryHolder( + @Nullable + private static ServerHolder getPrimaryHolder( final DruidCluster cluster, final Map tieredReplicants, final BalancerStrategy strategy, @@ -160,7 +159,7 @@ private static Optional getPrimaryHolder( final Predicate serverHolderPredicate ) { - final List candidates = Lists.newLinkedList(); + ServerHolder topCandidate = null; for (final Map.Entry entry : tieredReplicants.entrySet()) { final int expectedReplicantsInTier = entry.getValue(); @@ -188,13 +187,17 @@ private static Optional getPrimaryHolder( expectedReplicantsInTier ); } else { - candidates.add(candidate); + if (topCandidate == null) { + topCandidate = candidate; + } else { + if (candidate.getServer().getPriority() > topCandidate.getServer().getPriority()) { + topCandidate = candidate; + } + } } } - return candidates - .stream() - .max((s1, s2) -> Ints.compare(s1.getServer().getPriority(), s2.getServer().getPriority())); + return topCandidate; } private int assignReplicas( From 1a0e6a746b62c8b55f680dbeaed146b6bd20d1ec Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 6 Sep 2017 19:05:55 -0700 Subject: [PATCH 03/33] Fix on skipping drop for quick return. --- .../io/druid/server/coordinator/rules/LoadRule.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index a2e52beacaaf..b7f92a814966 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -89,15 +89,14 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim serverHolderPredicate ); - if (primaryHolderToLoad != null) { - - primaryHolderToLoad.getPeon().loadSegment(segment, null); - stats.addToTieredStat(ASSIGNED_COUNT, primaryHolderToLoad.getServer().getTier(), 1); - ++totalReplicantsInCluster; - } else { + if (primaryHolderToLoad == null) { log.trace("No primary holder found for %s", segment.getIdentifier()); - return stats; + return stats.accumulate(drop(loadStatus, segment, params)); } + + primaryHolderToLoad.getPeon().loadSegment(segment, null); + stats.addToTieredStat(ASSIGNED_COUNT, primaryHolderToLoad.getServer().getTier(), 1); + ++totalReplicantsInCluster; } else { primaryHolderToLoad = null; } From 1fe5115a7c0e4a9a278b1c59b0acd378e492facc Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 6 Sep 2017 19:06:52 -0700 Subject: [PATCH 04/33] change to debug logging for no replicants. --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index b7f92a814966..31ee745bbdaf 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -80,7 +80,7 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim final ServerHolder primaryHolderToLoad; if (totalReplicantsInCluster <= 0) { - log.trace("No replicants for %s", segment.getIdentifier()); + log.debug("No replicants for %s", segment.getIdentifier()); primaryHolderToLoad = getPrimaryHolder( params.getDruidCluster(), tieredReplicants, From a98c0aa6cd593402e297b1cfd529919882cd7c43 Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 6 Sep 2017 19:14:27 -0700 Subject: [PATCH 05/33] Fix on filter logic --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 31ee745bbdaf..fe3a155469b7 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -124,7 +124,7 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim serverQueue .stream() .filter(serverHolderPredicate) - .filter(holder -> primaryHolderToLoad != null && !holder.equals(primaryHolderToLoad)) + .filter(holder -> !holder.equals(primaryHolderToLoad)) .collect(Collectors.toList()); if (availableSegments.contains(segment)) { From b98fad7016cd516449c5034f5d531c5cdcd9d922 Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 6 Sep 2017 19:23:20 -0700 Subject: [PATCH 06/33] swapping if-else --- .../java/io/druid/server/coordinator/rules/LoadRule.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index fe3a155469b7..da1fcdb953f2 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -79,7 +79,9 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim int totalReplicantsInCluster = params.getSegmentReplicantLookup().getTotalReplicants(segment.getIdentifier()); final ServerHolder primaryHolderToLoad; - if (totalReplicantsInCluster <= 0) { + if (totalReplicantsInCluster > 0) { + primaryHolderToLoad = null; + } else { log.debug("No replicants for %s", segment.getIdentifier()); primaryHolderToLoad = getPrimaryHolder( params.getDruidCluster(), @@ -97,8 +99,6 @@ public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntim primaryHolderToLoad.getPeon().loadSegment(segment, null); stats.addToTieredStat(ASSIGNED_COUNT, primaryHolderToLoad.getServer().getTier(), 1); ++totalReplicantsInCluster; - } else { - primaryHolderToLoad = null; } for (Map.Entry entry : tieredReplicants.entrySet()) { From 97cc2304b28a491b4ce306bb22a3b65eb1b7d3b7 Mon Sep 17 00:00:00 2001 From: Wei Date: Fri, 8 Sep 2017 16:48:53 -0700 Subject: [PATCH 07/33] Fix on wrong "hasTier" logic --- .../src/main/java/io/druid/server/coordinator/DruidCluster.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/io/druid/server/coordinator/DruidCluster.java b/server/src/main/java/io/druid/server/coordinator/DruidCluster.java index 014c42ba91c4..55b01eb21732 100644 --- a/server/src/main/java/io/druid/server/coordinator/DruidCluster.java +++ b/server/src/main/java/io/druid/server/coordinator/DruidCluster.java @@ -147,6 +147,6 @@ public boolean hasRealtimes() public boolean hasTier(String tier) { MinMaxPriorityQueue servers = historicals.get(tier); - return (servers == null) || servers.isEmpty(); + return (servers != null) && !servers.isEmpty(); } } From 18802ab3db96b1e093180b405e16089c05e0bcc4 Mon Sep 17 00:00:00 2001 From: Wei Date: Fri, 8 Sep 2017 16:49:46 -0700 Subject: [PATCH 08/33] Refactoring of LoadRule --- .../server/coordinator/rules/LoadRule.java | 385 +++++++++--------- .../coordinator/rules/LoadRuleTest.java | 4 +- 2 files changed, 194 insertions(+), 195 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index da1fcdb953f2..e62ceb51ac64 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -19,26 +19,26 @@ package io.druid.server.coordinator.rules; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.MinMaxPriorityQueue; import com.metamx.emitter.EmittingLogger; import io.druid.java.util.common.IAE; -import io.druid.server.coordinator.BalancerStrategy; import io.druid.server.coordinator.CoordinatorStats; import io.druid.server.coordinator.DruidCluster; import io.druid.server.coordinator.DruidCoordinator; import io.druid.server.coordinator.DruidCoordinatorRuntimeParams; -import io.druid.server.coordinator.LoadPeonCallback; import io.druid.server.coordinator.ReplicationThrottler; import io.druid.server.coordinator.ServerHolder; import io.druid.timeline.DataSegment; +import it.unimi.dsi.fastutil.objects.Object2IntMap; +import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import javax.annotation.Nullable; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -52,268 +52,267 @@ public abstract class LoadRule implements Rule static final String DROPPED_COUNT = "droppedCount"; @Override - public CoordinatorStats run(DruidCoordinator coordinator, DruidCoordinatorRuntimeParams params, DataSegment segment) + public CoordinatorStats run( + final DruidCoordinator coordinator, + final DruidCoordinatorRuntimeParams params, + final DataSegment segment + ) { - final CoordinatorStats stats = new CoordinatorStats(); - final Set availableSegments = params.getAvailableSegments(); + final Object2IntMap targetReplicants = new Object2IntOpenHashMap<>(getTieredReplicants()); - final Map loadStatus = Maps.newHashMap(); + final Object2IntMap currentReplicants = new Object2IntOpenHashMap<>( + params.getSegmentReplicantLookup().getClusterTiers(segment.getIdentifier()) + ); - final Map tieredReplicants = getTieredReplicants(); - for (final String tier : tieredReplicants.keySet()) { - stats.addToTieredStat(ASSIGNED_COUNT, tier, 0); + final CoordinatorStats stats = new CoordinatorStats(); + if (params.getAvailableSegments().contains(segment)) { + assign(targetReplicants, currentReplicants, params, segment, stats); } - final BalancerStrategy strategy = params.getBalancerStrategy(); - - final int maxSegmentsInNodeLoadingQueue = params.getCoordinatorDynamicConfig() - .getMaxSegmentsInNodeLoadingQueue(); + drop(targetReplicants, currentReplicants, params, segment, stats); - final Predicate serverHolderPredicate; - if (maxSegmentsInNodeLoadingQueue > 0) { - serverHolderPredicate = s -> (s != null && s.getNumberOfSegmentsInQueue() < maxSegmentsInNodeLoadingQueue); - } else { - serverHolderPredicate = Objects::nonNull; - } - - int totalReplicantsInCluster = params.getSegmentReplicantLookup().getTotalReplicants(segment.getIdentifier()); + return stats; + } - final ServerHolder primaryHolderToLoad; - if (totalReplicantsInCluster > 0) { - primaryHolderToLoad = null; + private static void assign( + final Object2IntMap targetReplicants, + final Object2IntMap currentReplicants, + final DruidCoordinatorRuntimeParams params, + final DataSegment segment, + final CoordinatorStats stats + ) + { + // if primary replica already exists + if (!currentReplicants.isEmpty()) { + assignReplicas(targetReplicants, currentReplicants, params, segment, stats); } else { - log.debug("No replicants for %s", segment.getIdentifier()); - primaryHolderToLoad = getPrimaryHolder( - params.getDruidCluster(), - tieredReplicants, - strategy, - segment, - serverHolderPredicate - ); - + final ServerHolder primaryHolderToLoad = assignPrimary(targetReplicants, params, segment); if (primaryHolderToLoad == null) { - log.trace("No primary holder found for %s", segment.getIdentifier()); - return stats.accumulate(drop(loadStatus, segment, params)); + // cluster does not have any replicants and cannot identify primary holder + // this implies that no assignment could be done + return; } - primaryHolderToLoad.getPeon().loadSegment(segment, null); - stats.addToTieredStat(ASSIGNED_COUNT, primaryHolderToLoad.getServer().getTier(), 1); - ++totalReplicantsInCluster; + final String tier = primaryHolderToLoad.getServer().getTier(); + // assign replicas for the rest of the tier + final int numAssigned = 1 /* primary */ + assignReplicasForTier( + tier, + targetReplicants.getOrDefault(tier, 0), + currentReplicants.getOrDefault(tier, 0) + 1, + params, + getHolderList( + tier, + params.getDruidCluster(), + createPredicate(params), + holder -> !holder.equals(primaryHolderToLoad) + ), + segment + ); + stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); + + // tier with primary replica + final int targetReplicantsInTier = targetReplicants.removeInt(tier); + // assign replicas for the other tiers + assignReplicas(targetReplicants, currentReplicants, params, segment, stats); + // add back tier (this is for processing drop) + targetReplicants.put(tier, targetReplicantsInTier); } + } - for (Map.Entry entry : tieredReplicants.entrySet()) { - final String tier = entry.getKey(); - final int expectedReplicantsInTier = entry.getValue(); - - int totalReplicantsInTier = params.getSegmentReplicantLookup().getTotalReplicants(segment.getIdentifier(), tier); - if (primaryHolderToLoad != null && primaryHolderToLoad.getServer().getTier().equals(tier)) { - totalReplicantsInTier += 1; - } - - final int loadedReplicantsInTier = params.getSegmentReplicantLookup() - .getLoadedReplicants(segment.getIdentifier(), tier); - - final MinMaxPriorityQueue serverQueue = params.getDruidCluster().getHistoricalsByTier(tier); - - if (serverQueue == null) { - log.makeAlert("Tier[%s] has no servers! Check your cluster configuration!", tier).emit(); - continue; - } - - final List serverHolderList = - serverQueue - .stream() - .filter(serverHolderPredicate) - .filter(holder -> !holder.equals(primaryHolderToLoad)) - .collect(Collectors.toList()); - - if (availableSegments.contains(segment)) { - final int assignedCount = assignReplicas( - params.getReplicationManager(), - tier, - expectedReplicantsInTier, - totalReplicantsInTier, - strategy, - serverHolderList, - segment - ); - stats.addToTieredStat(ASSIGNED_COUNT, tier, assignedCount); - totalReplicantsInCluster += assignedCount; - } + private static Predicate createPredicate(final DruidCoordinatorRuntimeParams params) + { + final int maxSegmentsInNodeLoadingQueue = params.getCoordinatorDynamicConfig().getMaxSegmentsInNodeLoadingQueue(); + if (maxSegmentsInNodeLoadingQueue <= 0) { + return Objects::nonNull; + } else { + return s -> (s != null && s.getNumberOfSegmentsInQueue() < maxSegmentsInNodeLoadingQueue); + } + } - loadStatus.put(tier, expectedReplicantsInTier - loadedReplicantsInTier); + @SafeVarargs + private static List getHolderList( + final String tier, + final DruidCluster druidCluster, + final Predicate firstPredicate, + final Predicate... otherPredicates + ) + { + final MinMaxPriorityQueue queue = druidCluster.getHistoricalsByTier(tier); + if (queue == null) { + log.makeAlert("Tier[%s] has no servers! Check your cluster configuration!", tier).emit(); + return Collections.emptyList(); } - // Remove over-replication - stats.accumulate(drop(loadStatus, segment, params)); - return stats; + final Predicate predicate = Arrays.stream(otherPredicates).reduce(firstPredicate, Predicate::and); + return queue.stream().filter(predicate).collect(Collectors.toList()); } @Nullable - private static ServerHolder getPrimaryHolder( - final DruidCluster cluster, - final Map tieredReplicants, - final BalancerStrategy strategy, - final DataSegment segment, - final Predicate serverHolderPredicate + private static ServerHolder assignPrimary( + final Object2IntMap targetReplicants, + final DruidCoordinatorRuntimeParams params, + final DataSegment segment ) { ServerHolder topCandidate = null; - - for (final Map.Entry entry : tieredReplicants.entrySet()) { - final int expectedReplicantsInTier = entry.getValue(); - if (expectedReplicantsInTier <= 0) { + for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { + final int targetReplicantsInTier = entry.getIntValue(); + if (targetReplicantsInTier <= 0) { continue; } final String tier = entry.getKey(); - final MinMaxPriorityQueue serverQueue = cluster.getHistoricalsByTier(tier); - if (serverQueue == null) { - log.makeAlert("Tier[%s] has no servers! Check your cluster configuration!", tier).emit(); + final List holders = getHolderList(tier, params.getDruidCluster(), createPredicate(params)); + if (holders.isEmpty()) { continue; } - final ServerHolder candidate = strategy.findNewSegmentHomeReplicator( - segment, - serverQueue.stream().filter(serverHolderPredicate).collect(Collectors.toList()) - ); + final ServerHolder candidate = params.getBalancerStrategy().findNewSegmentHomeReplicator(segment, holders); if (candidate == null) { log.warn( "Not enough [%s] servers or node capacity to assign primary segment[%s]! Expected Replicants[%d]", - tier, - segment.getIdentifier(), - expectedReplicantsInTier + tier, segment.getIdentifier(), targetReplicantsInTier ); - } else { - if (topCandidate == null) { - topCandidate = candidate; - } else { - if (candidate.getServer().getPriority() > topCandidate.getServer().getPriority()) { - topCandidate = candidate; - } - } + } else if (topCandidate == null || candidate.getServer().getPriority() > topCandidate.getServer().getPriority()) { + topCandidate = candidate; } } + if (topCandidate != null) { + topCandidate.getPeon().loadSegment(segment, null); + } + return topCandidate; } - private int assignReplicas( - final ReplicationThrottler replicationManager, + private static void assignReplicas( + final Object2IntMap targetReplicants, + final Object2IntMap currentReplicants, + final DruidCoordinatorRuntimeParams params, + final DataSegment segment, + final CoordinatorStats stats + ) + { + for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { + final String tier = entry.getKey(); + final int numAssigned = assignReplicasForTier( + tier, + entry.getIntValue(), + currentReplicants.getOrDefault(tier, 0), + params, + getHolderList(tier, params.getDruidCluster(), createPredicate(params)), + segment + ); + stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); + } + } + + private static int assignReplicasForTier( final String tier, - final int expectedReplicantsInTier, - final int totalReplicantsInTier, - final BalancerStrategy strategy, - final List serverHolderList, + final int targetReplicantsInTier, + final int currentReplicantsInTier, + final DruidCoordinatorRuntimeParams params, + final List holders, final DataSegment segment ) { - int assignedCount = 0; - int currReplicantsInTier = totalReplicantsInTier; - while (currReplicantsInTier < expectedReplicantsInTier) { - if (!replicationManager.canCreateReplicant(tier)) { - break; - } + final int numToAssign = targetReplicantsInTier - currentReplicantsInTier; + if (numToAssign <= 0 || holders.isEmpty()) { + return 0; + } - final ServerHolder holder = strategy.findNewSegmentHomeReplicator(segment, serverHolderList); + final ReplicationThrottler throttler = params.getReplicationManager(); + for (int numAssigned = 0; numAssigned < numToAssign; numAssigned++) { + if (!throttler.canCreateReplicant(tier)) { + return numAssigned; + } + final ServerHolder holder = params.getBalancerStrategy().findNewSegmentHomeReplicator(segment, holders); if (holder == null) { log.warn( "Not enough [%s] servers or node capacity to assign segment[%s]! Expected Replicants[%d]", - tier, - segment.getIdentifier(), - expectedReplicantsInTier + tier, segment.getIdentifier(), targetReplicantsInTier ); - break; + return numAssigned; } - serverHolderList.remove(holder); - - replicationManager.registerReplicantCreation( - tier, - segment.getIdentifier(), - holder.getServer().getHost() - ); + holders.remove(holder); - holder.getPeon().loadSegment( - segment, - new LoadPeonCallback() - { - @Override - public void execute() - { - replicationManager.unregisterReplicantCreation( - tier, - segment.getIdentifier(), - holder.getServer().getHost() - ); - } - } - ); - - ++assignedCount; - ++currReplicantsInTier; + final String segmentId = segment.getIdentifier(); + final String holderHost = holder.getServer().getHost(); + throttler.registerReplicantCreation(tier, segmentId, holderHost); + holder.getPeon().loadSegment(segment, () -> throttler.unregisterReplicantCreation(tier, segmentId, holderHost)); } - return assignedCount; + return numToAssign; } - private CoordinatorStats drop( - final Map loadStatus, + private static void drop( + final Object2IntMap targetReplicants, + final Object2IntMap currentReplicants, + final DruidCoordinatorRuntimeParams params, final DataSegment segment, - final DruidCoordinatorRuntimeParams params + final CoordinatorStats stats ) { - CoordinatorStats stats = new CoordinatorStats(); + final DruidCluster druidCluster = params.getDruidCluster(); // Make sure we have enough loaded replicants in the correct tiers in the cluster before doing anything - for (Integer leftToLoad : loadStatus.values()) { - if (leftToLoad > 0) { - return stats; + for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { + final String tier = entry.getKey(); + // if there are replicants loading in cluster + if (druidCluster.hasTier(tier) && entry.getIntValue() > currentReplicants.getOrDefault(tier, 0)) { + return; } } - // Find all instances of this segment across tiers - Map replicantsByTier = params.getSegmentReplicantLookup().getClusterTiers(segment.getIdentifier()); - - for (Map.Entry entry : replicantsByTier.entrySet()) { + for (final Object2IntMap.Entry entry : currentReplicants.object2IntEntrySet()) { final String tier = entry.getKey(); - int loadedNumReplicantsForTier = entry.getValue(); - int expectedNumReplicantsForTier = getNumReplicants(tier); - stats.addToTieredStat(DROPPED_COUNT, tier, 0); + final MinMaxPriorityQueue holders = druidCluster.getHistoricalsByTier(tier); - MinMaxPriorityQueue serverQueue = params.getDruidCluster().getHistoricalsByTier(tier); - if (serverQueue == null) { - log.makeAlert("No holders found for tier[%s]", entry.getKey()).emit(); - continue; + final int numDropped; + if (holders == null) { + log.makeAlert("No holders found for tier[%s]", tier).emit(); + numDropped = 0; + } else { + final int numToDrop = entry.getIntValue() - targetReplicants.getOrDefault(tier, 0); + numDropped = dropForTier(numToDrop, holders, segment); + } + + stats.addToTieredStat(DROPPED_COUNT, tier, numDropped); + } + } + + private static int dropForTier( + final int numToDrop, + final MinMaxPriorityQueue holders, + final DataSegment segment + ) + { + int numDropped = 0; + + final List droppedHolders = new LinkedList<>(); + while (numDropped < numToDrop) { + final ServerHolder holder = holders.pollLast(); + if (holder == null) { + log.warn("Wtf, holder was null? I have no servers serving [%s]?", segment.getIdentifier()); + break; } - List droppedServers = Lists.newArrayList(); - while (loadedNumReplicantsForTier > expectedNumReplicantsForTier) { - final ServerHolder holder = serverQueue.pollLast(); - if (holder == null) { - log.warn("Wtf, holder was null? I have no servers serving [%s]?", segment.getIdentifier()); - break; - } - - if (holder.isServingSegment(segment)) { - holder.getPeon().dropSegment( - segment, - null - ); - --loadedNumReplicantsForTier; - stats.addToTieredStat(DROPPED_COUNT, tier, 1); - } - droppedServers.add(holder); + if (holder.isServingSegment(segment)) { + holder.getPeon().dropSegment(segment, null); + ++numDropped; } - serverQueue.addAll(droppedServers); + droppedHolders.add(holder); } + // add back the holders + holders.addAll(droppedHolders); - return stats; + return numDropped; } - protected void validateTieredReplicants(Map tieredReplicants) + protected static void validateTieredReplicants(final Map tieredReplicants) { if (tieredReplicants.size() == 0) { throw new IAE("A rule with empty tiered replicants is invalid"); diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index 785d710ee573..f260805a4244 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -133,7 +133,7 @@ public void testLoad() throws Exception 1000, ServerType.HISTORICAL, "hot", - 0 + 1 ).toImmutableDruidServer(), mockPeon ) @@ -534,7 +534,7 @@ public void testMaxLoadingQueueSize() throws Exception Assert.assertEquals(1L, stats1.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); Assert.assertEquals(1L, stats2.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); - Assert.assertEquals(0L, stats3.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); + Assert.assertFalse(stats3.getTiers(LoadRule.ASSIGNED_COUNT).contains("hot")); EasyMock.verify(throttler); } From 4ee9f581de4d1caee2343992fa8441376dff3016 Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 11:52:04 -0700 Subject: [PATCH 09/33] Rename createPredicate to createLoadQueueSizeLimitingPredicate --- .../io/druid/server/coordinator/rules/LoadRule.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index e62ceb51ac64..0805bb5d60b6 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -103,7 +103,7 @@ private static void assign( getHolderList( tier, params.getDruidCluster(), - createPredicate(params), + createLoadQueueSizeLimitingPredicate(params), holder -> !holder.equals(primaryHolderToLoad) ), segment @@ -119,7 +119,9 @@ private static void assign( } } - private static Predicate createPredicate(final DruidCoordinatorRuntimeParams params) + private static Predicate createLoadQueueSizeLimitingPredicate( + final DruidCoordinatorRuntimeParams params + ) { final int maxSegmentsInNodeLoadingQueue = params.getCoordinatorDynamicConfig().getMaxSegmentsInNodeLoadingQueue(); if (maxSegmentsInNodeLoadingQueue <= 0) { @@ -163,7 +165,7 @@ private static ServerHolder assignPrimary( final String tier = entry.getKey(); - final List holders = getHolderList(tier, params.getDruidCluster(), createPredicate(params)); + final List holders = getHolderList(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)); if (holders.isEmpty()) { continue; } @@ -201,7 +203,7 @@ private static void assignReplicas( entry.getIntValue(), currentReplicants.getOrDefault(tier, 0), params, - getHolderList(tier, params.getDruidCluster(), createPredicate(params)), + getHolderList(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)), segment ); stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); From d1feeb4d802b3aff63836ffe189b216746e02c86 Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 11:56:13 -0700 Subject: [PATCH 10/33] Rename getHolderList to getFilteredHolders --- .../java/io/druid/server/coordinator/rules/LoadRule.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 0805bb5d60b6..4ffa131ad4a8 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -100,7 +100,7 @@ private static void assign( targetReplicants.getOrDefault(tier, 0), currentReplicants.getOrDefault(tier, 0) + 1, params, - getHolderList( + getFilteredHolders( tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params), @@ -132,7 +132,7 @@ private static Predicate createLoadQueueSizeLimitingPredicate( } @SafeVarargs - private static List getHolderList( + private static List getFilteredHolders( final String tier, final DruidCluster druidCluster, final Predicate firstPredicate, @@ -165,7 +165,7 @@ private static ServerHolder assignPrimary( final String tier = entry.getKey(); - final List holders = getHolderList(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)); + final List holders = getFilteredHolders(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)); if (holders.isEmpty()) { continue; } @@ -203,7 +203,7 @@ private static void assignReplicas( entry.getIntValue(), currentReplicants.getOrDefault(tier, 0), params, - getHolderList(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)), + getFilteredHolders(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)), segment ); stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); From 1f5c04725d07352e0ec8aa6b199661170f0b3d5d Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 12:01:01 -0700 Subject: [PATCH 11/33] remove varargs --- .../druid/server/coordinator/rules/LoadRule.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 4ffa131ad4a8..678f0e5734d0 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -33,7 +33,6 @@ import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import javax.annotation.Nullable; -import java.util.Arrays; import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -103,8 +102,7 @@ private static void assign( getFilteredHolders( tier, params.getDruidCluster(), - createLoadQueueSizeLimitingPredicate(params), - holder -> !holder.equals(primaryHolderToLoad) + createLoadQueueSizeLimitingPredicate(params).and(holder -> !holder.equals(primaryHolderToLoad)) ), segment ); @@ -131,12 +129,10 @@ private static Predicate createLoadQueueSizeLimitingPredicate( } } - @SafeVarargs private static List getFilteredHolders( final String tier, final DruidCluster druidCluster, - final Predicate firstPredicate, - final Predicate... otherPredicates + final Predicate predicate ) { final MinMaxPriorityQueue queue = druidCluster.getHistoricalsByTier(tier); @@ -145,7 +141,6 @@ private static List getFilteredHolders( return Collections.emptyList(); } - final Predicate predicate = Arrays.stream(otherPredicates).reduce(firstPredicate, Predicate::and); return queue.stream().filter(predicate).collect(Collectors.toList()); } @@ -165,7 +160,11 @@ private static ServerHolder assignPrimary( final String tier = entry.getKey(); - final List holders = getFilteredHolders(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)); + final List holders = getFilteredHolders( + tier, + params.getDruidCluster(), + createLoadQueueSizeLimitingPredicate(params) + ); if (holders.isEmpty()) { continue; } From 441203e698f89a7a74bb347851779bb4ad33b7a7 Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 12:23:47 -0700 Subject: [PATCH 12/33] extract out currentReplicantsInTier --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 678f0e5734d0..e174809c2e03 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -277,7 +277,8 @@ private static void drop( log.makeAlert("No holders found for tier[%s]", tier).emit(); numDropped = 0; } else { - final int numToDrop = entry.getIntValue() - targetReplicants.getOrDefault(tier, 0); + final int currentReplicantsInTier = entry.getIntValue(); + final int numToDrop = currentReplicantsInTier - targetReplicants.getOrDefault(tier, 0); numDropped = dropForTier(numToDrop, holders, segment); } From 0aedc656f0cd2c1e98ba9c9d17e28e86652aeab1 Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 12:27:46 -0700 Subject: [PATCH 13/33] rename holders to holdersInTier --- .../java/io/druid/server/coordinator/rules/LoadRule.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index e174809c2e03..2bd04b27cf04 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -288,7 +288,7 @@ private static void drop( private static int dropForTier( final int numToDrop, - final MinMaxPriorityQueue holders, + final MinMaxPriorityQueue holdersInTier, final DataSegment segment ) { @@ -296,7 +296,7 @@ private static int dropForTier( final List droppedHolders = new LinkedList<>(); while (numDropped < numToDrop) { - final ServerHolder holder = holders.pollLast(); + final ServerHolder holder = holdersInTier.pollLast(); if (holder == null) { log.warn("Wtf, holder was null? I have no servers serving [%s]?", segment.getIdentifier()); break; @@ -309,7 +309,7 @@ private static int dropForTier( droppedHolders.add(holder); } // add back the holders - holders.addAll(droppedHolders); + holdersInTier.addAll(droppedHolders); return numDropped; } From 4b841906ab7417bc9ed624eb596f8dd90569cd55 Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 13:13:37 -0700 Subject: [PATCH 14/33] don't do temporary removal of tier. --- .../druid/server/coordinator/rules/LoadRule.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 2bd04b27cf04..f342a849cad0 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -83,7 +83,7 @@ private static void assign( { // if primary replica already exists if (!currentReplicants.isEmpty()) { - assignReplicas(targetReplicants, currentReplicants, params, segment, stats); + assignReplicas(targetReplicants, currentReplicants, params, segment, stats, null); } else { final ServerHolder primaryHolderToLoad = assignPrimary(targetReplicants, params, segment); if (primaryHolderToLoad == null) { @@ -108,12 +108,7 @@ private static void assign( ); stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); - // tier with primary replica - final int targetReplicantsInTier = targetReplicants.removeInt(tier); - // assign replicas for the other tiers - assignReplicas(targetReplicants, currentReplicants, params, segment, stats); - // add back tier (this is for processing drop) - targetReplicants.put(tier, targetReplicantsInTier); + assignReplicas(targetReplicants, currentReplicants, params, segment, stats, tier); } } @@ -192,11 +187,15 @@ private static void assignReplicas( final Object2IntMap currentReplicants, final DruidCoordinatorRuntimeParams params, final DataSegment segment, - final CoordinatorStats stats + final CoordinatorStats stats, + @Nullable final String primaryTier ) { for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { final String tier = entry.getKey(); + if (tier.equals(primaryTier)) { + continue; + } final int numAssigned = assignReplicasForTier( tier, entry.getIntValue(), From 15b6011179443877379e6bdf653e8b21195c40a9 Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 15:37:41 -0700 Subject: [PATCH 15/33] rename primaryTier to tierToSkip --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index f342a849cad0..3689aa7e5306 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -188,12 +188,12 @@ private static void assignReplicas( final DruidCoordinatorRuntimeParams params, final DataSegment segment, final CoordinatorStats stats, - @Nullable final String primaryTier + @Nullable final String tierToSkip ) { for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { final String tier = entry.getKey(); - if (tier.equals(primaryTier)) { + if (tier.equals(tierToSkip)) { continue; } final int numAssigned = assignReplicasForTier( From 61ebb16ee6b8b9d3b5f717ad5c354032bdd7333e Mon Sep 17 00:00:00 2001 From: Wei Date: Mon, 11 Sep 2017 15:54:58 -0700 Subject: [PATCH 16/33] change LinkedList to ArrayList --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 3689aa7e5306..73eeaf071b9f 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -33,8 +33,8 @@ import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import javax.annotation.Nullable; +import java.util.ArrayList; import java.util.Collections; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; @@ -293,7 +293,7 @@ private static int dropForTier( { int numDropped = 0; - final List droppedHolders = new LinkedList<>(); + final List droppedHolders = new ArrayList<>(); while (numDropped < numToDrop) { final ServerHolder holder = holdersInTier.pollLast(); if (holder == null) { From 849022ed86e0e697f27dcf4dc6da66ec0f8899fc Mon Sep 17 00:00:00 2001 From: Wei Date: Tue, 19 Sep 2017 20:15:30 -0700 Subject: [PATCH 17/33] Change MinMaxPriorityQueue in DruidCluster to TreeSet. --- .../server/coordinator/DruidCluster.java | 21 +- .../coordinator/SegmentReplicantLookup.java | 4 +- .../server/coordinator/ServerHolder.java | 32 +- .../helper/DruidCoordinatorBalancer.java | 6 +- .../DruidCoordinatorCleanupOvershadowed.java | 5 +- .../DruidCoordinatorCleanupUnneeded.java | 5 +- .../helper/DruidCoordinatorLogger.java | 4 +- .../server/coordinator/rules/LoadRule.java | 20 +- .../server/coordinator/DruidClusterTest.java | 40 +- .../DruidCoordinatorBalancerProfiler.java | 51 +- .../DruidCoordinatorBalancerTest.java | 20 +- .../DruidCoordinatorRuleRunnerTest.java | 530 ++++++++---------- ...uidCoordinatorCleanupOvershadowedTest.java | 43 +- .../rules/BroadcastDistributionRuleTest.java | 30 +- .../coordinator/rules/LoadRuleTest.java | 255 ++++----- 15 files changed, 532 insertions(+), 534 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/DruidCluster.java b/server/src/main/java/io/druid/server/coordinator/DruidCluster.java index 55b01eb21732..b034c24bae59 100644 --- a/server/src/main/java/io/druid/server/coordinator/DruidCluster.java +++ b/server/src/main/java/io/druid/server/coordinator/DruidCluster.java @@ -20,19 +20,20 @@ package io.druid.server.coordinator; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.MinMaxPriorityQueue; -import com.google.common.collect.Ordering; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.IAE; import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.NavigableSet; import java.util.Set; +import java.util.TreeSet; /** * Contains a representation of the current state of the cluster by tier. @@ -41,7 +42,7 @@ public class DruidCluster { private final Set realtimes; - private final Map> historicals; + private final Map> historicals; public DruidCluster() { @@ -52,7 +53,7 @@ public DruidCluster() @VisibleForTesting public DruidCluster( @Nullable Set realtimes, - Map> historicals + Map> historicals ) { this.realtimes = realtimes == null ? new HashSet<>() : new HashSet<>(realtimes); @@ -86,9 +87,9 @@ private void addRealtime(ServerHolder serverHolder) private void addHistorical(ServerHolder serverHolder) { final ImmutableDruidServer server = serverHolder.getServer(); - final MinMaxPriorityQueue tierServers = historicals.computeIfAbsent( + final NavigableSet tierServers = historicals.computeIfAbsent( server.getTier(), - k -> MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create() + k -> new TreeSet<>(Collections.reverseOrder()) ); tierServers.add(serverHolder); } @@ -98,7 +99,7 @@ public Set getRealtimes() return realtimes; } - public Map> getHistoricals() + public Map> getHistoricals() { return historicals; } @@ -108,7 +109,7 @@ public Iterable getTierNames() return historicals.keySet(); } - public MinMaxPriorityQueue getHistoricalsByTier(String tier) + public NavigableSet getHistoricalsByTier(String tier) { return historicals.get(tier); } @@ -124,7 +125,7 @@ public Collection getAllServers() return allServers; } - public Iterable> getSortedHistoricalsByTier() + public Iterable> getSortedHistoricalsByTier() { return historicals.values(); } @@ -146,7 +147,7 @@ public boolean hasRealtimes() public boolean hasTier(String tier) { - MinMaxPriorityQueue servers = historicals.get(tier); + NavigableSet servers = historicals.get(tier); return (servers != null) && !servers.isEmpty(); } } diff --git a/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java b/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java index 785fce77feeb..c3ced75e93af 100644 --- a/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java +++ b/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java @@ -21,12 +21,12 @@ import com.google.common.collect.HashBasedTable; import com.google.common.collect.Maps; -import com.google.common.collect.MinMaxPriorityQueue; import com.google.common.collect.Table; import io.druid.client.ImmutableDruidServer; import io.druid.timeline.DataSegment; import java.util.Map; +import java.util.NavigableSet; /** * A lookup for the number of replicants of a given segment for a certain tier. @@ -38,7 +38,7 @@ public static SegmentReplicantLookup make(DruidCluster cluster) final Table segmentsInCluster = HashBasedTable.create(); final Table loadingSegments = HashBasedTable.create(); - for (MinMaxPriorityQueue serversByType : cluster.getSortedHistoricalsByTier()) { + for (NavigableSet serversByType : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serversByType) { ImmutableDruidServer server = serverHolder.getServer(); diff --git a/server/src/main/java/io/druid/server/coordinator/ServerHolder.java b/server/src/main/java/io/druid/server/coordinator/ServerHolder.java index ad4fd7379dc6..ada6451a71ef 100644 --- a/server/src/main/java/io/druid/server/coordinator/ServerHolder.java +++ b/server/src/main/java/io/druid/server/coordinator/ServerHolder.java @@ -19,6 +19,7 @@ package io.druid.server.coordinator; +import com.google.common.primitives.Longs; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.logger.Logger; import io.druid.timeline.DataSegment; @@ -52,32 +53,32 @@ public LoadQueuePeon getPeon() return peon; } - public Long getMaxSize() + public long getMaxSize() { return server.getMaxSize(); } - public Long getCurrServerSize() + public long getCurrServerSize() { return server.getCurrSize(); } - public Long getLoadQueueSize() + public long getLoadQueueSize() { return peon.getLoadQueueSize(); } - public Long getSizeUsed() + public long getSizeUsed() { return getCurrServerSize() + getLoadQueueSize(); } - public Double getPercentUsed() + public double getPercentUsed() { - return (100 * getSizeUsed().doubleValue()) / getMaxSize(); + return (100.0 * getSizeUsed()) / getMaxSize(); } - public Long getAvailableSize() + public long getAvailableSize() { long maxSize = getMaxSize(); long sizeUsed = getSizeUsed(); @@ -114,7 +115,22 @@ public int getNumberOfSegmentsInQueue() @Override public int compareTo(ServerHolder serverHolder) { - return getAvailableSize().compareTo(serverHolder.getAvailableSize()); + int result = Longs.compare(getAvailableSize(), serverHolder.getAvailableSize()); + if (result != 0) { + return result; + } + + result = server.getHost().compareTo(serverHolder.server.getHost()); + if (result != 0) { + return result; + } + + result = server.getTier().compareTo(serverHolder.server.getTier()); + if (result != 0) { + return result; + } + + return server.getType().compareTo(serverHolder.server.getType()); } @Override diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java index 27360fcd7495..5f45212a3274 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java @@ -20,7 +20,6 @@ package io.druid.server.coordinator.helper; import com.google.common.collect.Lists; -import com.google.common.collect.MinMaxPriorityQueue; import com.metamx.emitter.EmittingLogger; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.StringUtils; @@ -38,6 +37,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.NavigableSet; import java.util.concurrent.ConcurrentHashMap; /** @@ -78,7 +78,7 @@ protected void reduceLifetimes(String tier) public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) { final CoordinatorStats stats = new CoordinatorStats(); - params.getDruidCluster().getHistoricals().forEach((String tier, MinMaxPriorityQueue servers) -> { + params.getDruidCluster().getHistoricals().forEach((String tier, NavigableSet servers) -> { balanceTier(params, tier, servers, stats); }); return params.buildFromExisting().withCoordinatorStats(stats).build(); @@ -87,7 +87,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) private void balanceTier( DruidCoordinatorRuntimeParams params, String tier, - MinMaxPriorityQueue servers, + NavigableSet servers, CoordinatorStats stats ) { diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java index 2819a51ec4cd..641b024e5093 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java @@ -20,8 +20,6 @@ package io.druid.server.coordinator.helper; import com.google.common.collect.Maps; -import com.google.common.collect.MinMaxPriorityQueue; - import io.druid.client.ImmutableDruidDataSource; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.guava.Comparators; @@ -34,6 +32,7 @@ import io.druid.timeline.VersionedIntervalTimeline; import java.util.Map; +import java.util.NavigableSet; public class DruidCoordinatorCleanupOvershadowed implements DruidCoordinatorHelper { @@ -55,7 +54,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) DruidCluster cluster = params.getDruidCluster(); Map> timelines = Maps.newHashMap(); - for (MinMaxPriorityQueue serverHolders : cluster.getSortedHistoricalsByTier()) { + for (NavigableSet serverHolders : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serverHolders) { ImmutableDruidServer server = serverHolder.getServer(); diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java index 89b4deb3175e..ea28d43c95c6 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java @@ -19,8 +19,6 @@ package io.druid.server.coordinator.helper; -import com.google.common.collect.MinMaxPriorityQueue; - import io.druid.client.ImmutableDruidDataSource; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.logger.Logger; @@ -33,6 +31,7 @@ import io.druid.server.coordinator.ServerHolder; import io.druid.timeline.DataSegment; +import java.util.NavigableSet; import java.util.Set; /** @@ -64,7 +63,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) // This is done to prevent a race condition in which the coordinator would drop all segments if it started running // cleanup before it finished polling the metadata storage for available segments for the first time. if (!availableSegments.isEmpty()) { - for (MinMaxPriorityQueue serverHolders : cluster.getSortedHistoricalsByTier()) { + for (NavigableSet serverHolders : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serverHolders) { ImmutableDruidServer server = serverHolder.getServer(); diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java index 4ab3870c0f1e..42fd902cf04d 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java @@ -19,7 +19,6 @@ package io.druid.server.coordinator.helper; -import com.google.common.collect.MinMaxPriorityQueue; import com.metamx.emitter.service.ServiceEmitter; import com.metamx.emitter.service.ServiceMetricEvent; import io.druid.client.DruidDataSource; @@ -36,6 +35,7 @@ import it.unimi.dsi.fastutil.objects.Object2LongMap; import java.util.List; +import java.util.NavigableSet; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -171,7 +171,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) ); log.info("Load Queues:"); - for (MinMaxPriorityQueue serverHolders : cluster.getSortedHistoricalsByTier()) { + for (NavigableSet serverHolders : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serverHolders) { ImmutableDruidServer server = serverHolder.getServer(); LoadQueuePeon queuePeon = serverHolder.getPeon(); diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 73eeaf071b9f..d391f6ce7dd1 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -19,7 +19,6 @@ package io.druid.server.coordinator.rules; -import com.google.common.collect.MinMaxPriorityQueue; import com.metamx.emitter.EmittingLogger; import io.druid.java.util.common.IAE; import io.druid.server.coordinator.CoordinatorStats; @@ -33,10 +32,11 @@ import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import javax.annotation.Nullable; -import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.NavigableSet; import java.util.Objects; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -130,7 +130,7 @@ private static List getFilteredHolders( final Predicate predicate ) { - final MinMaxPriorityQueue queue = druidCluster.getHistoricalsByTier(tier); + final NavigableSet queue = druidCluster.getHistoricalsByTier(tier); if (queue == null) { log.makeAlert("Tier[%s] has no servers! Check your cluster configuration!", tier).emit(); return Collections.emptyList(); @@ -269,7 +269,7 @@ private static void drop( for (final Object2IntMap.Entry entry : currentReplicants.object2IntEntrySet()) { final String tier = entry.getKey(); - final MinMaxPriorityQueue holders = druidCluster.getHistoricalsByTier(tier); + final NavigableSet holders = druidCluster.getHistoricalsByTier(tier); final int numDropped; if (holders == null) { @@ -287,28 +287,26 @@ private static void drop( private static int dropForTier( final int numToDrop, - final MinMaxPriorityQueue holdersInTier, + final NavigableSet holdersInTier, final DataSegment segment ) { int numDropped = 0; - final List droppedHolders = new ArrayList<>(); + final Iterator iterator = holdersInTier.descendingIterator(); while (numDropped < numToDrop) { - final ServerHolder holder = holdersInTier.pollLast(); - if (holder == null) { + if (!iterator.hasNext()) { log.warn("Wtf, holder was null? I have no servers serving [%s]?", segment.getIdentifier()); break; } + final ServerHolder holder = iterator.next(); + if (holder.isServingSegment(segment)) { holder.getPeon().dropSegment(segment, null); ++numDropped; } - droppedHolders.add(holder); } - // add back the holders - holdersInTier.addAll(droppedHolders); return numDropped; } diff --git a/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java b/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java index 18acacaa782a..d705a70ce3e9 100644 --- a/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java +++ b/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java @@ -22,8 +22,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.MinMaxPriorityQueue; -import com.google.common.collect.Ordering; import io.druid.client.ImmutableDruidDataSource; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.Intervals; @@ -36,10 +34,14 @@ import org.junit.Test; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.NavigableSet; import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; +import java.util.stream.Stream; public class DruidClusterTest { @@ -142,25 +144,23 @@ public void setup() ), ImmutableMap.of( "tier1", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - ImmutableList.of( - new ServerHolder( - new ImmutableDruidServer( - new DruidServerMetadata("name1", "host1", null, 100L, ServerType.HISTORICAL, "tier1", 0), - 0L, - ImmutableMap.of( - "src1", - dataSources.get("src1") - ), - ImmutableMap.of( - "segment1", - segments.get(0) - ) + Stream.of( + new ServerHolder( + new ImmutableDruidServer( + new DruidServerMetadata("name1", "host1", null, 100L, ServerType.HISTORICAL, "tier1", 0), + 0L, + ImmutableMap.of( + "src1", + dataSources.get("src1") ), - new LoadQueuePeonTester() - ) + ImmutableMap.of( + "segment1", + segments.get(0) + ) + ), + new LoadQueuePeonTester() ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); } @@ -186,7 +186,7 @@ public void testGetAllServers() cluster.add(newRealtime); cluster.add(newHistorical); final Set expectedRealtimes = cluster.getRealtimes(); - final Map> expectedHistoricals = cluster.getHistoricals(); + final Map> expectedHistoricals = cluster.getHistoricals(); final Collection allServers = cluster.getAllServers(); Assert.assertEquals(4, allServers.size()); diff --git a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerProfiler.java b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerProfiler.java index 8a67377377fd..b49445905f47 100644 --- a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerProfiler.java +++ b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerProfiler.java @@ -24,13 +24,13 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.collect.MinMaxPriorityQueue; import com.metamx.emitter.EmittingLogger; import com.metamx.emitter.service.ServiceEmitter; import io.druid.client.DruidServer; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.DateTimes; import io.druid.metadata.MetadataRuleManager; +import io.druid.server.coordinator.helper.DruidCoordinatorBalancer; import io.druid.server.coordinator.helper.DruidCoordinatorRuleRunner; import io.druid.server.coordinator.rules.PeriodLoadRule; import io.druid.server.coordinator.rules.Rule; @@ -41,10 +41,12 @@ import org.joda.time.Period; import org.junit.Before; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeSet; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class DruidCoordinatorBalancerProfiler { @@ -135,12 +137,15 @@ public void bigProfiler() .withDruidCluster( new DruidCluster( null, - ImmutableMap.>of( + ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(DruidCoordinatorBalancerTester.percentUsedComparator) - .create( - serverHolderList - ) + serverHolderList.stream().collect( + Collectors.toCollection( + () -> new TreeSet<>( + DruidCoordinatorBalancer.percentUsedComparator + ) + ) + ) ) ) ) @@ -163,12 +168,15 @@ public void bigProfiler() SegmentReplicantLookup.make( new DruidCluster( null, - ImmutableMap.>of( + ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(DruidCoordinatorBalancerTester.percentUsedComparator) - .create( - serverHolderList - ) + serverHolderList.stream().collect( + Collectors.toCollection( + () -> new TreeSet<>( + DruidCoordinatorBalancer.percentUsedComparator + ) + ) + ) ) ) ) @@ -219,15 +227,18 @@ public void profileRun() .withDruidCluster( new DruidCluster( null, - ImmutableMap.>of( + ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(DruidCoordinatorBalancerTester.percentUsedComparator) - .create( - Arrays.asList( - new ServerHolder(druidServer1, fromPeon), - new ServerHolder(druidServer2, toPeon) - ) - ) + Stream.of( + new ServerHolder(druidServer1, fromPeon), + new ServerHolder(druidServer2, toPeon) + ).collect( + Collectors.toCollection( + () -> new TreeSet<>( + DruidCoordinatorBalancer.percentUsedComparator + ) + ) + ) ) ) ) diff --git a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerTest.java b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerTest.java index 66a041744ecc..ec0b841fbcd2 100644 --- a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerTest.java +++ b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorBalancerTest.java @@ -23,11 +23,11 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.collect.MinMaxPriorityQueue; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.DateTimes; +import io.druid.server.coordinator.helper.DruidCoordinatorBalancer; import io.druid.timeline.DataSegment; import io.druid.timeline.partition.NoneShardSpec; import org.easymock.EasyMock; @@ -43,6 +43,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeSet; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -284,13 +285,14 @@ private DruidCoordinatorRuntimeParams.Builder defaullRuntimeParamsBuilder( null, ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(DruidCoordinatorBalancerTester.percentUsedComparator) - .create( - IntStream - .range(0, druidServers.size()) - .mapToObj(i -> new ServerHolder(druidServers.get(i), peons.get(i))) - .collect(Collectors.toList()) - ) + IntStream + .range(0, druidServers.size()) + .mapToObj(i -> new ServerHolder(druidServers.get(i), peons.get(i))) + .collect( + Collectors.toCollection( + () -> new TreeSet<>(DruidCoordinatorBalancer.percentUsedComparator) + ) + ) ) ) ) @@ -319,7 +321,7 @@ private void mockDruidServer( Map segments ) { - EasyMock.expect(druidServer.getName()).andReturn(name).atLeastOnce(); + EasyMock.expect(druidServer.getName()).andReturn(name).anyTimes(); EasyMock.expect(druidServer.getTier()).andReturn(tier).anyTimes(); EasyMock.expect(druidServer.getCurrSize()).andReturn(currentSize).atLeastOnce(); EasyMock.expect(druidServer.getMaxSize()).andReturn(maxSize).atLeastOnce(); diff --git a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java index 0cf1be38381a..7357c9cdf722 100644 --- a/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java +++ b/server/src/test/java/io/druid/server/coordinator/DruidCoordinatorRuleRunnerTest.java @@ -22,8 +22,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.collect.MinMaxPriorityQueue; -import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; @@ -52,10 +50,14 @@ import org.junit.Test; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.Executors; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** */ @@ -143,56 +145,50 @@ public void testRunThreeTiersOneReplicant() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverNorm", - "hostNorm", - null, - 1000, - ServerType.HISTORICAL, - "normal", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverNorm", + "hostNorm", + null, + 1000, + ServerType.HISTORICAL, + "normal", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), "cold", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverCold", - "hostCold", - null, - 1000, - ServerType.HISTORICAL, - "cold", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverCold", + "hostCold", + null, + 1000, + ServerType.HISTORICAL, + "cold", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -258,51 +254,47 @@ public void testRunTwoTiersTwoReplicants() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - mockPeon - ), - new ServerHolder( - new DruidServer( - "serverHot2", - "hostHot2", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + mockPeon + ), + new ServerHolder( + new DruidServer( + "serverHot2", + "hostHot2", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), "cold", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverCold", - "hostCold", - null, - 1000, - ServerType.HISTORICAL, - "cold", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverCold", + "hostCold", + null, + 1000, + ServerType.HISTORICAL, + "cold", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -379,31 +371,27 @@ public void testRunTwoTiersWithExistingSegments() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - normServer.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + normServer.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -466,22 +454,20 @@ public void testRunTwoTiersTierDoesNotExist() throws Exception null, ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverNorm", - "hostNorm", - null, - 1000, - ServerType.HISTORICAL, - "normal", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverNorm", + "hostNorm", + null, + 1000, + ServerType.HISTORICAL, + "normal", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -524,28 +510,28 @@ public void testRunRuleDoesNotExist() throws Exception ) ) ).atLeastOnce(); - EasyMock.replay(databaseRuleManager); + + EasyMock.expect(mockPeon.getLoadQueueSize()).andReturn(0L).anyTimes(); + EasyMock.replay(databaseRuleManager, mockPeon); DruidCluster druidCluster = new DruidCluster( null, ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverNorm", - "hostNorm", - null, - 1000, - ServerType.HISTORICAL, - "normal", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverNorm", + "hostNorm", + null, + 1000, + ServerType.HISTORICAL, + "normal", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -560,7 +546,7 @@ public void testRunRuleDoesNotExist() throws Exception ruleRunner.run(params); - EasyMock.verify(emitter); + EasyMock.verify(emitter, mockPeon); } @Test @@ -605,14 +591,12 @@ public void testDropRemove() throws Exception null, ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -689,18 +673,16 @@ public void testDropTooManyInSameTier() throws Exception null, ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server1.toImmutableDruidServer(), - mockPeon - ), - new ServerHolder( - server2.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server1.toImmutableDruidServer(), + mockPeon + ), + new ServerHolder( + server2.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -779,23 +761,19 @@ public void testDropTooManyInDifferentTiers() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( + Stream.of( new ServerHolder( server1.toImmutableDruidServer(), mockPeon - ) ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server2.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server2.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -870,23 +848,19 @@ public void testDontDropInDifferentTiers() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server1.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server1.toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server2.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server2.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -976,22 +950,20 @@ public void testDropServerActuallyServesSegment() throws Exception null, ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server1.toImmutableDruidServer(), - mockPeon - ), - new ServerHolder( - server2.toImmutableDruidServer(), - anotherMockPeon - ), - new ServerHolder( - server3.toImmutableDruidServer(), - anotherMockPeon - ) + Stream.of( + new ServerHolder( + server1.toImmutableDruidServer(), + mockPeon + ), + new ServerHolder( + server2.toImmutableDruidServer(), + anotherMockPeon + ), + new ServerHolder( + server3.toImmutableDruidServer(), + anotherMockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -1050,34 +1022,32 @@ public void testReplicantThrottle() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - mockPeon - ), - new ServerHolder( - new DruidServer( - "serverHot2", - "hostHot2", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + mockPeon + ), + new ServerHolder( + new DruidServer( + "serverHot2", + "hostHot2", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -1178,39 +1148,35 @@ public void testReplicantThrottleAcrossTiers() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 1 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 1 + ).toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), DruidServer.DEFAULT_TIER, - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverNorm", - "hostNorm", - null, - 1000, - ServerType.HISTORICAL, - DruidServer.DEFAULT_TIER, - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverNorm", + "hostNorm", + null, + 1000, + ServerType.HISTORICAL, + DruidServer.DEFAULT_TIER, + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -1303,18 +1269,16 @@ public void testDropReplicantThrottle() throws Exception null, ImmutableMap.of( "normal", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server1.toImmutableDruidServer(), - mockPeon - ), - new ServerHolder( - server2.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server1.toImmutableDruidServer(), + mockPeon + ), + new ServerHolder( + server2.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -1388,22 +1352,20 @@ public void testRulesRunOnNonOvershadowedSegmentsOnly() throws Exception null, ImmutableMap.of( DruidServer.DEFAULT_TIER, - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - DruidServer.DEFAULT_TIER, - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + DruidServer.DEFAULT_TIER, + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); diff --git a/server/src/test/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowedTest.java b/server/src/test/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowedTest.java index cdcdf436d907..839d5b8ec43a 100644 --- a/server/src/test/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowedTest.java +++ b/server/src/test/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowedTest.java @@ -22,11 +22,10 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.MinMaxPriorityQueue; -import com.google.common.collect.Ordering; import io.druid.client.ImmutableDruidDataSource; import io.druid.client.ImmutableDruidServer; import io.druid.java.util.common.DateTimes; +import io.druid.server.coordination.ServerType; import io.druid.server.coordinator.CoordinatorStats; import io.druid.server.coordinator.DruidCluster; import io.druid.server.coordinator.DruidCoordinator; @@ -41,6 +40,9 @@ import java.util.Collections; import java.util.List; +import java.util.TreeSet; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class DruidCoordinatorCleanupOvershadowedTest { @@ -71,11 +73,28 @@ public void testRun() druidCoordinatorCleanupOvershadowed = new DruidCoordinatorCleanupOvershadowed(coordinator); availableSegments = ImmutableList.of(segmentV1, segmentV0, segmentV2); - druidCluster = new DruidCluster( - null, - ImmutableMap.of("normal", MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Collections.singletonList(new ServerHolder(druidServer, mockPeon)) - ))); + // Dummy values for comparisons in TreeSet + EasyMock.expect(mockPeon.getLoadQueueSize()) + .andReturn(0L) + .anyTimes(); + EasyMock.expect(druidServer.getMaxSize()) + .andReturn(0L) + .anyTimes(); + EasyMock.expect(druidServer.getCurrSize()) + .andReturn(0L) + .anyTimes(); + EasyMock.expect(druidServer.getName()) + .andReturn("") + .anyTimes(); + EasyMock.expect(druidServer.getHost()) + .andReturn("") + .anyTimes(); + EasyMock.expect(druidServer.getTier()) + .andReturn("") + .anyTimes(); + EasyMock.expect(druidServer.getType()) + .andReturn(ServerType.HISTORICAL) + .anyTimes(); EasyMock.expect(druidServer.getDataSources()) .andReturn(ImmutableList.of(druidDataSource)) @@ -88,6 +107,16 @@ public void testRun() coordinator.removeSegment(segmentV0); EasyMock.expectLastCall(); EasyMock.replay(mockPeon, coordinator, druidServer, druidDataSource); + + druidCluster = new DruidCluster( + null, + ImmutableMap.of( + "normal", + Stream.of( + new ServerHolder(druidServer, mockPeon) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) + )); + DruidCoordinatorRuntimeParams params = DruidCoordinatorRuntimeParams.newBuilder() .withAvailableSegments(availableSegments) .withCoordinatorStats(new CoordinatorStats()) diff --git a/server/src/test/java/io/druid/server/coordinator/rules/BroadcastDistributionRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/BroadcastDistributionRuleTest.java index f4719a141b93..4da23cdd801d 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/BroadcastDistributionRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/BroadcastDistributionRuleTest.java @@ -23,8 +23,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.collect.MinMaxPriorityQueue; -import com.google.common.collect.Ordering; import io.druid.client.DruidServer; import io.druid.java.util.common.DateTimes; import io.druid.java.util.common.Intervals; @@ -40,7 +38,11 @@ import org.junit.Before; import org.junit.Test; +import java.util.Collections; import java.util.List; +import java.util.TreeSet; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -198,21 +200,17 @@ public void setUp() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Lists.newArrayList( - holdersOfLargeSegments.get(0), - holderOfSmallSegment, - holdersOfLargeSegments2.get(0) - ) - ), + Stream.of( + holdersOfLargeSegments.get(0), + holderOfSmallSegment, + holdersOfLargeSegments2.get(0) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), DruidServer.DEFAULT_TIER, - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Lists.newArrayList( - holdersOfLargeSegments.get(1), - holdersOfLargeSegments.get(2), - holdersOfLargeSegments2.get(1) - ) - ) + Stream.of( + holdersOfLargeSegments.get(1), + holdersOfLargeSegments.get(2), + holdersOfLargeSegments2.get(1) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); } diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index f260805a4244..4fdaa065f860 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -23,8 +23,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.collect.MinMaxPriorityQueue; -import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; @@ -63,7 +61,10 @@ import java.util.Arrays; import java.util.Collections; import java.util.Map; +import java.util.TreeSet; import java.util.concurrent.Executors; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** */ @@ -119,53 +120,49 @@ public void testLoad() throws Exception DruidServer.DEFAULT_TIER, 2 )); + final DataSegment segment = createDataSegment("foo"); + + throttler.registerReplicantCreation(DruidServer.DEFAULT_TIER, segment.getIdentifier(), "hostNorm"); + EasyMock.expectLastCall().once(); + + EasyMock.replay(throttler, mockPeon); + DruidCluster druidCluster = new DruidCluster( null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 1 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 1 + ).toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), DruidServer.DEFAULT_TIER, - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverNorm", - "hostNorm", - null, - 1000, - ServerType.HISTORICAL, - DruidServer.DEFAULT_TIER, - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverNorm", + "hostNorm", + null, + 1000, + ServerType.HISTORICAL, + DruidServer.DEFAULT_TIER, + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); - final DataSegment segment = createDataSegment("foo"); - - throttler.registerReplicantCreation(DruidServer.DEFAULT_TIER, segment.getIdentifier(), "hostNorm"); - EasyMock.expectLastCall().once(); - - EasyMock.replay(throttler, mockPeon); - CoordinatorStats stats = rule.run( null, DruidCoordinatorRuntimeParams.newBuilder() @@ -206,51 +203,47 @@ public void testLoadPriority() throws Exception null, ImmutableMap.of( "tier1", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "server1", - "host1", - null, - 1000, - ServerType.HISTORICAL, - "tier1", - 0 - ).toImmutableDruidServer(), - mockPeon1 - ) + Stream.of( + new ServerHolder( + new DruidServer( + "server1", + "host1", + null, + 1000, + ServerType.HISTORICAL, + "tier1", + 0 + ).toImmutableDruidServer(), + mockPeon1 ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), "tier2", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "server2", - "host2", - null, - 1000, - ServerType.HISTORICAL, - "tier2", - 1 - ).toImmutableDruidServer(), - mockPeon2 - ), - new ServerHolder( - new DruidServer( - "server3", - "host3", - null, - 1000, - ServerType.HISTORICAL, - "tier2", - 1 - ).toImmutableDruidServer(), - mockPeon2 - ) + Stream.of( + new ServerHolder( + new DruidServer( + "server2", + "host2", + null, + 1000, + ServerType.HISTORICAL, + "tier2", + 1 + ).toImmutableDruidServer(), + mockPeon2 + ), + new ServerHolder( + new DruidServer( + "server3", + "host3", + null, + 1000, + ServerType.HISTORICAL, + "tier2", + 1 + ).toImmutableDruidServer(), + mockPeon2 ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -313,23 +306,19 @@ public void testDrop() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server1.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server1.toImmutableDruidServer(), + mockPeon ) - ), + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))), DruidServer.DEFAULT_TIER, - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server2.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server2.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -368,22 +357,20 @@ public void testLoadWithNonExistentTier() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -446,18 +433,16 @@ public void testDropWithNonExistentTier() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - server1.toImmutableDruidServer(), - mockPeon - ), - new ServerHolder( - server2.toImmutableDruidServer(), - mockPeon - ) + Stream.of( + new ServerHolder( + server1.toImmutableDruidServer(), + mockPeon + ), + new ServerHolder( + server2.toImmutableDruidServer(), + mockPeon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); @@ -493,22 +478,20 @@ public void testMaxLoadingQueueSize() throws Exception null, ImmutableMap.of( "hot", - MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( - Arrays.asList( - new ServerHolder( - new DruidServer( - "serverHot", - "hostHot", - null, - 1000, - ServerType.HISTORICAL, - "hot", - 0 - ).toImmutableDruidServer(), - peon - ) + Stream.of( + new ServerHolder( + new DruidServer( + "serverHot", + "hostHot", + null, + 1000, + ServerType.HISTORICAL, + "hot", + 0 + ).toImmutableDruidServer(), + peon ) - ) + ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) ) ); From 4e465019fa3b6ba1a247a52e5ac48053822fc604 Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 20 Sep 2017 11:30:27 -0700 Subject: [PATCH 18/33] Adding some comments. --- .../java/io/druid/server/coordinator/rules/LoadRule.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index d391f6ce7dd1..5b7968ff0cc2 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -94,9 +94,10 @@ private static void assign( final String tier = primaryHolderToLoad.getServer().getTier(); // assign replicas for the rest of the tier - final int numAssigned = 1 /* primary */ + assignReplicasForTier( + final int numAssigned = 1 /* for primary replica */ + assignReplicasForTier( tier, targetReplicants.getOrDefault(tier, 0), + // note: adding 1 to currentReplicantsInTier to account for the one assigned as primary replica currentReplicants.getOrDefault(tier, 0) + 1, params, getFilteredHolders( @@ -108,7 +109,8 @@ private static void assign( ); stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); - assignReplicas(targetReplicants, currentReplicants, params, segment, stats, tier); + // do assign replicas for the other tiers. + assignReplicas(targetReplicants, currentReplicants, params, segment, stats, tier /* to skip */); } } From aef9fc760f26c6c4ba33da14fb389603821a476a Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 20 Sep 2017 11:37:19 -0700 Subject: [PATCH 19/33] Modify log messages in light of predicates. --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 5b7968ff0cc2..cdb2236d50dc 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -169,7 +169,7 @@ private static ServerHolder assignPrimary( final ServerHolder candidate = params.getBalancerStrategy().findNewSegmentHomeReplicator(segment, holders); if (candidate == null) { log.warn( - "Not enough [%s] servers or node capacity to assign primary segment[%s]! Expected Replicants[%d]", + "No available [%s] servers or node capacity to assign primary segment[%s]! Expected Replicants[%d]", tier, segment.getIdentifier(), targetReplicantsInTier ); } else if (topCandidate == null || candidate.getServer().getPriority() > topCandidate.getServer().getPriority()) { @@ -233,7 +233,7 @@ private static int assignReplicasForTier( final ServerHolder holder = params.getBalancerStrategy().findNewSegmentHomeReplicator(segment, holders); if (holder == null) { log.warn( - "Not enough [%s] servers or node capacity to assign segment[%s]! Expected Replicants[%d]", + "No available [%s] servers or node capacity to assign segment[%s]! Expected Replicants[%d]", tier, segment.getIdentifier(), targetReplicantsInTier ); return numAssigned; From 9a4766d2cbf9dff661432e6a36dc00b3a83f9b3b Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 20 Sep 2017 16:54:55 -0700 Subject: [PATCH 20/33] Add in-method comments --- .../server/coordinator/rules/LoadRule.java | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index cdb2236d50dc..85672df7f978 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -57,6 +57,7 @@ public CoordinatorStats run( final DataSegment segment ) { + // get the "snapshots" of targetReplicants and currentReplicants for assignments. final Object2IntMap targetReplicants = new Object2IntOpenHashMap<>(getTieredReplicants()); final Object2IntMap currentReplicants = new Object2IntOpenHashMap<>( @@ -64,6 +65,8 @@ public CoordinatorStats run( ); final CoordinatorStats stats = new CoordinatorStats(); + + // performs if (params.getAvailableSegments().contains(segment)) { assign(targetReplicants, currentReplicants, params, segment, stats); } @@ -141,6 +144,14 @@ private static List getFilteredHolders( return queue.stream().filter(predicate).collect(Collectors.toList()); } + /*** + * Iterates through each tier and find the respective segment homes; with the found segment homes, selects the one + * with the highest priority to be the holder for the primary replica. + * @param targetReplicants + * @param params + * @param segment + * @return + */ @Nullable private static ServerHolder assignPrimary( final Object2IntMap targetReplicants, @@ -151,6 +162,7 @@ private static ServerHolder assignPrimary( ServerHolder topCandidate = null; for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { final int targetReplicantsInTier = entry.getIntValue(); + // sanity check: target number of replicants should be more than zero. if (targetReplicantsInTier <= 0) { continue; } @@ -162,6 +174,7 @@ private static ServerHolder assignPrimary( params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params) ); + // no holders satisfy the predicate if (holders.isEmpty()) { continue; } @@ -169,10 +182,14 @@ private static ServerHolder assignPrimary( final ServerHolder candidate = params.getBalancerStrategy().findNewSegmentHomeReplicator(segment, holders); if (candidate == null) { log.warn( - "No available [%s] servers or node capacity to assign primary segment[%s]! Expected Replicants[%d]", + "No available [%s] servers or node capacity to assign primary segment[%s]! " + + "Expected Replicants[%d]", tier, segment.getIdentifier(), targetReplicantsInTier ); - } else if (topCandidate == null || candidate.getServer().getPriority() > topCandidate.getServer().getPriority()) { + } else if ( + topCandidate == null || + candidate.getServer().getPriority() > topCandidate.getServer().getPriority() + ) { topCandidate = candidate; } } @@ -184,6 +201,16 @@ private static ServerHolder assignPrimary( return topCandidate; } + /*** + * + * @param targetReplicants + * @param currentReplicants + * @param params + * @param segment + * @param stats + * @param tierToSkip if not null, this tier will be skipped from doing assignment, use when primary replica was + * assgined. + */ private static void assignReplicas( final Object2IntMap targetReplicants, final Object2IntMap currentReplicants, @@ -220,6 +247,7 @@ private static int assignReplicasForTier( ) { final int numToAssign = targetReplicantsInTier - currentReplicantsInTier; + // if nothing to assign or no holders available for assignment if (numToAssign <= 0 || holders.isEmpty()) { return 0; } @@ -260,6 +288,7 @@ private static void drop( final DruidCluster druidCluster = params.getDruidCluster(); // Make sure we have enough loaded replicants in the correct tiers in the cluster before doing anything + // This enforces that loading is completed before we attempt to drop stuffs as a safety measure for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { final String tier = entry.getKey(); // if there are replicants loading in cluster @@ -295,6 +324,7 @@ private static int dropForTier( { int numDropped = 0; + // Use the reverse order to get the holders with least available size first. final Iterator iterator = holdersInTier.descendingIterator(); while (numDropped < numToDrop) { if (!iterator.hasNext()) { From 40cd580f94c77cc8dff8dd49ca48962e3991c09b Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 20 Sep 2017 17:19:25 -0700 Subject: [PATCH 21/33] Don't create new Object2IntOpenHashMap for each run() call. --- .../server/coordinator/rules/LoadRule.java | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 85672df7f978..0fc3ca3a26b8 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -50,6 +50,9 @@ public abstract class LoadRule implements Rule static final String ASSIGNED_COUNT = "assignedCount"; static final String DROPPED_COUNT = "droppedCount"; + private final Object2IntMap targetReplicants = new Object2IntOpenHashMap<>(); + private final Object2IntMap currentReplicants = new Object2IntOpenHashMap<>(); + @Override public CoordinatorStats run( final DruidCoordinator coordinator, @@ -57,28 +60,29 @@ public CoordinatorStats run( final DataSegment segment ) { - // get the "snapshots" of targetReplicants and currentReplicants for assignments. - final Object2IntMap targetReplicants = new Object2IntOpenHashMap<>(getTieredReplicants()); - - final Object2IntMap currentReplicants = new Object2IntOpenHashMap<>( - params.getSegmentReplicantLookup().getClusterTiers(segment.getIdentifier()) - ); + try { + // get the "snapshots" of targetReplicants and currentReplicants for assignments. + targetReplicants.putAll(getTieredReplicants()); + currentReplicants.putAll(params.getSegmentReplicantLookup().getClusterTiers(segment.getIdentifier())); - final CoordinatorStats stats = new CoordinatorStats(); + final CoordinatorStats stats = new CoordinatorStats(); - // performs - if (params.getAvailableSegments().contains(segment)) { - assign(targetReplicants, currentReplicants, params, segment, stats); - } + // performs + if (params.getAvailableSegments().contains(segment)) { + assign(params, segment, stats); + } - drop(targetReplicants, currentReplicants, params, segment, stats); + drop(params, segment, stats); - return stats; + return stats; + } + finally { + targetReplicants.clear(); + currentReplicants.clear(); + } } - private static void assign( - final Object2IntMap targetReplicants, - final Object2IntMap currentReplicants, + private void assign( final DruidCoordinatorRuntimeParams params, final DataSegment segment, final CoordinatorStats stats @@ -86,9 +90,9 @@ private static void assign( { // if primary replica already exists if (!currentReplicants.isEmpty()) { - assignReplicas(targetReplicants, currentReplicants, params, segment, stats, null); + assignReplicas(params, segment, stats, null); } else { - final ServerHolder primaryHolderToLoad = assignPrimary(targetReplicants, params, segment); + final ServerHolder primaryHolderToLoad = assignPrimary(params, segment); if (primaryHolderToLoad == null) { // cluster does not have any replicants and cannot identify primary holder // this implies that no assignment could be done @@ -113,7 +117,7 @@ private static void assign( stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); // do assign replicas for the other tiers. - assignReplicas(targetReplicants, currentReplicants, params, segment, stats, tier /* to skip */); + assignReplicas(params, segment, stats, tier /* to skip */); } } @@ -147,14 +151,13 @@ private static List getFilteredHolders( /*** * Iterates through each tier and find the respective segment homes; with the found segment homes, selects the one * with the highest priority to be the holder for the primary replica. - * @param targetReplicants + * * @param params * @param segment * @return */ @Nullable - private static ServerHolder assignPrimary( - final Object2IntMap targetReplicants, + private ServerHolder assignPrimary( final DruidCoordinatorRuntimeParams params, final DataSegment segment ) @@ -203,17 +206,13 @@ private static ServerHolder assignPrimary( /*** * - * @param targetReplicants - * @param currentReplicants * @param params * @param segment * @param stats * @param tierToSkip if not null, this tier will be skipped from doing assignment, use when primary replica was * assgined. */ - private static void assignReplicas( - final Object2IntMap targetReplicants, - final Object2IntMap currentReplicants, + private void assignReplicas( final DruidCoordinatorRuntimeParams params, final DataSegment segment, final CoordinatorStats stats, @@ -277,9 +276,7 @@ private static int assignReplicasForTier( return numToAssign; } - private static void drop( - final Object2IntMap targetReplicants, - final Object2IntMap currentReplicants, + private void drop( final DruidCoordinatorRuntimeParams params, final DataSegment segment, final CoordinatorStats stats From 748112c9790623d479b53419650821167b9b8c1a Mon Sep 17 00:00:00 2001 From: Wei Date: Wed, 20 Sep 2017 18:19:49 -0700 Subject: [PATCH 22/33] Cache result from strategy call in the primary assignment to be reused during the same run. --- .../server/coordinator/rules/LoadRule.java | 31 +++++++--- .../coordinator/rules/LoadRuleTest.java | 56 +++++++++++++------ 2 files changed, 63 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 0fc3ca3a26b8..26791d98046a 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -33,6 +33,7 @@ import javax.annotation.Nullable; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -53,6 +54,9 @@ public abstract class LoadRule implements Rule private final Object2IntMap targetReplicants = new Object2IntOpenHashMap<>(); private final Object2IntMap currentReplicants = new Object2IntOpenHashMap<>(); + // Cache to hold unsued results from strategy call in assignPrimary + private final Map strategyCache = new HashMap<>(); + @Override public CoordinatorStats run( final DruidCoordinator coordinator, @@ -79,6 +83,7 @@ public CoordinatorStats run( finally { targetReplicants.clear(); currentReplicants.clear(); + strategyCache.clear(); } } @@ -189,15 +194,21 @@ private ServerHolder assignPrimary( "Expected Replicants[%d]", tier, segment.getIdentifier(), targetReplicantsInTier ); - } else if ( - topCandidate == null || - candidate.getServer().getPriority() > topCandidate.getServer().getPriority() - ) { - topCandidate = candidate; + } else { + // cache the result for later use. + strategyCache.put(tier, candidate); + if ( + topCandidate == null || + candidate.getServer().getPriority() > topCandidate.getServer().getPriority() + ) { + topCandidate = candidate; + } } } if (topCandidate != null) { + // remove tier for primary replica + strategyCache.remove(topCandidate.getServer().getTier()); topCandidate.getPeon().loadSegment(segment, null); } @@ -236,7 +247,7 @@ private void assignReplicas( } } - private static int assignReplicasForTier( + private int assignReplicasForTier( final String tier, final int targetReplicantsInTier, final int currentReplicantsInTier, @@ -257,7 +268,13 @@ private static int assignReplicasForTier( return numAssigned; } - final ServerHolder holder = params.getBalancerStrategy().findNewSegmentHomeReplicator(segment, holders); + // Retrieves from cache if available + ServerHolder holder = strategyCache.remove(tier); + // Does strategy call if not in cache + if (holder == null) { + holder = params.getBalancerStrategy().findNewSegmentHomeReplicator(segment, holders); + } + if (holder == null) { log.warn( "No available [%s] servers or node capacity to assign segment[%s]! Expected Replicants[%d]", diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index 4fdaa065f860..f32cfe634db4 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -88,6 +88,8 @@ public class LoadRuleTest private ListeningExecutorService exec; private BalancerStrategy balancerStrategy; + private BalancerStrategy mockBalancerStrategy; + @Before public void setUp() throws Exception { @@ -97,6 +99,8 @@ public void setUp() throws Exception exec = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(1)); balancerStrategy = new CostBalancerStrategyFactory().createBalancerStrategy(exec); + + mockBalancerStrategy = EasyMock.createMock(BalancerStrategy.class); } @After @@ -125,7 +129,11 @@ public void testLoad() throws Exception throttler.registerReplicantCreation(DruidServer.DEFAULT_TIER, segment.getIdentifier(), "hostNorm"); EasyMock.expectLastCall().once(); - EasyMock.replay(throttler, mockPeon); + EasyMock.expect(mockBalancerStrategy.findNewSegmentHomeReplicator(EasyMock.anyObject(), EasyMock.anyObject())) + .andDelegateTo(balancerStrategy) + .times(3); + + EasyMock.replay(throttler, mockPeon, mockBalancerStrategy); DruidCluster druidCluster = new DruidCluster( null, @@ -169,7 +177,7 @@ public void testLoad() throws Exception .withDruidCluster(druidCluster) .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) .withReplicationManager(throttler) - .withBalancerStrategy(balancerStrategy) + .withBalancerStrategy(mockBalancerStrategy) .withBalancerReferenceTimestamp(DateTimes.of("2013-01-01")) .withAvailableSegments(Arrays.asList(segment)).build(), segment @@ -178,7 +186,7 @@ public void testLoad() throws Exception Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, DruidServer.DEFAULT_TIER)); - EasyMock.verify(throttler, mockPeon); + EasyMock.verify(throttler, mockPeon, mockBalancerStrategy); } @Test @@ -192,7 +200,11 @@ public void testLoadPriority() throws Exception mockPeon2.loadSegment(EasyMock.anyObject(), EasyMock.isNull()); EasyMock.expectLastCall().once(); - EasyMock.replay(throttler, mockPeon1, mockPeon2); + EasyMock.expect(mockBalancerStrategy.findNewSegmentHomeReplicator(EasyMock.anyObject(), EasyMock.anyObject())) + .andDelegateTo(balancerStrategy) + .times(2); + + EasyMock.replay(throttler, mockPeon1, mockPeon2, mockBalancerStrategy); final LoadRule rule = createLoadRule(ImmutableMap.of( "tier1", 10, @@ -255,7 +267,7 @@ public void testLoadPriority() throws Exception .withDruidCluster(druidCluster) .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) .withReplicationManager(throttler) - .withBalancerStrategy(balancerStrategy) + .withBalancerStrategy(mockBalancerStrategy) .withBalancerReferenceTimestamp(new DateTime("2013-01-01")) .withAvailableSegments(Collections.singletonList(segment)).build(), segment @@ -264,7 +276,7 @@ public void testLoadPriority() throws Exception Assert.assertEquals(0L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "tier1")); Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "tier2")); - EasyMock.verify(throttler, mockPeon1, mockPeon2); + EasyMock.verify(throttler, mockPeon1, mockPeon2, mockBalancerStrategy); } @Test @@ -273,7 +285,7 @@ public void testDrop() throws Exception final LoadQueuePeon mockPeon = createEmptyPeon(); mockPeon.dropSegment(EasyMock.anyObject(), EasyMock.anyObject()); EasyMock.expectLastCall().atLeastOnce(); - EasyMock.replay(throttler, mockPeon); + EasyMock.replay(throttler, mockPeon, mockBalancerStrategy); LoadRule rule = createLoadRule(ImmutableMap.of( "hot", 0, @@ -328,7 +340,7 @@ public void testDrop() throws Exception .withDruidCluster(druidCluster) .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) .withReplicationManager(throttler) - .withBalancerStrategy(balancerStrategy) + .withBalancerStrategy(mockBalancerStrategy) .withBalancerReferenceTimestamp(DateTimes.of("2013-01-01")) .withAvailableSegments(Arrays.asList(segment)).build(), segment @@ -346,7 +358,12 @@ public void testLoadWithNonExistentTier() throws Exception final LoadQueuePeon mockPeon = createEmptyPeon(); mockPeon.loadSegment(EasyMock.anyObject(), EasyMock.anyObject()); EasyMock.expectLastCall().atLeastOnce(); - EasyMock.replay(throttler, mockPeon); + + EasyMock.expect(mockBalancerStrategy.findNewSegmentHomeReplicator(EasyMock.anyObject(), EasyMock.anyObject())) + .andDelegateTo(balancerStrategy) + .times(1); + + EasyMock.replay(throttler, mockPeon, mockBalancerStrategy); LoadRule rule = createLoadRule(ImmutableMap.of( "nonExistentTier", 1, @@ -382,7 +399,7 @@ public void testLoadWithNonExistentTier() throws Exception .withDruidCluster(druidCluster) .withSegmentReplicantLookup(SegmentReplicantLookup.make(new DruidCluster())) .withReplicationManager(throttler) - .withBalancerStrategy(balancerStrategy) + .withBalancerStrategy(mockBalancerStrategy) .withBalancerReferenceTimestamp(DateTimes.of("2013-01-01")) .withAvailableSegments(Arrays.asList(segment)).build(), segment @@ -390,7 +407,7 @@ public void testLoadWithNonExistentTier() throws Exception Assert.assertEquals(1L, stats.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); - EasyMock.verify(throttler, mockPeon); + EasyMock.verify(throttler, mockPeon, mockBalancerStrategy); } @Test @@ -399,7 +416,8 @@ public void testDropWithNonExistentTier() throws Exception final LoadQueuePeon mockPeon = createEmptyPeon(); mockPeon.dropSegment(EasyMock.anyObject(), EasyMock.anyObject()); EasyMock.expectLastCall().atLeastOnce(); - EasyMock.replay(throttler, mockPeon); + + EasyMock.replay(throttler, mockPeon, mockBalancerStrategy); LoadRule rule = createLoadRule(ImmutableMap.of( "nonExistentTier", 1, @@ -452,7 +470,7 @@ public void testDropWithNonExistentTier() throws Exception .withDruidCluster(druidCluster) .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) .withReplicationManager(throttler) - .withBalancerStrategy(balancerStrategy) + .withBalancerStrategy(mockBalancerStrategy) .withBalancerReferenceTimestamp(DateTimes.of("2013-01-01")) .withAvailableSegments(Arrays.asList(segment)).build(), segment @@ -460,13 +478,17 @@ public void testDropWithNonExistentTier() throws Exception Assert.assertEquals(1L, stats.getTieredStat("droppedCount", "hot")); - EasyMock.verify(throttler, mockPeon); + EasyMock.verify(throttler, mockPeon, mockBalancerStrategy); } @Test public void testMaxLoadingQueueSize() throws Exception { - EasyMock.replay(throttler); + EasyMock.expect(mockBalancerStrategy.findNewSegmentHomeReplicator(EasyMock.anyObject(), EasyMock.anyObject())) + .andDelegateTo(balancerStrategy) + .times(2); + + EasyMock.replay(throttler, mockBalancerStrategy); final LoadQueuePeonTester peon = new LoadQueuePeonTester(); @@ -505,7 +527,7 @@ public void testMaxLoadingQueueSize() throws Exception .withDruidCluster(druidCluster) .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) .withReplicationManager(throttler) - .withBalancerStrategy(balancerStrategy) + .withBalancerStrategy(mockBalancerStrategy) .withBalancerReferenceTimestamp(DateTimes.of("2013-01-01")) .withAvailableSegments(Arrays.asList(dataSegment1, dataSegment2, dataSegment3)) .withDynamicConfigs(new CoordinatorDynamicConfig.Builder().withMaxSegmentsInNodeLoadingQueue(2).build()) @@ -519,7 +541,7 @@ public void testMaxLoadingQueueSize() throws Exception Assert.assertEquals(1L, stats2.getTieredStat(LoadRule.ASSIGNED_COUNT, "hot")); Assert.assertFalse(stats3.getTiers(LoadRule.ASSIGNED_COUNT).contains("hot")); - EasyMock.verify(throttler); + EasyMock.verify(throttler, mockBalancerStrategy); } private DataSegment createDataSegment(String dataSource) From 0dde8f564556030a77eaa28989dd0f0916766cf4 Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 21 Sep 2017 08:41:22 -0700 Subject: [PATCH 23/33] Spelling mistake --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 26791d98046a..f58e2c15dfbc 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -54,7 +54,7 @@ public abstract class LoadRule implements Rule private final Object2IntMap targetReplicants = new Object2IntOpenHashMap<>(); private final Object2IntMap currentReplicants = new Object2IntOpenHashMap<>(); - // Cache to hold unsued results from strategy call in assignPrimary + // Cache to hold unused results from strategy call in assignPrimary private final Map strategyCache = new HashMap<>(); @Override From 558da4a30455276e18f255afb58e7a0c7494a221 Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 21 Sep 2017 15:34:26 -0700 Subject: [PATCH 24/33] Cleaning up javadoc. --- .../server/coordinator/rules/LoadRule.java | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index f58e2c15dfbc..ae174203103d 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -153,13 +153,9 @@ private static List getFilteredHolders( return queue.stream().filter(predicate).collect(Collectors.toList()); } - /*** + /** * Iterates through each tier and find the respective segment homes; with the found segment homes, selects the one * with the highest priority to be the holder for the primary replica. - * - * @param params - * @param segment - * @return */ @Nullable private ServerHolder assignPrimary( @@ -197,10 +193,8 @@ private ServerHolder assignPrimary( } else { // cache the result for later use. strategyCache.put(tier, candidate); - if ( - topCandidate == null || - candidate.getServer().getPriority() > topCandidate.getServer().getPriority() - ) { + if (topCandidate == null || + candidate.getServer().getPriority() > topCandidate.getServer().getPriority()) { topCandidate = candidate; } } @@ -215,11 +209,7 @@ private ServerHolder assignPrimary( return topCandidate; } - /*** - * - * @param params - * @param segment - * @param stats + /** * @param tierToSkip if not null, this tier will be skipped from doing assignment, use when primary replica was * assgined. */ From 8784e69bab8c3ca3069d842c4a956717ae322a3c Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 21 Sep 2017 16:02:18 -0700 Subject: [PATCH 25/33] refactor out loading in progress check. --- .../server/coordinator/rules/LoadRule.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index ae174203103d..c18008e671da 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -293,12 +293,8 @@ private void drop( // Make sure we have enough loaded replicants in the correct tiers in the cluster before doing anything // This enforces that loading is completed before we attempt to drop stuffs as a safety measure - for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { - final String tier = entry.getKey(); - // if there are replicants loading in cluster - if (druidCluster.hasTier(tier) && entry.getIntValue() > currentReplicants.getOrDefault(tier, 0)) { - return; - } + if (loadingInProgress(druidCluster)) { + return; } for (final Object2IntMap.Entry entry : currentReplicants.object2IntEntrySet()) { @@ -320,6 +316,22 @@ private void drop( } } + /** + * Returns true if at least one tier in target replica assignment exists in cluster but does not have enough replicas. + */ + private boolean loadingInProgress(final DruidCluster druidCluster) + { + for (final Object2IntMap.Entry entry : targetReplicants.object2IntEntrySet()) { + final String tier = entry.getKey(); + // if there are replicants loading in cluster + if (druidCluster.hasTier(tier) && entry.getIntValue() > currentReplicants.getOrDefault(tier, 0)) { + return true; + } + } + + return false; + } + private static int dropForTier( final int numToDrop, final NavigableSet holdersInTier, From bf3d79ff1515ac389a70ebfff590d2661a1fc2a6 Mon Sep 17 00:00:00 2001 From: Wei Date: Tue, 26 Sep 2017 09:24:02 -0700 Subject: [PATCH 26/33] Removed redundant comment. --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index c18008e671da..4cab55245e39 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -291,8 +291,7 @@ private void drop( { final DruidCluster druidCluster = params.getDruidCluster(); - // Make sure we have enough loaded replicants in the correct tiers in the cluster before doing anything - // This enforces that loading is completed before we attempt to drop stuffs as a safety measure + // This enforces that loading is completed before we attempt to drop stuffs as a safety measure. if (loadingInProgress(druidCluster)) { return; } From 33c651997fdedce6eaed5243bf4e93e55ebefd30 Mon Sep 17 00:00:00 2001 From: Wei Date: Tue, 26 Sep 2017 09:54:39 -0700 Subject: [PATCH 27/33] Removed forbidden API --- .../java/io/druid/server/coordinator/rules/LoadRuleTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index f32cfe634db4..387ee651edc5 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -268,7 +268,7 @@ public void testLoadPriority() throws Exception .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) .withReplicationManager(throttler) .withBalancerStrategy(mockBalancerStrategy) - .withBalancerReferenceTimestamp(new DateTime("2013-01-01")) + .withBalancerReferenceTimestamp(DateTime.parse("2013-01-01")) .withAvailableSegments(Collections.singletonList(segment)).build(), segment ); From d7b7c5524d90f6c8fd798ac486b4adcf0064379b Mon Sep 17 00:00:00 2001 From: Wei Date: Tue, 26 Sep 2017 12:06:12 -0700 Subject: [PATCH 28/33] Correct non-forbidden API. --- .../java/io/druid/server/coordinator/rules/LoadRuleTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index 387ee651edc5..0f846265d1dc 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -268,7 +268,7 @@ public void testLoadPriority() throws Exception .withSegmentReplicantLookup(SegmentReplicantLookup.make(druidCluster)) .withReplicationManager(throttler) .withBalancerStrategy(mockBalancerStrategy) - .withBalancerReferenceTimestamp(DateTime.parse("2013-01-01")) + .withBalancerReferenceTimestamp(DateTimes.of("2013-01-01")) .withAvailableSegments(Collections.singletonList(segment)).build(), segment ); From 6630ae31e3be926bc9b744b90976ae8322926d50 Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 28 Sep 2017 07:49:25 -0700 Subject: [PATCH 29/33] Precision in variable type for NavigableSet. --- .../io/druid/server/coordinator/SegmentReplicantLookup.java | 4 ++-- .../server/coordinator/helper/DruidCoordinatorBalancer.java | 3 ++- .../helper/DruidCoordinatorCleanupOvershadowed.java | 4 ++-- .../coordinator/helper/DruidCoordinatorCleanupUnneeded.java | 4 ++-- .../server/coordinator/helper/DruidCoordinatorLogger.java | 3 +-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java b/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java index c3ced75e93af..f23f1199e86e 100644 --- a/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java +++ b/server/src/main/java/io/druid/server/coordinator/SegmentReplicantLookup.java @@ -26,7 +26,7 @@ import io.druid.timeline.DataSegment; import java.util.Map; -import java.util.NavigableSet; +import java.util.SortedSet; /** * A lookup for the number of replicants of a given segment for a certain tier. @@ -38,7 +38,7 @@ public static SegmentReplicantLookup make(DruidCluster cluster) final Table segmentsInCluster = HashBasedTable.create(); final Table loadingSegments = HashBasedTable.create(); - for (NavigableSet serversByType : cluster.getSortedHistoricalsByTier()) { + for (SortedSet serversByType : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serversByType) { ImmutableDruidServer server = serverHolder.getServer(); diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java index 5f45212a3274..0ec2aeb494b2 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorBalancer.java @@ -38,6 +38,7 @@ import java.util.List; import java.util.Map; import java.util.NavigableSet; +import java.util.SortedSet; import java.util.concurrent.ConcurrentHashMap; /** @@ -87,7 +88,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) private void balanceTier( DruidCoordinatorRuntimeParams params, String tier, - NavigableSet servers, + SortedSet servers, CoordinatorStats stats ) { diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java index 641b024e5093..779c186ea036 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupOvershadowed.java @@ -32,7 +32,7 @@ import io.druid.timeline.VersionedIntervalTimeline; import java.util.Map; -import java.util.NavigableSet; +import java.util.SortedSet; public class DruidCoordinatorCleanupOvershadowed implements DruidCoordinatorHelper { @@ -54,7 +54,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) DruidCluster cluster = params.getDruidCluster(); Map> timelines = Maps.newHashMap(); - for (NavigableSet serverHolders : cluster.getSortedHistoricalsByTier()) { + for (SortedSet serverHolders : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serverHolders) { ImmutableDruidServer server = serverHolder.getServer(); diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java index ea28d43c95c6..5471f3116aa7 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorCleanupUnneeded.java @@ -31,8 +31,8 @@ import io.druid.server.coordinator.ServerHolder; import io.druid.timeline.DataSegment; -import java.util.NavigableSet; import java.util.Set; +import java.util.SortedSet; /** */ @@ -63,7 +63,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) // This is done to prevent a race condition in which the coordinator would drop all segments if it started running // cleanup before it finished polling the metadata storage for available segments for the first time. if (!availableSegments.isEmpty()) { - for (NavigableSet serverHolders : cluster.getSortedHistoricalsByTier()) { + for (SortedSet serverHolders : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serverHolders) { ImmutableDruidServer server = serverHolder.getServer(); diff --git a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java index 42fd902cf04d..c575d826648d 100644 --- a/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java +++ b/server/src/main/java/io/druid/server/coordinator/helper/DruidCoordinatorLogger.java @@ -35,7 +35,6 @@ import it.unimi.dsi.fastutil.objects.Object2LongMap; import java.util.List; -import java.util.NavigableSet; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -171,7 +170,7 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) ); log.info("Load Queues:"); - for (NavigableSet serverHolders : cluster.getSortedHistoricalsByTier()) { + for (Iterable serverHolders : cluster.getSortedHistoricalsByTier()) { for (ServerHolder serverHolder : serverHolders) { ImmutableDruidServer server = serverHolder.getServer(); LoadQueuePeon queuePeon = serverHolder.getPeon(); From 174d73d6e66aa7f283e94bb947c9691edbcbcc9d Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 28 Sep 2017 07:51:18 -0700 Subject: [PATCH 30/33] Obsolete comment. --- .../main/java/io/druid/server/coordinator/rules/LoadRule.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 4cab55245e39..272981a924d3 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -71,7 +71,6 @@ public CoordinatorStats run( final CoordinatorStats stats = new CoordinatorStats(); - // performs if (params.getAvailableSegments().contains(segment)) { assign(params, segment, stats); } From aae2df98172d626cc2edf747c7703603d997bc2a Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 28 Sep 2017 08:28:50 -0700 Subject: [PATCH 31/33] Clarity in method call and moving retrieval of ServerHolder into method call. --- .../server/coordinator/rules/LoadRule.java | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 272981a924d3..4e3dc7190dfa 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -103,19 +103,16 @@ private void assign( return; } + int numAssigned = 1; // 1 replica (i.e., primary replica) already assigned + final String tier = primaryHolderToLoad.getServer().getTier(); // assign replicas for the rest of the tier - final int numAssigned = 1 /* for primary replica */ + assignReplicasForTier( + numAssigned += assignReplicasForTier( tier, targetReplicants.getOrDefault(tier, 0), - // note: adding 1 to currentReplicantsInTier to account for the one assigned as primary replica - currentReplicants.getOrDefault(tier, 0) + 1, + numAssigned, // note that the currentReplicantsInTier is the just-assigned primary replica. params, - getFilteredHolders( - tier, - params.getDruidCluster(), - createLoadQueueSizeLimitingPredicate(params).and(holder -> !holder.equals(primaryHolderToLoad)) - ), + createLoadQueueSizeLimitingPredicate(params).and(holder -> !holder.equals(primaryHolderToLoad)), segment ); stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); @@ -210,7 +207,7 @@ private ServerHolder assignPrimary( /** * @param tierToSkip if not null, this tier will be skipped from doing assignment, use when primary replica was - * assgined. + * assigned. */ private void assignReplicas( final DruidCoordinatorRuntimeParams params, @@ -229,25 +226,34 @@ private void assignReplicas( entry.getIntValue(), currentReplicants.getOrDefault(tier, 0), params, - getFilteredHolders(tier, params.getDruidCluster(), createLoadQueueSizeLimitingPredicate(params)), + createLoadQueueSizeLimitingPredicate(params), segment ); stats.addToTieredStat(ASSIGNED_COUNT, tier, numAssigned); } } + /** + * @param predicate {@link Predicate} used to pre-filter {@link ServerHolder}s retrieved from {@link DruidCluster}. + */ private int assignReplicasForTier( final String tier, final int targetReplicantsInTier, final int currentReplicantsInTier, final DruidCoordinatorRuntimeParams params, - final List holders, + final Predicate predicate, final DataSegment segment ) { final int numToAssign = targetReplicantsInTier - currentReplicantsInTier; - // if nothing to assign or no holders available for assignment - if (numToAssign <= 0 || holders.isEmpty()) { + // if nothing to assign + if (numToAssign <= 0) { + return 0; + } + + final List holders = getFilteredHolders(tier, params.getDruidCluster(), predicate); + // if no holders available for assignment + if (holders.isEmpty()) { return 0; } From cb69caee5c8970f84fd1355eddc1dcd42aa477cf Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 28 Sep 2017 08:32:32 -0700 Subject: [PATCH 32/33] Comment on mutability of CoordinatoorStats. --- .../java/io/druid/server/coordinator/rules/LoadRule.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java index 4e3dc7190dfa..2d6c163ced81 100644 --- a/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/io/druid/server/coordinator/rules/LoadRule.java @@ -86,6 +86,9 @@ public CoordinatorStats run( } } + /** + * @param stats {@link CoordinatorStats} to accumulate assignment statistics. + */ private void assign( final DruidCoordinatorRuntimeParams params, final DataSegment segment, @@ -206,6 +209,7 @@ private ServerHolder assignPrimary( } /** + * @param stats {@link CoordinatorStats} to accumulate assignment statistics. * @param tierToSkip if not null, this tier will be skipped from doing assignment, use when primary replica was * assigned. */ @@ -288,6 +292,9 @@ private int assignReplicasForTier( return numToAssign; } + /** + * @param stats {@link CoordinatorStats} to accumulate assignment statistics. + */ private void drop( final DruidCoordinatorRuntimeParams params, final DataSegment segment, From 428a8205977822206d091311839fad3bdc2ed1fe Mon Sep 17 00:00:00 2001 From: Wei Date: Thu, 28 Sep 2017 10:08:17 -0700 Subject: [PATCH 33/33] Added auxiliary fixture for dropping. --- .../server/coordinator/rules/LoadRuleTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java index 0f846265d1dc..d57808a6d617 100644 --- a/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/io/druid/server/coordinator/rules/LoadRuleTest.java @@ -314,6 +314,15 @@ public void testDrop() throws Exception 0 ); server2.addDataSegment(segment.getIdentifier(), segment); + DruidServer server3 = new DruidServer( + "serverNormNotServing", + "hostNorm", + null, + 10, + ServerType.HISTORICAL, + DruidServer.DEFAULT_TIER, + 0 + ); DruidCluster druidCluster = new DruidCluster( null, ImmutableMap.of( @@ -329,6 +338,10 @@ public void testDrop() throws Exception new ServerHolder( server2.toImmutableDruidServer(), mockPeon + ), + new ServerHolder( + server3.toImmutableDruidServer(), + mockPeon ) ).collect(Collectors.toCollection(() -> new TreeSet<>(Collections.reverseOrder()))) )