egor-ryashin commented on a change in pull request #6349: maintenance mode for Historical URL: https://github.com/apache/incubator-druid/pull/6349#discussion_r220923763
########## File path: server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorBalancerTest.java ########## @@ -195,6 +201,186 @@ public void testMoveToEmptyServerBalancer() Assert.assertEquals(2, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); } + /** + * Server 1 has 2 segments. + * Server 2 (maintenance) has 2 segments. + * Server 3 is empty. + * Maintenance has priority 7. + * Max segments to move is 3. + * 2 (of 2) segments should be moved from Server 2 and 1 (of 2) from Server 1. + */ + @Test + public void testMoveMaintenancePriority() + { + mockDruidServer(druidServer1, "1", "normal", 30L, 100L, ImmutableMap.of(segment1.getIdentifier(), segment1, segment2.getIdentifier(), segment2)); + mockDruidServer(druidServer2, "2", "normal", 30L, 100L, ImmutableMap.of(segment3.getIdentifier(), segment3, segment4.getIdentifier(), segment4)); + mockDruidServer(druidServer3, "3", "normal", 0L, 100L, Collections.emptyMap()); + + EasyMock.replay(druidServer4); + + mockCoordinator(coordinator); + + BalancerStrategy strategy = EasyMock.createMock(BalancerStrategy.class); + EasyMock.expect(strategy.pickSegmentToMove(ImmutableList.of(new ServerHolder(druidServer2, peon2, false)))) + .andReturn(new BalancerSegmentHolder(druidServer2, segment3)) + .andReturn(new BalancerSegmentHolder(druidServer2, segment4)); + EasyMock.expect(strategy.pickSegmentToMove(anyObject())) + .andReturn(new BalancerSegmentHolder(druidServer1, segment1)) + .andReturn(new BalancerSegmentHolder(druidServer1, segment2)); + + EasyMock.expect(strategy.findNewSegmentHomeBalancer(anyObject(), anyObject())).andReturn(new ServerHolder(druidServer3, peon3)).anyTimes(); + replay(strategy); + + DruidCoordinatorRuntimeParams params = defaultRuntimeParamsBuilder( + ImmutableList.of(druidServer1, druidServer2, druidServer3), + ImmutableList.of(peon1, peon2, peon3), + ImmutableList.of(false, true, false) + ) + .withDynamicConfigs( + CoordinatorDynamicConfig.builder() + .withMaxSegmentsToMove( + 3 + ).withMaintenanceModeSegmentsPriority(6).build()// ceil(3 * 0.6) = 2 segments from servers in maintenance + ) + .withBalancerStrategy(strategy) + .build(); + + params = new DruidCoordinatorBalancerTester(coordinator).run(params); + Assert.assertEquals(3L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); + Assert.assertThat(peon3.getSegmentsToLoad(), is(equalTo(ImmutableSet.of(segment1, segment3, segment4)))); + } + + @Test + public void testZeroMaintenancePriority() + { + DruidCoordinatorRuntimeParams params = setupParamsForMaintenancePriority(0); + params = new DruidCoordinatorBalancerTester(coordinator).run(params); + Assert.assertEquals(1L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); + Assert.assertThat(peon3.getSegmentsToLoad(), is(equalTo(ImmutableSet.of(segment1)))); + } + + @Test + public void testMaxMaintenancePriority() + { + DruidCoordinatorRuntimeParams params = setupParamsForMaintenancePriority(10); + params = new DruidCoordinatorBalancerTester(coordinator).run(params); + Assert.assertEquals(1L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); + Assert.assertThat(peon3.getSegmentsToLoad(), is(equalTo(ImmutableSet.of(segment2)))); + } + + /** + * Should balance segments as usual (ignoring priority) with empty maintenanceList. + */ + @Test + public void testMoveMaintenancePriorityWithNoMaintenance() + { + mockDruidServer(druidServer1, "1", "normal", 30L, 100L, ImmutableMap.of(segment1.getIdentifier(), segment1, segment2.getIdentifier(), segment2)); + mockDruidServer(druidServer2, "2", "normal", 0L, 100L, ImmutableMap.of(segment3.getIdentifier(), segment3, segment4.getIdentifier(), segment4)); + mockDruidServer(druidServer3, "3", "normal", 0L, 100L, Collections.emptyMap()); + + EasyMock.replay(druidServer4); + + mockCoordinator(coordinator); + + BalancerStrategy strategy = EasyMock.createMock(BalancerStrategy.class); + EasyMock.expect(strategy.pickSegmentToMove(anyObject())) + .andReturn(new BalancerSegmentHolder(druidServer1, segment1)) + .andReturn(new BalancerSegmentHolder(druidServer1, segment2)) + .andReturn(new BalancerSegmentHolder(druidServer2, segment3)) + .andReturn(new BalancerSegmentHolder(druidServer2, segment4)); + + EasyMock.expect(strategy.findNewSegmentHomeBalancer(anyObject(), anyObject())).andReturn(new ServerHolder(druidServer3, peon3)).anyTimes(); + replay(strategy); + + DruidCoordinatorRuntimeParams params = defaultRuntimeParamsBuilder( + ImmutableList.of(druidServer1, druidServer2, druidServer3), + ImmutableList.of(peon1, peon2, peon3), + ImmutableList.of(false, false, false) + ) + .withDynamicConfigs( + CoordinatorDynamicConfig.builder() Review comment: BTW, as I just noticed such weird placement is silently done by IntelliJ refactoring engine, for example, each time I extract a variable I have to manually edit the code afterwards, kind of inefficient. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org