From fa248faa527dc7078ad6bb0510a1e97701e05e2f Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Mon, 12 Sep 2022 13:05:17 +0530 Subject: [PATCH 01/10] Add coordinator test framework --- .../java/util/metrics/StubServiceEmitter.java | 17 +- .../server/coordinator/DruidCoordinator.java | 10 +- .../coordinator/duty/BalanceSegments.java | 1 + .../server/coordinator/duty/RunRules.java | 1 + .../coordinator/BalanceSegmentsTest.java | 22 +- .../coordinator/CreateDataSegments.java | 148 ++++++ .../server/coordinator/RunRulesTest.java | 26 +- .../simulate/BlockingExecutorService.java | 344 +++++++++++++ .../simulate/CoordinatorSimulation.java | 92 ++++ .../CoordinatorSimulationBaseTest.java | 169 +++++++ .../simulate/CoordinatorSimulationImpl.java | 464 ++++++++++++++++++ .../simulate/SegmentBalancingTest.java | 104 ++++ .../simulate/SegmentLoadingTest.java | 59 +++ .../simulate/TestDruidLeaderSelector.java | 79 +++ .../simulate/TestMetadataRuleManager.java | 111 +++++ .../TestSegmentLoadingHttpClient.java | 168 +++++++ .../simulate/TestSegmentsMetadataManager.java | 193 ++++++++ .../simulate/TestServerInventoryView.java | 210 ++++++++ server/src/test/resources/log4j2.xml | 35 ++ 19 files changed, 2217 insertions(+), 36 deletions(-) create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationImpl.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/TestDruidLeaderSelector.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java create mode 100644 server/src/test/resources/log4j2.xml diff --git a/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java b/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java index 38f715f848aa..9d6032382ad1 100644 --- a/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java +++ b/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java @@ -20,6 +20,7 @@ package org.apache.druid.java.util.metrics; import org.apache.druid.java.util.emitter.core.Event; +import org.apache.druid.java.util.emitter.service.AlertEvent; import org.apache.druid.java.util.emitter.service.ServiceEmitter; import java.util.ArrayList; @@ -27,7 +28,7 @@ public class StubServiceEmitter extends ServiceEmitter { - private List events = new ArrayList<>(); + private final List events = new ArrayList<>(); public StubServiceEmitter(String service, String host) { @@ -37,7 +38,18 @@ public StubServiceEmitter(String service, String host) @Override public void emit(Event event) { - events.add(event); + if (event instanceof AlertEvent) { + final AlertEvent alertEvent = (AlertEvent) event; + System.out.printf( + "[%s] [%s] [%s]: %s%n", + alertEvent.getSeverity(), + alertEvent.getService(), + alertEvent.getFeed(), + alertEvent.getDescription() + ); + } else { + events.add(event); + } } public List getEvents() @@ -53,6 +65,7 @@ public void start() @Override public void flush() { + events.clear(); } @Override diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java index 6b1e29d49167..2244d9ab4d45 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java @@ -483,7 +483,7 @@ public void moveSegment( ); } - final String toLoadQueueSegPath = + final String toLoadQueueSegPath = curator == null ? null : ZKPaths.makePath(zkPaths.getLoadQueuePath(), toServer.getName(), segmentId.toString()); final LoadPeonCallback loadPeonCallback = () -> { @@ -973,6 +973,14 @@ public List getDuties() { return duties; } + + @Override + public String toString() + { + return "DutiesRunnable{" + + "dutiesRunnableAlias='" + dutiesRunnableAlias + '\'' + + '}'; + } } /** diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java index d2a1c4c8daaf..198a7cf5e8f6 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java @@ -92,6 +92,7 @@ private void balanceTier( ) { + log.info("Balancing segments in tier [%s]", tier); if (params.getUsedSegments().size() == 0) { log.info("Metadata segments are not available. Cannot balance."); // suppress emit zero stats diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java index cf285891f615..f6a96f220b88 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java @@ -71,6 +71,7 @@ public RunRules(ReplicationThrottler replicatorThrottler, DruidCoordinator coord @Override public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) { + final long start = System.currentTimeMillis(); replicatorThrottler.updateParams( coordinator.getDynamicConfigs().getReplicationThrottleLimit(), coordinator.getDynamicConfigs().getReplicantLifetime(), diff --git a/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsTest.java index d4a89abb3d23..a7c594fe094c 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsTest.java @@ -30,7 +30,6 @@ import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.partition.NoneShardSpec; import org.easymock.EasyMock; -import org.hamcrest.Matchers; import org.joda.time.DateTime; import org.joda.time.Interval; import org.junit.After; @@ -277,9 +276,9 @@ public void testMoveDecommissioningMaxPercentOfMaxSegmentsToMove() params = new BalanceSegmentsTester(coordinator).run(params); EasyMock.verify(strategy); Assert.assertEquals(3L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); - Assert.assertThat( - peon3.getSegmentsToLoad(), - Matchers.is(Matchers.equalTo(ImmutableSet.of(segment1, segment3, segment4))) + Assert.assertEquals( + ImmutableSet.of(segment1, segment3, segment4), + peon3.getSegmentsToLoad() ); } @@ -289,7 +288,7 @@ public void testZeroDecommissioningMaxPercentOfMaxSegmentsToMove() DruidCoordinatorRuntimeParams params = setupParamsForDecommissioningMaxPercentOfMaxSegmentsToMove(0); params = new BalanceSegmentsTester(coordinator).run(params); Assert.assertEquals(1L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); - Assert.assertThat(peon3.getSegmentsToLoad(), Matchers.is(Matchers.equalTo(ImmutableSet.of(segment1)))); + Assert.assertEquals(ImmutableSet.of(segment1), peon3.getSegmentsToLoad()); } @Test @@ -298,7 +297,7 @@ public void testMaxDecommissioningMaxPercentOfMaxSegmentsToMove() DruidCoordinatorRuntimeParams params = setupParamsForDecommissioningMaxPercentOfMaxSegmentsToMove(10); params = new BalanceSegmentsTester(coordinator).run(params); Assert.assertEquals(1L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); - Assert.assertThat(peon3.getSegmentsToLoad(), Matchers.is(Matchers.equalTo(ImmutableSet.of(segment2)))); + Assert.assertEquals(ImmutableSet.of(segment2), peon3.getSegmentsToLoad()); } /** @@ -347,9 +346,9 @@ public void testMoveDecommissioningMaxPercentOfMaxSegmentsToMoveWithNoDecommissi params = new BalanceSegmentsTester(coordinator).run(params); EasyMock.verify(strategy); Assert.assertEquals(3L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); - Assert.assertThat( - peon3.getSegmentsToLoad(), - Matchers.is(Matchers.equalTo(ImmutableSet.of(segment2, segment3, segment4))) + Assert.assertEquals( + ImmutableSet.of(segment2, segment3, segment4), + peon3.getSegmentsToLoad() ); } @@ -603,10 +602,7 @@ public void testThatDynamicConfigIsHonoredWhenPickingSegmentToMove() params = new BalanceSegmentsTester(coordinator).run(params); EasyMock.verify(strategy); Assert.assertEquals(1L, params.getCoordinatorStats().getTieredStat("movedCount", "normal")); - Assert.assertThat( - peon3.getSegmentsToLoad(), - Matchers.is(Matchers.equalTo(ImmutableSet.of(segment3))) - ); + Assert.assertEquals(ImmutableSet.of(segment3), peon3.getSegmentsToLoad()); } @Test diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java new file mode 100644 index 000000000000..968da1dd357f --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator; + +import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.segment.IndexIO; +import org.apache.druid.timeline.DataSegment; +import org.apache.druid.timeline.partition.NumberedShardSpec; +import org.joda.time.DateTime; +import org.joda.time.Interval; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * Creates {@link DataSegment}s for a given datasource. + */ +public class CreateDataSegments +{ + private final String datasource; + + private DateTime startTime; + private Granularity granularity; + private int numPartitionsPerInterval; + private int numIntervals; + + public static CreateDataSegments ofDatasource(String datasource) + { + return new CreateDataSegments(datasource); + } + + private CreateDataSegments(String datasource) + { + this.datasource = datasource; + } + + public CreateDataSegments forIntervals(int numIntervals, Granularity intervalSize) + { + this.numIntervals = numIntervals; + this.granularity = intervalSize; + return this; + } + + public CreateDataSegments andPartitionsPerInterval(int numPartitionsPerInterval) + { + this.numPartitionsPerInterval = numPartitionsPerInterval; + return this; + } + + public CreateDataSegments startingAt(String startOfFirstInterval) + { + this.startTime = DateTimes.of(startOfFirstInterval); + return this; + } + + public List eachOfSizeMb(long sizeMb) + { + final List segments = new ArrayList<>(); + + int uniqueIdInInterval = 0; + DateTime nextStart = startTime; + for (int numInterval = 0; numInterval < numIntervals; ++numInterval) { + Interval nextInterval = new Interval(nextStart, granularity.increment(nextStart)); + for (int numPartition = 0; numPartition < numPartitionsPerInterval; ++numPartition) { + segments.add( + new NumberedDataSegment( + datasource, + nextInterval, + new NumberedShardSpec(numPartition, numPartitionsPerInterval), + ++uniqueIdInInterval, + sizeMb + ) + ); + } + nextStart = granularity.increment(nextStart); + } + + /*intervalsByGranularity.granularityIntervalsIterator().forEachRemaining( + subInterval -> { + for (int i = 0; i < numPartitionsPerInterval; ++i) { + segments.add( + new NumberedDataSegment( + datasource, + subInterval, + new NumberedShardSpec(i, numPartitionsPerInterval), + uniqueIdInInterval.incrementAndGet(), + sizeMb + ) + ); + } + } + );*/ + + return segments; + } + + private static class NumberedDataSegment extends DataSegment + { + private final int uniqueId; + + private NumberedDataSegment( + String datasource, + Interval interval, + NumberedShardSpec shardSpec, + int uinqueId, + long size + ) + { + super( + datasource, + interval, + "1", + Collections.emptyMap(), + Collections.emptyList(), + Collections.emptyList(), + shardSpec, + IndexIO.CURRENT_VERSION_ID, + size + ); + this.uniqueId = uinqueId; + } + + @Override + public String toString() + { + return "{" + getDataSource() + "::" + uniqueId + "}"; + } + } +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java index dc4a6f0abe43..1184cb0660e1 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java @@ -26,6 +26,7 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.emitter.EmittingLogger; import org.apache.druid.java.util.emitter.service.ServiceEmitter; import org.apache.druid.java.util.emitter.service.ServiceEventBuilder; @@ -41,8 +42,6 @@ import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.partition.NoneShardSpec; import org.easymock.EasyMock; -import org.joda.time.DateTime; -import org.joda.time.Interval; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -81,24 +80,11 @@ public void setUp() databaseRuleManager = EasyMock.createMock(MetadataRuleManager.class); segmentsMetadataManager = EasyMock.createNiceMock(SegmentsMetadataManager.class); - DateTime start = DateTimes.of("2012-01-01"); - usedSegments = new ArrayList<>(); - for (int i = 0; i < 24; i++) { - usedSegments.add( - new DataSegment( - "test", - new Interval(start, start.plusHours(1)), - DateTimes.nowUtc().toString(), - new HashMap<>(), - new ArrayList<>(), - new ArrayList<>(), - NoneShardSpec.instance(), - IndexIO.CURRENT_VERSION_ID, - 1 - ) - ); - start = start.plusHours(1); - } + usedSegments = CreateDataSegments.ofDatasource("test") + .forIntervals(24, Granularities.HOUR) + .startingAt("2012-01-01") + .andPartitionsPerInterval(1) + .eachOfSizeMb(1); ruleRunner = new RunRules(new ReplicationThrottler(24, 1, false), coordinator); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java new file mode 100644 index 000000000000..3c1eb13daea2 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java @@ -0,0 +1,344 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.logger.Logger; + +import java.util.Collection; +import java.util.List; +import java.util.Queue; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.Delayed; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +/** + * An executor that keeps submitted tasks in a queue until they are explicitly + * invoked by calling one of these methods: + *
    + *
  • {@link #finishNextPendingTask()}
    • + *
  • {@link #finishNextPendingTasks(int)}
    • + *
  • {@link #finishAllPendingTasks()}
    • + *
+ */ +public class BlockingExecutorService implements ScheduledExecutorService +{ + private static final Logger log = new Logger(BlockingExecutorService.class); + + private final String nameFormat; + private final boolean ignoreScheduledTasks; + private final Queue> taskQueue = new ConcurrentLinkedQueue<>(); + + public BlockingExecutorService(String nameFormat, boolean ignoreScheduledTasks) + { + this.nameFormat = nameFormat; + this.ignoreScheduledTasks = ignoreScheduledTasks; + } + + public boolean hasPendingTasks() + { + return !taskQueue.isEmpty(); + } + + /** + * Executes the next pending task on the calling thread itself. + */ + public int finishNextPendingTask() + { + log.debug("[%s] Executing next pending task", nameFormat); + Task task = taskQueue.poll(); + if (task != null) { + task.executeNow(); + return 1; + } else { + return 0; + } + } + + /** + * Executes the next {@code numTasksToExecute} pending tasks on the calling + * thread itself. + */ + public int finishNextPendingTasks(int numTasksToExecute) + { + log.debug("[%s] Executing %d pending tasks", nameFormat, numTasksToExecute); + int executedTaskCount = 0; + for (; executedTaskCount < numTasksToExecute; ++executedTaskCount) { + Task task = taskQueue.poll(); + if (task == null) { + break; + } else { + task.executeNow(); + } + } + return executedTaskCount; + } + + /** + * Executes all the remaining pending tasks on the calling thread itself. + *

+ * Note: This method can keep running forever if another thread keeps submitting + * new tasks to the executor. + */ + public int finishAllPendingTasks() + { + log.debug("[%s] Executing all pending tasks", nameFormat); + Task task; + int executedTaskCount = 0; + while ((task = taskQueue.poll()) != null) { + task.executeNow(); + ++executedTaskCount; + } + + return executedTaskCount; + } + + // Task submission operations + @Override + public Future submit(Callable task) + { + return addTaskToQueue(task); + } + + @Override + public Future submit(Runnable task, T result) + { + return addTaskToQueue(() -> { + task.run(); + return result; + }); + } + + @Override + public Future submit(Runnable task) + { + return addTaskToQueue(() -> { + task.run(); + return null; + }); + } + + @Override + public void execute(Runnable command) + { + submit(command); + } + + @Override + public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) + { + if (ignoreScheduledTasks) { + log.debug("[%s] Ignoring scheduled task", nameFormat); + return new WrappingScheduledFuture<>(CompletableFuture.completedFuture(null)); + } + + // Ignore the delay and just queue the task + return new WrappingScheduledFuture<>(submit(command)); + } + + @Override + public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) + { + if (ignoreScheduledTasks) { + log.debug("[%s] Ignoring scheduled task", nameFormat); + return new WrappingScheduledFuture<>(CompletableFuture.completedFuture(null)); + } + + // Ignore the delay and just queue the task + return new WrappingScheduledFuture<>(submit(callable)); + } + + private Future addTaskToQueue(Callable callable) + { + Task task = new Task<>(callable); + taskQueue.add(task); + return task.future; + } + + // Termination operations + @Override + public void shutdown() + { + + } + + @Override + public List shutdownNow() + { + return null; + } + + @Override + public boolean isShutdown() + { + return false; + } + + @Override + public boolean isTerminated() + { + return false; + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException + { + return false; + } + + // Unsupported operations + @Override + public List> invokeAll(Collection> tasks) + { + throw new UnsupportedOperationException(); + } + + @Override + public List> invokeAll( + Collection> tasks, + long timeout, + TimeUnit unit + ) + { + throw new UnsupportedOperationException(); + } + + @Override + public T invokeAny(Collection> tasks) + { + throw new UnsupportedOperationException(); + } + + @Override + public T invokeAny(Collection> tasks, long timeout, TimeUnit unit) + { + throw new UnsupportedOperationException(); + } + + @Override + public ScheduledFuture scheduleAtFixedRate( + Runnable command, + long initialDelay, + long period, + TimeUnit unit + ) + { + throw new UnsupportedOperationException(); + } + + @Override + public ScheduledFuture scheduleWithFixedDelay( + Runnable command, + long initialDelay, + long delay, + TimeUnit unit + ) + { + throw new UnsupportedOperationException(); + } + + /** + * Task that can be invoked to complete the corresponding future. + */ + private static class Task + { + private final Callable callable; + private final CompletableFuture future = new CompletableFuture<>(); + + private Task(Callable callable) + { + this.callable = callable; + } + + private void executeNow() + { + try { + T result = callable.call(); + future.complete(result); + } + catch (Exception e) { + throw new ISE("Error while executing task", e); + } + } + } + + /** + * Wraps a Future into a ScheduledFuture. + */ + private static class WrappingScheduledFuture implements ScheduledFuture + { + private final Future future; + + private WrappingScheduledFuture(Future future) + { + this.future = future; + } + + @Override + public long getDelay(TimeUnit unit) + { + return 0; + } + + @Override + public int compareTo(Delayed o) + { + return 0; + } + + @Override + public boolean cancel(boolean mayInterruptIfRunning) + { + return future.cancel(mayInterruptIfRunning); + } + + @Override + public boolean isCancelled() + { + return future.isCancelled(); + } + + @Override + public boolean isDone() + { + return future.isDone(); + } + + @Override + public V get() throws InterruptedException, ExecutionException + { + return future.get(); + } + + @Override + public V get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException + { + return future.get(timeout, unit); + } + } + +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java new file mode 100644 index 000000000000..2ba5fce67836 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.client.DruidServer; +import org.apache.druid.java.util.emitter.core.Event; + +import java.util.List; + +/** + * Runner for a coordinator simulation. + */ +public interface CoordinatorSimulation +{ + /** + * Starts the simulation if not already started. + */ + void start(); + + /** + * Stops the simulation. + */ + void stop(); + + CoordinatorState coordinator(); + + ClusterState cluster(); + + /** + * Represents the state of the coordinator during a simulation. + */ + interface CoordinatorState + { + /** + * Runs a single coordinator cycle. + */ + void runCycle(); + + /** + * Synchronizes the inventory view maintained by the coordinator with the + * actual state of the cluster. + */ + void syncInventoryView(); + + /** + * Gets the inventory view of the specified server as maintained by the + * coordinator. + */ + DruidServer getInventoryView(String serverName); + + /** + * Returns the metric events emitted in the previous coordinator run. + */ + List getMetricEvents(); + + /** + * Gets the load percentage of the specified datasource as seen by the coordinator. + */ + double getLoadPercentage(String datasource); + } + + /** + * Represents the state of the cluster during a simulation. + */ + interface ClusterState + { + /** + * Finishes load of all the segments that were queued in the previous + * coordinator run. Also handles the responses and executes the respective + * callbacks on the coordinator. + */ + void loadQueuedSegments(); + + } +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java new file mode 100644 index 000000000000..17ac6d4ee23f --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -0,0 +1,169 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.client.DruidServer; +import org.apache.druid.java.util.emitter.core.Event; +import org.apache.druid.server.coordination.ServerType; +import org.apache.druid.server.coordinator.rules.ForeverLoadRule; +import org.apache.druid.server.coordinator.rules.Rule; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +/** + * Base test for coordinator simulations. + *

+ * Each test must call {@link #startSimulation(CoordinatorSimulation)} to start + * the simulation. {@link CoordinatorSimulation#stop()} should not be called as + * the simulation is stopped when cleaning up after the test in {@link #tearDown()}. + */ +public class CoordinatorSimulationBaseTest + implements CoordinatorSimulation.CoordinatorState, CoordinatorSimulation.ClusterState +{ + static final double DOUBLE_DELTA = 10e-9; + + static class Rules + { + static final List T1_X1 = Collections.singletonList( + new ForeverLoadRule(Collections.singletonMap(Tier.T1, 1)) + ); + } + + static class Tier + { + static final String T1 = "_default_tier"; + } + + private CoordinatorSimulation sim; + + @Before + public void setUp() + { + sim = null; + } + + @After + public void tearDown() + { + if (sim != null) { + sim.stop(); + } + } + + void startSimulation(CoordinatorSimulation simulation) + { + this.sim = simulation; + simulation.start(); + } + + @Override + public void runCycle() + { + sim.coordinator().runCycle(); + } + + @Override + public List getMetricEvents() + { + return sim.coordinator().getMetricEvents(); + } + + @Override + public DruidServer getInventoryView(String serverName) + { + return sim.coordinator().getInventoryView(serverName); + } + + @Override + public void syncInventoryView() + { + sim.coordinator().syncInventoryView(); + } + + @Override + public void loadQueuedSegments() + { + sim.cluster().loadQueuedSegments(); + } + + @Override + public double getLoadPercentage(String datasource) + { + return sim.coordinator().getLoadPercentage(datasource); + } + + // Verification methods + void verifyDatasourceIsFullyLoaded(String datasource) + { + Assert.assertEquals(100.0, getLoadPercentage(datasource), DOUBLE_DELTA); + } + + void verifyLatestMetricValue(String metricName, Number expectedValue) + { + final List observedValues = getMetricValues(metricName); + Number latestValue = observedValues.get(observedValues.size() - 1); + Assert.assertEquals(expectedValue, latestValue); + } + + private List getMetricValues(String metricName) + { + final List metricValues = new ArrayList<>(); + + for (Event event : sim.coordinator().getMetricEvents()) { + final Map map = event.toMap(); + final String eventMetricName = (String) map.get("metric"); + if (eventMetricName != null && eventMetricName.equals(metricName)) { + metricValues.add((Number) map.get("value")); + } + } + + return metricValues; + } + + // Utility methods + + /** + * Creates a tier of historicals. + */ + static List createHistoricalTier(String tier, int numServers, long serverSizeMb) + { + List servers = new ArrayList<>(); + for (int i = 0; i < numServers; ++i) { + servers.add(createHistorical(i, tier, serverSizeMb)); + } + return servers; + } + + /** + * Creates a historical. + */ + static DruidServer createHistorical(int uniqueIdInTier, String tier, long serverSizeMb) + { + final String name = tier + "__" + "hist__" + uniqueIdInTier; + return new DruidServer(name, name, name, serverSizeMb, ServerType.HISTORICAL, tier, 1); + } + +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationImpl.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationImpl.java new file mode 100644 index 000000000000..e42be2b03925 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationImpl.java @@ -0,0 +1,464 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import com.fasterxml.jackson.databind.InjectableValues; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Preconditions; +import org.apache.druid.client.DruidServer; +import org.apache.druid.common.config.JacksonConfigManager; +import org.apache.druid.curator.ZkEnablementConfig; +import org.apache.druid.curator.discovery.ServiceAnnouncer; +import org.apache.druid.jackson.DefaultObjectMapper; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.concurrent.ScheduledExecutorFactory; +import org.apache.druid.java.util.common.lifecycle.Lifecycle; +import org.apache.druid.java.util.emitter.EmittingLogger; +import org.apache.druid.java.util.emitter.core.Event; +import org.apache.druid.java.util.http.client.HttpClient; +import org.apache.druid.java.util.metrics.StubServiceEmitter; +import org.apache.druid.server.coordinator.BalancerStrategyFactory; +import org.apache.druid.server.coordinator.CachingCostBalancerStrategyConfig; +import org.apache.druid.server.coordinator.CachingCostBalancerStrategyFactory; +import org.apache.druid.server.coordinator.CoordinatorCompactionConfig; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; +import org.apache.druid.server.coordinator.DruidCoordinator; +import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.LoadQueueTaskMaster; +import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; +import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroups; +import org.apache.druid.server.coordinator.rules.Rule; +import org.apache.druid.server.lookup.cache.LookupCoordinatorManager; +import org.apache.druid.timeline.DataSegment; +import org.easymock.EasyMock; +import org.joda.time.Duration; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Implementation of {@link CoordinatorSimulation}. + * + * @see #builder() + */ +public class CoordinatorSimulationImpl implements CoordinatorSimulation, + CoordinatorSimulation.CoordinatorState, CoordinatorSimulation.ClusterState +{ + private static final ObjectMapper OBJECT_MAPPER = new DefaultObjectMapper() + .setInjectableValues( + new InjectableValues.Std().addValue( + DataSegment.PruneSpecsHolder.class, + DataSegment.PruneSpecsHolder.DEFAULT + ) + ); + private static final long DEFAULT_COORDINATOR_PERIOD = 100L; + + private final AtomicBoolean running = new AtomicBoolean(false); + + private final DruidCoordinator coordinator; + private final Environment env; + + private CoordinatorSimulationImpl(DruidCoordinator coordinator, Environment env) + { + this.coordinator = coordinator; + this.env = env; + } + + /** + * Creates a new simulation builder. + */ + public static Builder builder() + { + return new Builder(); + } + + @Override + public void start() + { + if (!running.compareAndSet(false, true)) { + throw new ISE("Simulation is already running"); + } + + try { + env.setUp(); + coordinator.start(); + } + catch (Exception e) { + throw new ISE(e, "Exception while running simulation"); + } + } + + @Override + public void stop() + { + coordinator.stop(); + env.leaderSelector.stopBeingLeader(); + env.tearDown(); + } + + @Override + public CoordinatorState coordinator() + { + return this; + } + + @Override + public ClusterState cluster() + { + return this; + } + + // Blocked mode processing invocations + + @Override + public void runCycle() + { + verifySimulationRunning(); + env.serviceEmitter.flush(); + env.executorFactory.coordinatorRunner.finishNextPendingTasks(1); + } + + @Override + public void syncInventoryView() + { + verifySimulationRunning(); + env.coordinatorInventoryView.sync(env.historicalInventoryView); + } + + @Override + public DruidServer getInventoryView(String serverName) + { + return env.coordinatorInventoryView.getInventoryValue(serverName); + } + + @Override + public void loadQueuedSegments() + { + verifySimulationRunning(); + + final BlockingExecutorService loadQueueExecutor = env.executorFactory.loadQueueExecutor; + while (loadQueueExecutor.hasPendingTasks()) { + // Drain all the items from the load queue executor + // This sends at most 1 load/drop request to each server + loadQueueExecutor.finishAllPendingTasks(); + + // Load all the queued segments, handle their responses and execute callbacks + int loadedSegments = env.executorFactory.historicalLoader.finishAllPendingTasks(); + loadQueueExecutor.finishNextPendingTasks(loadedSegments); + env.executorFactory.loadCallbackExecutor.finishAllPendingTasks(); + } + } + + private void verifySimulationRunning() + { + if (!running.get()) { + throw new ISE("Simulation hasn't been started yet."); + } + } + + @Override + public double getLoadPercentage(String datasource) + { + return coordinator.getLoadStatus().get(datasource); + } + + @Override + public List getMetricEvents() + { + return new ArrayList<>(env.serviceEmitter.getEvents()); + } + + /** + * Builder for a coordinator simulation. + */ + public static class Builder + { + private BalancerStrategyFactory balancerStrategyFactory; + private List servers; + private List segments; + private final Map> datasourceRules = new HashMap<>(); + + public Builder balancer(BalancerStrategyFactory strategyFactory) + { + this.balancerStrategyFactory = strategyFactory; + return this; + } + + public Builder servers(List servers) + { + this.servers = servers; + return this; + } + + public Builder segments(List segments) + { + this.segments = segments; + return this; + } + + public Builder rulesForDatasource(String datasource, List rules) + { + this.datasourceRules.put(datasource, rules); + return this; + } + + public CoordinatorSimulationImpl build() + { + Preconditions.checkArgument( + servers != null && !servers.isEmpty(), + "Cannot run simulation for an empty cluster" + ); + + // Prepare the config + final DruidCoordinatorConfig coordinatorConfig = new TestDruidCoordinatorConfig.Builder() + .withCoordinatorStartDelay(new Duration(1L)) + .withCoordinatorPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) + .withCoordinatorKillPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) + .withLoadQueuePeonRepeatDelay(new Duration("PT0S")) + .withLoadQueuePeonType("http") + .withCoordinatorKillIgnoreDurationToRetain(false) + .build(); + + // Prepare the environment + final TestServerInventoryView serverInventoryView = new TestServerInventoryView(); + servers.forEach(serverInventoryView::addServer); + + final TestSegmentsMetadataManager segmentManager = new TestSegmentsMetadataManager(); + if (segments != null) { + segments.forEach(segmentManager::addSegment); + } + + final TestMetadataRuleManager ruleManager = new TestMetadataRuleManager(); + datasourceRules.forEach( + (datasource, rules) -> + ruleManager.overrideRule(datasource, rules, null) + ); + + final Environment env = new Environment( + new TestDruidLeaderSelector(), + serverInventoryView, + segmentManager, + ruleManager, + coordinatorConfig + ); + + // Build the coordinator + final DruidCoordinator coordinator = new DruidCoordinator( + env.coordinatorConfig, + null, + env.jacksonConfigManager, + env.segmentManager, + env.historicalInventoryView, + env.ruleManager, + () -> null, + env.serviceEmitter, + env.executorFactory, + null, + env.loadQueueTaskMaster, + new ServiceAnnouncer.Noop(), + null, + Collections.emptySet(), + null, + new CoordinatorCustomDutyGroups(Collections.emptySet()), + balancerStrategyFactory != null ? balancerStrategyFactory + : buildCachingCostBalancerStrategy(env), + env.lookupCoordinatorManager, + env.leaderSelector, + OBJECT_MAPPER, + ZkEnablementConfig.ENABLED + ); + + return new CoordinatorSimulationImpl(coordinator, env); + } + + private BalancerStrategyFactory buildCachingCostBalancerStrategy(Environment env) + { + try { + return new CachingCostBalancerStrategyFactory( + env.coordinatorInventoryView, + env.lifecycle, + new CachingCostBalancerStrategyConfig() + ); + } + catch (Exception e) { + throw new ISE(e, "Error building balancer strategy"); + } + } + } + + /** + * Environment for a coordinator simulation. + */ + private static class Environment + { + private final Lifecycle lifecycle = new Lifecycle("coord-sim"); + + // Executors + private final ExecutorFactory executorFactory; + + private final TestDruidLeaderSelector leaderSelector; + private final TestSegmentsMetadataManager segmentManager; + private final TestMetadataRuleManager ruleManager; + private final TestServerInventoryView historicalInventoryView; + + private final LoadQueueTaskMaster loadQueueTaskMaster; + private final StubServiceEmitter serviceEmitter + = new StubServiceEmitter("coordinator", "coordinator"); + private final TestServerInventoryView coordinatorInventoryView; + + private final JacksonConfigManager jacksonConfigManager; + private final LookupCoordinatorManager lookupCoordinatorManager; + private final DruidCoordinatorConfig coordinatorConfig; + + private final List mocks = new ArrayList<>(); + + private Environment( + TestDruidLeaderSelector leaderSelector, + TestServerInventoryView historicalInventoryView, + TestSegmentsMetadataManager segmentManager, + TestMetadataRuleManager ruleManager, + DruidCoordinatorConfig coordinatorConfig + ) + { + this.leaderSelector = leaderSelector; + this.historicalInventoryView = historicalInventoryView; + this.segmentManager = segmentManager; + this.ruleManager = ruleManager; + this.coordinatorConfig = coordinatorConfig; + + this.executorFactory = new ExecutorFactory(); + this.coordinatorInventoryView = new TestServerInventoryView(); + HttpClient httpClient = new TestSegmentLoadingHttpClient( + OBJECT_MAPPER, + historicalInventoryView::getChangeHandlerForHost, + executorFactory.create(1, ExecutorFactory.HISTORICAL_LOADER) + ); + + this.loadQueueTaskMaster = new LoadQueueTaskMaster( + null, + OBJECT_MAPPER, + executorFactory.create(1, ExecutorFactory.LOAD_QUEUE_EXECUTOR), + executorFactory.create(1, ExecutorFactory.LOAD_CALLBACK_EXECUTOR), + coordinatorConfig, + httpClient, + null + ); + + this.jacksonConfigManager = mockConfigManager(); + this.lookupCoordinatorManager = EasyMock.createNiceMock(LookupCoordinatorManager.class); + mocks.add(jacksonConfigManager); + mocks.add(lookupCoordinatorManager); + } + + private void setUp() throws Exception + { + EmittingLogger.registerEmitter(serviceEmitter); + historicalInventoryView.setUp(); + coordinatorInventoryView.sync(historicalInventoryView); + coordinatorInventoryView.setUp(); + lifecycle.start(); + executorFactory.setUp(); + leaderSelector.becomeLeader(); + EasyMock.replay(mocks.toArray()); + } + + private void tearDown() + { + EasyMock.verify(mocks.toArray()); + lifecycle.stop(); + } + + private JacksonConfigManager mockConfigManager() + { + final CoordinatorDynamicConfig dynamicConfig = + CoordinatorDynamicConfig + .builder() + .withMaxSegmentsToMove(100) + .withMaxSegmentsInNodeLoadingQueue(0) + .withReplicationThrottleLimit(100000) + .build(); + + final JacksonConfigManager jacksonConfigManager + = EasyMock.createMock(JacksonConfigManager.class); + EasyMock.expect( + jacksonConfigManager.watch( + EasyMock.eq(CoordinatorDynamicConfig.CONFIG_KEY), + EasyMock.eq(CoordinatorDynamicConfig.class), + EasyMock.anyObject() + ) + ).andReturn(new AtomicReference<>(dynamicConfig)).anyTimes(); + + EasyMock.expect( + jacksonConfigManager.watch( + EasyMock.eq(CoordinatorCompactionConfig.CONFIG_KEY), + EasyMock.eq(CoordinatorCompactionConfig.class), + EasyMock.anyObject() + ) + ).andReturn(new AtomicReference<>(CoordinatorCompactionConfig.empty())).anyTimes(); + + return jacksonConfigManager; + } + } + + private static class ExecutorFactory implements ScheduledExecutorFactory + { + static final String HISTORICAL_LOADER = "historical-loader-%d"; + static final String LOAD_QUEUE_EXECUTOR = "load-queue-%d"; + static final String LOAD_CALLBACK_EXECUTOR = "load-callback-%d"; + static final String COORDINATOR_RUNNER = "Coordinator-Exec--%d"; + + private final Map blockingExecutors = new HashMap<>(); + + private BlockingExecutorService historicalLoader; + private BlockingExecutorService loadQueueExecutor; + private BlockingExecutorService loadCallbackExecutor; + private BlockingExecutorService coordinatorRunner; + + @Override + public ScheduledExecutorService create(int corePoolSize, String nameFormat) + { + boolean isCoordinatorRunner = COORDINATOR_RUNNER.equals(nameFormat); + return blockingExecutors.computeIfAbsent( + nameFormat, + k -> new BlockingExecutorService(k, !isCoordinatorRunner) + ); + } + + private BlockingExecutorService findExecutor(String nameFormat) + { + return blockingExecutors.get(nameFormat); + } + + private void setUp() + { + coordinatorRunner = findExecutor(COORDINATOR_RUNNER); + historicalLoader = findExecutor(HISTORICAL_LOADER); + loadQueueExecutor = findExecutor(LOAD_QUEUE_EXECUTOR); + loadCallbackExecutor = findExecutor(LOAD_CALLBACK_EXECUTOR); + } + } + +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java new file mode 100644 index 000000000000..661e0a864a68 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.client.DruidServer; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.server.coordinator.CostBalancerStrategyFactory; +import org.apache.druid.server.coordinator.CreateDataSegments; +import org.apache.druid.timeline.DataSegment; +import org.junit.Assert; +import org.junit.Test; + +import java.util.List; + +/** + * Coordinator simulation test to verify behaviour of segment balancing. + */ +public class SegmentBalancingTest extends CoordinatorSimulationBaseTest +{ + @Test + public void testBalancingWithInventoryViewUpdates() + { + testBalancingWhenInventoryIsSynced(true); + + // Fix https://github.com/apache/druid/issues/12881 to enable this test case + // testBalancingWhenInventoryIsSynced(false); + } + + private void testBalancingWhenInventoryIsSynced(boolean syncInventory) + { + final String datasource = "wikitest"; + + // Setup servers and segments + final List historicals = createHistoricalTier(Tier.T1, 2, 10_000); + final List segments = + CreateDataSegments.ofDatasource(datasource) + .forIntervals(1, Granularities.DAY) + .startingAt("2022-01-01") + .andPartitionsPerInterval(10) + .eachOfSizeMb(500); + + // Put all the segments on historicalA + final DruidServer historicalA = historicals.get(0); + final DruidServer historicalB = historicals.get(1); + segments.forEach(historicalA::addDataSegment); + + // Build and run the simulation + final CoordinatorSimulation sim = + CoordinatorSimulationImpl.builder() + .balancer(new CostBalancerStrategyFactory()) + .segments(segments) + .servers(historicals) + .rulesForDatasource(datasource, Rules.T1_X1) + .build(); + + startSimulation(sim); + runCycle(); + + // Verify that segments have been chosen for balancing + verifyLatestMetricValue("segment/moved/count", 5L); + + loadQueuedSegments(); + if (syncInventory) { + syncInventoryView(); + } + + // Verify that segments have now been balanced out + final DruidServer historicalViewA = getInventoryView(historicalA.getName()); + final DruidServer historicalViewB = getInventoryView(historicalB.getName()); + Assert.assertEquals(5, historicalViewA.getTotalSegments()); + Assert.assertEquals(5, historicalViewB.getTotalSegments()); + verifyDatasourceIsFullyLoaded(datasource); + } + + @Test + public void testBalancingSkipsOvershadowedSegments() + { + + } + + @Test + public void testBalancingOfFullyReplicatedSegment() + { + + } + +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java new file mode 100644 index 000000000000..fef7e0a00379 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.junit.Test; + +/** + * Coordinator simulation test to verify behaviour of segment loading. + */ +public class SegmentLoadingTest extends CoordinatorSimulationBaseTest +{ + @Test + public void testPrimaryReplicaOnTierIsNotThrottled() + { + + } + + @Test + public void testReplicationThrottleLimit() + { + + } + + @Test + public void testHistoricalsAreNotOverAssigned() + { + + } + + @Test + public void testDropHappensAfterAllPrimaryLoads() + { + + } + + @Test + public void testLoadingOfFullyReplicatedSegment() + { + + } + +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestDruidLeaderSelector.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestDruidLeaderSelector.java new file mode 100644 index 000000000000..d84cbcff6efe --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestDruidLeaderSelector.java @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.discovery.DruidLeaderSelector; + +import javax.annotation.Nullable; +import java.util.concurrent.atomic.AtomicBoolean; + +public class TestDruidLeaderSelector implements DruidLeaderSelector +{ + private final AtomicBoolean isLeader = new AtomicBoolean(false); + private volatile Listener listener; + + public void becomeLeader() + { + if (isLeader.compareAndSet(false, true) && listener != null) { + listener.becomeLeader(); + } + } + + public void stopBeingLeader() + { + if (isLeader.compareAndSet(true, false) && listener != null) { + listener.stopBeingLeader(); + } + } + + @Nullable + @Override + public String getCurrentLeader() + { + return "me"; + } + + @Override + public boolean isLeader() + { + return isLeader.get(); + } + + @Override + public int localTerm() + { + return 0; + } + + @Override + public void registerListener(Listener listener) + { + this.listener = listener; + if (isLeader()) { + listener.becomeLeader(); + } + } + + @Override + public void unregisterListener() + { + listener = null; + } +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java new file mode 100644 index 000000000000..9ca037b0cfbf --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.audit.AuditInfo; +import org.apache.druid.metadata.MetadataRuleManager; +import org.apache.druid.server.coordinator.rules.ForeverLoadRule; +import org.apache.druid.server.coordinator.rules.Rule; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class TestMetadataRuleManager implements MetadataRuleManager +{ + private final Map> rules = new HashMap<>(); + + private static final String DEFAULT_DATASOURCE = "_default"; + + public TestMetadataRuleManager() + { + rules.put( + DEFAULT_DATASOURCE, + Collections.singletonList(new ForeverLoadRule(null)) + ); + } + + @Override + public void start() + { + // do nothing + } + + @Override + public void stop() + { + // do nothing + } + + @Override + public void poll() + { + // do nothing + } + + @Override + public Map> getAllRules() + { + return rules; + } + + @Override + public List getRules(final String dataSource) + { + List retVal = rules.get(dataSource); + return retVal == null ? new ArrayList<>() : retVal; + } + + @Override + public List getRulesWithDefault(final String dataSource) + { + List retVal = new ArrayList<>(); + final Map> theRules = rules; + if (theRules.get(dataSource) != null) { + retVal.addAll(theRules.get(dataSource)); + } + if (theRules.get(DEFAULT_DATASOURCE) != null) { + retVal.addAll(theRules.get(DEFAULT_DATASOURCE)); + } + return retVal; + } + + @Override + public boolean overrideRule(final String dataSource, final List newRules, final AuditInfo auditInfo) + { + rules.put(dataSource, newRules); + return true; + } + + @Override + public int removeRulesForEmptyDatasourcesOlderThan(long timestamp) + { + return 0; + } + + public void removeRulesForDatasource(String dataSource) + { + if (!DEFAULT_DATASOURCE.equals(dataSource)) { + rules.remove(dataSource); + } + } +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java new file mode 100644 index 000000000000..9f340a298246 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningScheduledExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import org.apache.druid.java.util.http.client.HttpClient; +import org.apache.druid.java.util.http.client.Request; +import org.apache.druid.java.util.http.client.response.HttpResponseHandler; +import org.apache.druid.server.coordination.DataSegmentChangeCallback; +import org.apache.druid.server.coordination.DataSegmentChangeHandler; +import org.apache.druid.server.coordination.DataSegmentChangeRequest; +import org.apache.druid.server.coordination.SegmentLoadDropHandler; +import org.jboss.netty.buffer.ChannelBuffers; +import org.jboss.netty.handler.codec.http.DefaultHttpResponse; +import org.jboss.netty.handler.codec.http.HttpResponse; +import org.jboss.netty.handler.codec.http.HttpResponseStatus; +import org.jboss.netty.handler.codec.http.HttpVersion; +import org.joda.time.Duration; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.List; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.Function; +import java.util.stream.Collectors; + +public class TestSegmentLoadingHttpClient implements HttpClient +{ + private static final HttpResponseHandler.TrafficCop NOOP_TRAFFIC_COP = checkNum -> 0L; + private static final DataSegmentChangeCallback NOOP_CALLBACK = () -> { + }; + + private final ObjectMapper objectMapper; + private final Function hostToHandler; + + private final ListeningScheduledExecutorService executorService; + + public TestSegmentLoadingHttpClient( + ObjectMapper objectMapper, + Function hostToHandler, + ScheduledExecutorService executorService + ) + { + this.objectMapper = objectMapper; + this.hostToHandler = hostToHandler; + this.executorService = MoreExecutors.listeningDecorator(executorService); + } + + @Override + public ListenableFuture go( + Request request, + HttpResponseHandler handler + ) + { + throw new UnsupportedOperationException(); + } + + @Override + public ListenableFuture go( + Request request, + HttpResponseHandler handler, + Duration readTimeout + ) + { + return executorService.submit(() -> processRequest(request, handler, readTimeout)); + } + + private Final processRequest( + Request request, + HttpResponseHandler handler, + Duration readTimeout + ) + { + try { + // Fail the request if there is no handler for this host + final DataSegmentChangeHandler changeHandler = hostToHandler + .apply(request.getUrl().getHost()); + if (changeHandler == null) { + final HttpResponse failureResponse = + new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.NOT_FOUND); + failureResponse.setContent(ChannelBuffers.EMPTY_BUFFER); + handler.handleResponse(failureResponse, NOOP_TRAFFIC_COP); + return (Final) new ByteArrayInputStream(new byte[0]); + } + + // Handle change requests and serialize + final byte[] serializedContent; + try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + objectMapper.writeValue(baos, processRequest(request, changeHandler)); + serializedContent = baos.toByteArray(); + } + + // Set response content and status + final HttpResponse response = + new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK); + response.setContent(ChannelBuffers.EMPTY_BUFFER); + handler.handleResponse(response, NOOP_TRAFFIC_COP); + return (Final) new ByteArrayInputStream(serializedContent); + } + catch (Exception e) { + throw new RuntimeException(e); + } + } + + /** + * Processes all the changes in the request. + */ + private List processRequest( + Request request, + DataSegmentChangeHandler changeHandler + ) throws IOException + { + final List changeRequests = objectMapper.readValue( + request.getContent().array(), + new TypeReference>() + { + } + ); + + return changeRequests + .stream() + .map(changeRequest -> processRequest(changeRequest, changeHandler)) + .collect(Collectors.toList()); + } + + /** + * Processes each DataSegmentChangeRequest using the handler. + */ + private SegmentLoadDropHandler.DataSegmentChangeRequestAndStatus processRequest( + DataSegmentChangeRequest request, + DataSegmentChangeHandler handler + ) + { + SegmentLoadDropHandler.Status status; + try { + request.go(handler, NOOP_CALLBACK); + status = SegmentLoadDropHandler.Status.SUCCESS; + } + catch (Exception e) { + status = SegmentLoadDropHandler.Status.failed(e.getMessage()); + } + + return new SegmentLoadDropHandler + .DataSegmentChangeRequestAndStatus(request, status); + } +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java new file mode 100644 index 000000000000..d0c319664058 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import com.google.common.base.Optional; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.client.DataSourcesSnapshot; +import org.apache.druid.client.ImmutableDruidDataSource; +import org.apache.druid.metadata.SegmentsMetadataManager; +import org.apache.druid.metadata.UnknownSegmentIdsException; +import org.apache.druid.timeline.DataSegment; +import org.apache.druid.timeline.Partitions; +import org.apache.druid.timeline.SegmentId; +import org.apache.druid.timeline.VersionedIntervalTimeline; +import org.joda.time.DateTime; +import org.joda.time.Interval; + +import javax.annotation.Nullable; +import java.util.Collection; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +public class TestSegmentsMetadataManager implements SegmentsMetadataManager +{ + private final ConcurrentMap segments = new ConcurrentHashMap<>(); + private final ConcurrentMap usedSegments = new ConcurrentHashMap<>(); + + public void addSegment(DataSegment segment) + { + segments.put(segment.getId(), segment); + usedSegments.put(segment.getId(), segment); + } + + public void removeSegment(DataSegment segment) + { + segments.remove(segment.getId()); + usedSegments.remove(segment.getId()); + } + + @Override + public void startPollingDatabasePeriodically() + { + + } + + @Override + public void stopPollingDatabasePeriodically() + { + + } + + @Override + public boolean isPollingDatabasePeriodically() + { + return true; + } + + @Override + public int markAsUsedAllNonOvershadowedSegmentsInDataSource(String dataSource) + { + return 0; + } + + @Override + public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interval interval) + { + return 0; + } + + @Override + public int markAsUsedNonOvershadowedSegments(String dataSource, Set segmentIds) + throws UnknownSegmentIdsException + { + return 0; + } + + @Override + public boolean markSegmentAsUsed(String segmentId) + { + // TODO + return false; + } + + @Override + public int markAsUnusedAllSegmentsInDataSource(String dataSource) + { + return 0; + } + + @Override + public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval) + { + return 0; + } + + @Override + public int markSegmentsAsUnused(Set segmentIds) + { + segmentIds.forEach(usedSegments::remove); + return 0; + } + + @Override + public boolean markSegmentAsUnused(SegmentId segmentId) + { + usedSegments.remove(segmentId); + return true; + } + + @Nullable + @Override + public ImmutableDruidDataSource getImmutableDataSourceWithUsedSegments(String dataSource) + { + return null; + } + + @Override + public Collection getImmutableDataSourcesWithAllUsedSegments() + { + return getSnapshotOfDataSourcesWithAllUsedSegments().getDataSourcesWithAllUsedSegments(); + } + + @Override + public Set getOvershadowedSegments() + { + return getSnapshotOfDataSourcesWithAllUsedSegments().getOvershadowedSegments(); + } + + @Override + public DataSourcesSnapshot getSnapshotOfDataSourcesWithAllUsedSegments() + { + return DataSourcesSnapshot.fromUsedSegments(usedSegments.values(), ImmutableMap.of()); + } + + @Override + public Iterable iterateAllUsedSegments() + { + return usedSegments.values(); + } + + @Override + public Optional> iterateAllUsedNonOvershadowedSegmentsForDatasourceInterval( + String datasource, + Interval interval, + boolean requiresLatest + ) + { + VersionedIntervalTimeline usedSegmentsTimeline + = getSnapshotOfDataSourcesWithAllUsedSegments().getUsedSegmentsTimelinesPerDataSource().get(datasource); + return Optional.fromNullable(usedSegmentsTimeline) + .transform(timeline -> timeline.findNonOvershadowedObjectsInInterval( + interval, + Partitions.ONLY_COMPLETE + )); + } + + @Override + public Set retrieveAllDataSourceNames() + { + return null; + } + + @Override + public List getUnusedSegmentIntervals(String dataSource, DateTime maxEndTime, int limit) + { + return null; + } + + @Override + public void poll() + { + + } +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java new file mode 100644 index 000000000000..885495346f21 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java @@ -0,0 +1,210 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.client.DruidServer; +import org.apache.druid.client.ServerInventoryView; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.logger.Logger; +import org.apache.druid.server.coordination.DataSegmentChangeCallback; +import org.apache.druid.server.coordination.DataSegmentChangeHandler; +import org.apache.druid.timeline.DataSegment; + +import javax.annotation.Nullable; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; + +public class TestServerInventoryView implements ServerInventoryView +{ + private static final Logger LOG = new Logger(TestServerInventoryView.class); + + private final ConcurrentHashMap servers = new ConcurrentHashMap<>(); + private final ConcurrentHashMap segmentChangeHandlers = new ConcurrentHashMap<>(); + + private final ConcurrentHashMap segmentCallbacks = new ConcurrentHashMap<>(); + private final List serverChangeHandlers = new ArrayList<>(); + + public void setUp() + { + segmentCallbacks.forEach( + (segmentCallback, executor) -> + executor.execute(segmentCallback::segmentViewInitialized) + ); + } + + /** + * Synchronizes this inventory view with the given inventory view. + */ + public void sync(ServerInventoryView other) + { + // Clear the current inventory + for (ServerChangeHandler handler : serverChangeHandlers) { + servers.values().forEach(handler::removeServer); + } + servers.clear(); + segmentChangeHandlers.clear(); + + for (DruidServer server : other.getInventory()) { + addServer(new DruidServer( + server.getName(), + server.getHostAndPort(), + server.getHostAndTlsPort(), + server.getMaxSize(), + server.getType(), + server.getTier(), + server.getPriority() + )); + DataSegmentChangeHandler handler = getChangeHandlerForHost(server.getName()); + for (DataSegment segment : server.iterateAllSegments()) { + handler.addSegment(segment, null); + } + } + } + + public void addServer(DruidServer server) + { + servers.put(server.getName(), server); + segmentChangeHandlers.put(server.getName(), new ChangeHandler(server)); + } + + public void removeServer(DruidServer server) + { + servers.remove(server.getName()); + segmentChangeHandlers.remove(server.getName()); + + for (ServerChangeHandler handler : serverChangeHandlers) { + handler.removeServer(server); + } + } + + public DataSegmentChangeHandler getChangeHandlerForHost(String serverName) + { + return segmentChangeHandlers.get(serverName); + } + + @Nullable + @Override + public DruidServer getInventoryValue(String serverKey) + { + return servers.get(serverKey); + } + + @Override + public Collection getInventory() + { + return Collections.unmodifiableCollection(servers.values()); + } + + @Override + public boolean isStarted() + { + return true; + } + + @Override + public boolean isSegmentLoadedByServer(String serverKey, DataSegment segment) + { + DruidServer server = servers.get(serverKey); + return server != null && server.getSegment(segment.getId()) != null; + } + + @Override + public void registerServerRemovedCallback(Executor exec, ServerRemovedCallback callback) + { + serverChangeHandlers.add(new ServerChangeHandler(callback, exec)); + } + + @Override + public void registerSegmentCallback(Executor exec, SegmentCallback callback) + { + segmentCallbacks.put(callback, exec); + } + + private class ChangeHandler implements DataSegmentChangeHandler + { + private final DruidServer server; + + private ChangeHandler(DruidServer server) + { + this.server = server; + } + + @Override + public void addSegment( + DataSegment segment, + @Nullable DataSegmentChangeCallback callback + ) + { + LOG.info("Adding segment [%s] to server [%s]", segment.getId(), server.getName()); + + if (server.getMaxSize() - server.getCurrSize() >= segment.getSize()) { + server.addDataSegment(segment); + segmentCallbacks.forEach( + (segmentCallback, executor) -> executor.execute( + () -> segmentCallback.segmentAdded(server.getMetadata(), segment) + ) + ); + } else { + throw new ISE( + "Not enough free space on server %s. Segment size [%d]. Current free space [%d]", + server.getName(), + segment.getSize(), + server.getMaxSize() - server.getCurrSize() + ); + } + } + + @Override + public void removeSegment( + DataSegment segment, + @Nullable DataSegmentChangeCallback callback + ) + { + LOG.info("Removing segment [%s] from server [%s]", segment.getId(), server.getName()); + server.removeDataSegment(segment.getId()); + segmentCallbacks.forEach( + (segmentCallback, executor) -> executor.execute( + () -> segmentCallback.segmentAdded(server.getMetadata(), segment) + ) + ); + } + } + + private static class ServerChangeHandler + { + private final Executor executor; + private final ServerRemovedCallback callback; + + private ServerChangeHandler(ServerRemovedCallback callback, Executor executor) + { + this.callback = callback; + this.executor = executor; + } + + private void removeServer(DruidServer server) + { + executor.execute(() -> callback.serverRemoved(server)); + } + } +} diff --git a/server/src/test/resources/log4j2.xml b/server/src/test/resources/log4j2.xml new file mode 100644 index 000000000000..d335523d0863 --- /dev/null +++ b/server/src/test/resources/log4j2.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + From 61c71ecb062674c58cd9623231d1d5d92307bcc9 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Mon, 12 Sep 2022 13:41:23 +0530 Subject: [PATCH 02/10] Remove outdated changes --- .../java/util/metrics/StubServiceEmitter.java | 5 ++++- .../server/coordinator/duty/RunRules.java | 1 - .../coordinator/CreateDataSegments.java | 19 +++---------------- .../simulate/TestServerInventoryView.java | 6 +++--- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java b/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java index 9d6032382ad1..cb8926500a72 100644 --- a/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java +++ b/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java @@ -19,6 +19,7 @@ package org.apache.druid.java.util.metrics; +import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.java.util.emitter.core.Event; import org.apache.druid.java.util.emitter.service.AlertEvent; import org.apache.druid.java.util.emitter.service.ServiceEmitter; @@ -28,6 +29,8 @@ public class StubServiceEmitter extends ServiceEmitter { + private static final Logger log = new Logger(StubServiceEmitter.class); + private final List events = new ArrayList<>(); public StubServiceEmitter(String service, String host) @@ -40,7 +43,7 @@ public void emit(Event event) { if (event instanceof AlertEvent) { final AlertEvent alertEvent = (AlertEvent) event; - System.out.printf( + log.warn( "[%s] [%s] [%s]: %s%n", alertEvent.getSeverity(), alertEvent.getService(), diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java index f6a96f220b88..cf285891f615 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/RunRules.java @@ -71,7 +71,6 @@ public RunRules(ReplicationThrottler replicatorThrottler, DruidCoordinator coord @Override public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) { - final long start = System.currentTimeMillis(); replicatorThrottler.updateParams( coordinator.getDynamicConfigs().getReplicationThrottleLimit(), coordinator.getDynamicConfigs().getReplicantLifetime(), diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java index 968da1dd357f..7663180c9a42 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java @@ -94,25 +94,12 @@ public List eachOfSizeMb(long sizeMb) nextStart = granularity.increment(nextStart); } - /*intervalsByGranularity.granularityIntervalsIterator().forEachRemaining( - subInterval -> { - for (int i = 0; i < numPartitionsPerInterval; ++i) { - segments.add( - new NumberedDataSegment( - datasource, - subInterval, - new NumberedShardSpec(i, numPartitionsPerInterval), - uniqueIdInInterval.incrementAndGet(), - sizeMb - ) - ); - } - } - );*/ - return segments; } + /** + * Simple implementation of DataSegment with a unique integer id to make debugging easier. + */ private static class NumberedDataSegment extends DataSegment { private final int uniqueId; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java index 885495346f21..bf9ccfe2c28a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java @@ -85,7 +85,7 @@ public void sync(ServerInventoryView other) public void addServer(DruidServer server) { servers.put(server.getName(), server); - segmentChangeHandlers.put(server.getName(), new ChangeHandler(server)); + segmentChangeHandlers.put(server.getName(), new SegmentChangeHandler(server)); } public void removeServer(DruidServer server) @@ -141,11 +141,11 @@ public void registerSegmentCallback(Executor exec, SegmentCallback callback) segmentCallbacks.put(callback, exec); } - private class ChangeHandler implements DataSegmentChangeHandler + private class SegmentChangeHandler implements DataSegmentChangeHandler { private final DruidServer server; - private ChangeHandler(DruidServer server) + private SegmentChangeHandler(DruidServer server) { this.server = server; } From 63bf32567bd603a3d1aae2375351c60b83b3499f Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 14 Sep 2022 11:44:12 +0530 Subject: [PATCH 03/10] Add more tests --- .../coordinator/duty/BalanceSegments.java | 7 +- .../simulate/BlockingExecutorService.java | 113 +---- .../simulate/CoordinatorSimulation.java | 7 +- .../CoordinatorSimulationBaseTest.java | 113 ++++- ...java => CoordinatorSimulationBuilder.java} | 419 ++++++++++-------- .../simulate/SegmentBalancingTest.java | 80 ++-- .../simulate/SegmentLoadingTest.java | 162 ++++++- .../simulate/TestSegmentsMetadataManager.java | 12 +- .../simulate/TestServerInventoryView.java | 6 +- .../WrappingScheduledExecutorService.java | 240 ++++++++++ 10 files changed, 795 insertions(+), 364 deletions(-) rename server/src/test/java/org/apache/druid/server/coordinator/simulate/{CoordinatorSimulationImpl.java => CoordinatorSimulationBuilder.java} (50%) create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java index 198a7cf5e8f6..d08bb59568fa 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java @@ -251,14 +251,15 @@ private Pair balanceServers( if (moveSegment(segmentToMoveHolder, destinationHolder.getServer(), params)) { moved++; } else { + log.info("Segment [%s] cannot be moved. Maybe no space left or already being served or already in queue.", segmentToMove.getId()); unmoved++; } } else { - log.debug("Segment [%s] is 'optimally' placed.", segmentToMove.getId()); + log.info("Segment [%s] is 'optimally' placed.", segmentToMove.getId()); unmoved++; } } else { - log.debug("No valid movement destinations for segment [%s].", segmentToMove.getId()); + log.info("No valid movement destinations for segment [%s].", segmentToMove.getId()); unmoved++; } } @@ -291,7 +292,7 @@ protected boolean moveSegment( if (!toPeon.getSegmentsToLoad().contains(segmentToMove) && (toServer.getSegment(segmentId) == null) && new ServerHolder(toServer, toPeon).getAvailableSize() > segmentToMove.getSize()) { - log.debug("Moving [%s] from [%s] to [%s]", segmentId, fromServer.getName(), toServer.getName()); + log.info("Moving [%s] from [%s] to [%s]", segmentId, fromServer.getName(), toServer.getName()); LoadPeonCallback callback = null; try { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java index 3c1eb13daea2..efb664eba603 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java @@ -28,13 +28,9 @@ import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.Delayed; -import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; /** * An executor that keeps submitted tasks in a queue until they are explicitly @@ -45,18 +41,16 @@ *
  • {@link #finishAllPendingTasks()}
    • * */ -public class BlockingExecutorService implements ScheduledExecutorService +public class BlockingExecutorService implements ExecutorService { private static final Logger log = new Logger(BlockingExecutorService.class); private final String nameFormat; - private final boolean ignoreScheduledTasks; private final Queue> taskQueue = new ConcurrentLinkedQueue<>(); - public BlockingExecutorService(String nameFormat, boolean ignoreScheduledTasks) + public BlockingExecutorService(String nameFormat) { this.nameFormat = nameFormat; - this.ignoreScheduledTasks = ignoreScheduledTasks; } public boolean hasPendingTasks() @@ -148,30 +142,6 @@ public void execute(Runnable command) submit(command); } - @Override - public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) - { - if (ignoreScheduledTasks) { - log.debug("[%s] Ignoring scheduled task", nameFormat); - return new WrappingScheduledFuture<>(CompletableFuture.completedFuture(null)); - } - - // Ignore the delay and just queue the task - return new WrappingScheduledFuture<>(submit(command)); - } - - @Override - public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) - { - if (ignoreScheduledTasks) { - log.debug("[%s] Ignoring scheduled task", nameFormat); - return new WrappingScheduledFuture<>(CompletableFuture.completedFuture(null)); - } - - // Ignore the delay and just queue the task - return new WrappingScheduledFuture<>(submit(callable)); - } - private Future addTaskToQueue(Callable callable) { Task task = new Task<>(callable); @@ -239,28 +209,6 @@ public T invokeAny(Collection> tasks, long timeout, Ti throw new UnsupportedOperationException(); } - @Override - public ScheduledFuture scheduleAtFixedRate( - Runnable command, - long initialDelay, - long period, - TimeUnit unit - ) - { - throw new UnsupportedOperationException(); - } - - @Override - public ScheduledFuture scheduleWithFixedDelay( - Runnable command, - long initialDelay, - long delay, - TimeUnit unit - ) - { - throw new UnsupportedOperationException(); - } - /** * Task that can be invoked to complete the corresponding future. */ @@ -286,59 +234,4 @@ private void executeNow() } } - /** - * Wraps a Future into a ScheduledFuture. - */ - private static class WrappingScheduledFuture implements ScheduledFuture - { - private final Future future; - - private WrappingScheduledFuture(Future future) - { - this.future = future; - } - - @Override - public long getDelay(TimeUnit unit) - { - return 0; - } - - @Override - public int compareTo(Delayed o) - { - return 0; - } - - @Override - public boolean cancel(boolean mayInterruptIfRunning) - { - return future.cancel(mayInterruptIfRunning); - } - - @Override - public boolean isCancelled() - { - return future.isCancelled(); - } - - @Override - public boolean isDone() - { - return future.isDone(); - } - - @Override - public V get() throws InterruptedException, ExecutionException - { - return future.get(); - } - - @Override - public V get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException - { - return future.get(timeout, unit); - } - } - } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java index 2ba5fce67836..c497258cdfdc 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java @@ -43,6 +43,11 @@ public interface CoordinatorSimulation ClusterState cluster(); + static CoordinatorSimulationBuilder builder() + { + return new CoordinatorSimulationBuilder(); + } + /** * Represents the state of the coordinator during a simulation. */ @@ -51,7 +56,7 @@ interface CoordinatorState /** * Runs a single coordinator cycle. */ - void runCycle(); + void runCoordinatorCycle(); /** * Synchronizes the inventory view maintained by the coordinator with the diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java index 17ac6d4ee23f..3f189017db91 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -20,18 +20,25 @@ package org.apache.druid.server.coordinator.simulate; import org.apache.druid.client.DruidServer; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.emitter.core.Event; import org.apache.druid.server.coordination.ServerType; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; +import org.apache.druid.server.coordinator.CreateDataSegments; import org.apache.druid.server.coordinator.rules.ForeverLoadRule; import org.apache.druid.server.coordinator.rules.Rule; +import org.apache.druid.timeline.DataSegment; import org.junit.After; import org.junit.Assert; import org.junit.Before; import java.util.ArrayList; -import java.util.Collections; +import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; /** * Base test for coordinator simulations. @@ -39,37 +46,29 @@ * Each test must call {@link #startSimulation(CoordinatorSimulation)} to start * the simulation. {@link CoordinatorSimulation#stop()} should not be called as * the simulation is stopped when cleaning up after the test in {@link #tearDown()}. + *

    + * Tests that verify balancing behaviour should set + * {@link CoordinatorDynamicConfig#useBatchedSegmentSampler()} to true. + * Otherwise, the segment sampling is random and can produce repeated values + * leading to flakiness in the tests. The simulation sets this field to true by + * default. */ -public class CoordinatorSimulationBaseTest +public abstract class CoordinatorSimulationBaseTest implements CoordinatorSimulation.CoordinatorState, CoordinatorSimulation.ClusterState { static final double DOUBLE_DELTA = 10e-9; - static class Rules - { - static final List T1_X1 = Collections.singletonList( - new ForeverLoadRule(Collections.singletonMap(Tier.T1, 1)) - ); - } - - static class Tier - { - static final String T1 = "_default_tier"; - } - private CoordinatorSimulation sim; @Before - public void setUp() - { - sim = null; - } + public abstract void setUp(); @After public void tearDown() { if (sim != null) { sim.stop(); + sim = null; } } @@ -80,9 +79,9 @@ void startSimulation(CoordinatorSimulation simulation) } @Override - public void runCycle() + public void runCoordinatorCycle() { - sim.coordinator().runCycle(); + sim.coordinator().runCoordinatorCycle(); } @Override @@ -144,6 +143,32 @@ private List getMetricValues(String metricName) } // Utility methods + static List createWikiSegmentsForIntervals( + int numIntervals, + Granularity granularity, + int partitionsPerInterval + ) + { + return CreateDataSegments.ofDatasource(DS.WIKI) + .forIntervals(1, Granularities.DAY) + .startingAt("2022-01-01") + .andPartitionsPerInterval(10) + .eachOfSizeMb(500); + } + + static CoordinatorDynamicConfig createDynamicConfig( + int maxSegmentsToMove, + int maxSegmentsInNodeLoadingQueue, + int replicationThrottleLimit + ) + { + return CoordinatorDynamicConfig.builder() + .withMaxSegmentsToMove(maxSegmentsToMove) + .withReplicationThrottleLimit(replicationThrottleLimit) + .withMaxSegmentsInNodeLoadingQueue(maxSegmentsInNodeLoadingQueue) + .withUseBatchedSegmentSampler(true) + .build(); + } /** * Creates a tier of historicals. @@ -166,4 +191,52 @@ static DruidServer createHistorical(int uniqueIdInTier, String tier, long server return new DruidServer(name, name, name, serverSizeMb, ServerType.HISTORICAL, tier, 1); } + // Utility and constant holder classes + + static class DS + { + static final String WIKI = "wiki"; + } + + static class Tier + { + static final String T1 = "tier_t1"; + static final String T2 = "tier_t2"; + } + + /** + * Builder for retention rules. + * + * @see Load + */ + interface RuleBuilder + { + + } + + /** + * Builder for a load rule. + */ + static class Load implements RuleBuilder + { + private final Map tieredReplicants = new HashMap<>(); + + static Load on(String tier, int numReplicas) + { + Load load = new Load(); + load.tieredReplicants.put(tier, numReplicas); + return load; + } + + Load andOn(String tier, int numReplicas) + { + tieredReplicants.put(tier, numReplicas); + return this; + } + + Rule forever() + { + return new ForeverLoadRule(tieredReplicants); + } + } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationImpl.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java similarity index 50% rename from server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationImpl.java rename to server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java index e42be2b03925..746eeb9caca8 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationImpl.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java @@ -28,6 +28,7 @@ import org.apache.druid.curator.discovery.ServiceAnnouncer; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.concurrent.DirectExecutorService; import org.apache.druid.java.util.common.concurrent.ScheduledExecutorFactory; import org.apache.druid.java.util.common.lifecycle.Lifecycle; import org.apache.druid.java.util.emitter.EmittingLogger; @@ -51,22 +52,22 @@ import org.joda.time.Duration; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; /** - * Implementation of {@link CoordinatorSimulation}. - * - * @see #builder() + * Builder for {@link CoordinatorSimulation}. */ -public class CoordinatorSimulationImpl implements CoordinatorSimulation, - CoordinatorSimulation.CoordinatorState, CoordinatorSimulation.ClusterState +public class CoordinatorSimulationBuilder { + private static final long DEFAULT_COORDINATOR_PERIOD = 100L; private static final ObjectMapper OBJECT_MAPPER = new DefaultObjectMapper() .setInjectableValues( new InjectableValues.Std().addValue( @@ -74,239 +75,265 @@ public class CoordinatorSimulationImpl implements CoordinatorSimulation, DataSegment.PruneSpecsHolder.DEFAULT ) ); - private static final long DEFAULT_COORDINATOR_PERIOD = 100L; - private final AtomicBoolean running = new AtomicBoolean(false); - - private final DruidCoordinator coordinator; - private final Environment env; - - private CoordinatorSimulationImpl(DruidCoordinator coordinator, Environment env) + private BalancerStrategyFactory balancerStrategyFactory; + private CoordinatorDynamicConfig dynamicConfig = + CoordinatorDynamicConfig.builder() + .withUseBatchedSegmentSampler(true) + .build(); + private List servers; + private List segments; + private final Map> datasourceRules = new HashMap<>(); + private boolean immediateLoading; + + public CoordinatorSimulationBuilder balancer(BalancerStrategyFactory strategyFactory) { - this.coordinator = coordinator; - this.env = env; + this.balancerStrategyFactory = strategyFactory; + return this; } - /** - * Creates a new simulation builder. - */ - public static Builder builder() + public CoordinatorSimulationBuilder servers(List servers) { - return new Builder(); + this.servers = servers; + return this; } - @Override - public void start() + public CoordinatorSimulationBuilder servers(DruidServer... servers) { - if (!running.compareAndSet(false, true)) { - throw new ISE("Simulation is already running"); - } - - try { - env.setUp(); - coordinator.start(); - } - catch (Exception e) { - throw new ISE(e, "Exception while running simulation"); - } + return servers(Arrays.asList(servers)); } - @Override - public void stop() + public CoordinatorSimulationBuilder segments(List segments) { - coordinator.stop(); - env.leaderSelector.stopBeingLeader(); - env.tearDown(); + this.segments = segments; + return this; } - @Override - public CoordinatorState coordinator() + public CoordinatorSimulationBuilder rules(String datasource, Rule... rules) { + this.datasourceRules.put(datasource, Arrays.asList(rules)); return this; } - @Override - public ClusterState cluster() + /** + * Causes segments to be loaded as soon as they are queued. + */ + public CoordinatorSimulationBuilder immediateSegmentLoading() { + this.immediateLoading = true; return this; } - // Blocked mode processing invocations - - @Override - public void runCycle() + /** + * Specifies the CoordinatorDynamicConfig to be used in the simulation. + *

    + * If not specified, the default values are used. + */ + public CoordinatorSimulationBuilder dynamicConfig(CoordinatorDynamicConfig dynamicConfig) { - verifySimulationRunning(); - env.serviceEmitter.flush(); - env.executorFactory.coordinatorRunner.finishNextPendingTasks(1); + this.dynamicConfig = dynamicConfig; + return this; } - @Override - public void syncInventoryView() + public CoordinatorSimulation build() { - verifySimulationRunning(); - env.coordinatorInventoryView.sync(env.historicalInventoryView); - } + Preconditions.checkArgument( + servers != null && !servers.isEmpty(), + "Cannot run simulation for an empty cluster" + ); + + // Prepare the config + final DruidCoordinatorConfig coordinatorConfig = new TestDruidCoordinatorConfig.Builder() + .withCoordinatorStartDelay(new Duration(1L)) + .withCoordinatorPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) + .withCoordinatorKillPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) + .withLoadQueuePeonRepeatDelay(new Duration("PT0S")) + .withLoadQueuePeonType("http") + .withCoordinatorKillIgnoreDurationToRetain(false) + .build(); + + // Prepare the environment + final TestServerInventoryView serverInventoryView = new TestServerInventoryView(); + servers.forEach(serverInventoryView::addServer); + + final TestSegmentsMetadataManager segmentManager = new TestSegmentsMetadataManager(); + if (segments != null) { + segments.forEach(segmentManager::addSegment); + } - @Override - public DruidServer getInventoryView(String serverName) - { - return env.coordinatorInventoryView.getInventoryValue(serverName); + final TestMetadataRuleManager ruleManager = new TestMetadataRuleManager(); + datasourceRules.forEach( + (datasource, rules) -> + ruleManager.overrideRule(datasource, rules, null) + ); + + final Environment env = new Environment( + new TestDruidLeaderSelector(), + serverInventoryView, + segmentManager, + ruleManager, + coordinatorConfig, + dynamicConfig, + immediateLoading + ); + + // Build the coordinator + final DruidCoordinator coordinator = new DruidCoordinator( + env.coordinatorConfig, + null, + env.jacksonConfigManager, + env.segmentManager, + env.historicalInventoryView, + env.ruleManager, + () -> null, + env.serviceEmitter, + env.executorFactory, + null, + env.loadQueueTaskMaster, + new ServiceAnnouncer.Noop(), + null, + Collections.emptySet(), + null, + new CoordinatorCustomDutyGroups(Collections.emptySet()), + balancerStrategyFactory != null ? balancerStrategyFactory + : buildCachingCostBalancerStrategy(env), + env.lookupCoordinatorManager, + env.leaderSelector, + OBJECT_MAPPER, + ZkEnablementConfig.ENABLED + ); + + return new SimulationImpl(coordinator, env); } - @Override - public void loadQueuedSegments() + private BalancerStrategyFactory buildCachingCostBalancerStrategy(Environment env) { - verifySimulationRunning(); - - final BlockingExecutorService loadQueueExecutor = env.executorFactory.loadQueueExecutor; - while (loadQueueExecutor.hasPendingTasks()) { - // Drain all the items from the load queue executor - // This sends at most 1 load/drop request to each server - loadQueueExecutor.finishAllPendingTasks(); - - // Load all the queued segments, handle their responses and execute callbacks - int loadedSegments = env.executorFactory.historicalLoader.finishAllPendingTasks(); - loadQueueExecutor.finishNextPendingTasks(loadedSegments); - env.executorFactory.loadCallbackExecutor.finishAllPendingTasks(); + try { + return new CachingCostBalancerStrategyFactory( + env.coordinatorInventoryView, + env.lifecycle, + new CachingCostBalancerStrategyConfig() + ); } - } - - private void verifySimulationRunning() - { - if (!running.get()) { - throw new ISE("Simulation hasn't been started yet."); + catch (Exception e) { + throw new ISE(e, "Error building balancer strategy"); } } - @Override - public double getLoadPercentage(String datasource) - { - return coordinator.getLoadStatus().get(datasource); - } - - @Override - public List getMetricEvents() - { - return new ArrayList<>(env.serviceEmitter.getEvents()); - } - /** - * Builder for a coordinator simulation. + * Implementation of {@link CoordinatorSimulation}. */ - public static class Builder + private static class SimulationImpl implements CoordinatorSimulation, + CoordinatorSimulation.CoordinatorState, CoordinatorSimulation.ClusterState { - private BalancerStrategyFactory balancerStrategyFactory; - private List servers; - private List segments; - private final Map> datasourceRules = new HashMap<>(); + private final AtomicBoolean running = new AtomicBoolean(false); + + private final Environment env; + private final DruidCoordinator coordinator; - public Builder balancer(BalancerStrategyFactory strategyFactory) + private SimulationImpl(DruidCoordinator coordinator, Environment env) { - this.balancerStrategyFactory = strategyFactory; - return this; + this.env = env; + this.coordinator = coordinator; } - public Builder servers(List servers) + @Override + public void start() { - this.servers = servers; - return this; + if (!running.compareAndSet(false, true)) { + throw new ISE("Simulation is already running"); + } + + try { + env.setUp(); + coordinator.start(); + } + catch (Exception e) { + throw new ISE(e, "Exception while running simulation"); + } } - public Builder segments(List segments) + @Override + public void stop() + { + coordinator.stop(); + env.leaderSelector.stopBeingLeader(); + env.tearDown(); + } + + @Override + public CoordinatorState coordinator() { - this.segments = segments; return this; } - public Builder rulesForDatasource(String datasource, List rules) + @Override + public ClusterState cluster() { - this.datasourceRules.put(datasource, rules); return this; } - public CoordinatorSimulationImpl build() + @Override + public void runCoordinatorCycle() { - Preconditions.checkArgument( - servers != null && !servers.isEmpty(), - "Cannot run simulation for an empty cluster" - ); + verifySimulationRunning(); + env.serviceEmitter.flush(); + env.executorFactory.coordinatorRunner.finishNextPendingTasks(1); + } - // Prepare the config - final DruidCoordinatorConfig coordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withCoordinatorStartDelay(new Duration(1L)) - .withCoordinatorPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) - .withCoordinatorKillPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) - .withLoadQueuePeonRepeatDelay(new Duration("PT0S")) - .withLoadQueuePeonType("http") - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - - // Prepare the environment - final TestServerInventoryView serverInventoryView = new TestServerInventoryView(); - servers.forEach(serverInventoryView::addServer); - - final TestSegmentsMetadataManager segmentManager = new TestSegmentsMetadataManager(); - if (segments != null) { - segments.forEach(segmentManager::addSegment); - } + @Override + public void syncInventoryView() + { + verifySimulationRunning(); + env.coordinatorInventoryView.sync(env.historicalInventoryView); + } - final TestMetadataRuleManager ruleManager = new TestMetadataRuleManager(); - datasourceRules.forEach( - (datasource, rules) -> - ruleManager.overrideRule(datasource, rules, null) - ); + @Override + public DruidServer getInventoryView(String serverName) + { + return env.coordinatorInventoryView.getInventoryValue(serverName); + } - final Environment env = new Environment( - new TestDruidLeaderSelector(), - serverInventoryView, - segmentManager, - ruleManager, - coordinatorConfig + @Override + public void loadQueuedSegments() + { + verifySimulationRunning(); + Preconditions.checkState( + !env.immediateLoading, + "Cannot invoke loadQueuedSegments as simulation is running in immediate loading mode." ); - // Build the coordinator - final DruidCoordinator coordinator = new DruidCoordinator( - env.coordinatorConfig, - null, - env.jacksonConfigManager, - env.segmentManager, - env.historicalInventoryView, - env.ruleManager, - () -> null, - env.serviceEmitter, - env.executorFactory, - null, - env.loadQueueTaskMaster, - new ServiceAnnouncer.Noop(), - null, - Collections.emptySet(), - null, - new CoordinatorCustomDutyGroups(Collections.emptySet()), - balancerStrategyFactory != null ? balancerStrategyFactory - : buildCachingCostBalancerStrategy(env), - env.lookupCoordinatorManager, - env.leaderSelector, - OBJECT_MAPPER, - ZkEnablementConfig.ENABLED - ); + final BlockingExecutorService loadQueueExecutor = env.executorFactory.loadQueueExecutor; + while (loadQueueExecutor.hasPendingTasks()) { + // Drain all the items from the load queue executor + // This sends at most 1 load/drop request to each server + loadQueueExecutor.finishAllPendingTasks(); - return new CoordinatorSimulationImpl(coordinator, env); + // Load all the queued segments, handle their responses and execute callbacks + int loadedSegments = env.executorFactory.historicalLoader.finishAllPendingTasks(); + loadQueueExecutor.finishNextPendingTasks(loadedSegments); + env.executorFactory.loadCallbackExecutor.finishAllPendingTasks(); + } } - private BalancerStrategyFactory buildCachingCostBalancerStrategy(Environment env) + private void verifySimulationRunning() { - try { - return new CachingCostBalancerStrategyFactory( - env.coordinatorInventoryView, - env.lifecycle, - new CachingCostBalancerStrategyConfig() - ); - } - catch (Exception e) { - throw new ISE(e, "Error building balancer strategy"); + if (!running.get()) { + throw new ISE("Simulation hasn't been started yet."); } } + + @Override + public double getLoadPercentage(String datasource) + { + return coordinator.getLoadStatus().get(datasource); + } + + @Override + public List getMetricEvents() + { + return new ArrayList<>(env.serviceEmitter.getEvents()); + } } /** @@ -332,6 +359,7 @@ private static class Environment private final JacksonConfigManager jacksonConfigManager; private final LookupCoordinatorManager lookupCoordinatorManager; private final DruidCoordinatorConfig coordinatorConfig; + private final boolean immediateLoading; private final List mocks = new ArrayList<>(); @@ -340,7 +368,9 @@ private Environment( TestServerInventoryView historicalInventoryView, TestSegmentsMetadataManager segmentManager, TestMetadataRuleManager ruleManager, - DruidCoordinatorConfig coordinatorConfig + DruidCoordinatorConfig coordinatorConfig, + CoordinatorDynamicConfig dynamicConfig, + boolean immediateLoading ) { this.leaderSelector = leaderSelector; @@ -348,8 +378,9 @@ private Environment( this.segmentManager = segmentManager; this.ruleManager = ruleManager; this.coordinatorConfig = coordinatorConfig; + this.immediateLoading = immediateLoading; - this.executorFactory = new ExecutorFactory(); + this.executorFactory = new ExecutorFactory(immediateLoading); this.coordinatorInventoryView = new TestServerInventoryView(); HttpClient httpClient = new TestSegmentLoadingHttpClient( OBJECT_MAPPER, @@ -367,7 +398,7 @@ private Environment( null ); - this.jacksonConfigManager = mockConfigManager(); + this.jacksonConfigManager = mockConfigManager(dynamicConfig); this.lookupCoordinatorManager = EasyMock.createNiceMock(LookupCoordinatorManager.class); mocks.add(jacksonConfigManager); mocks.add(lookupCoordinatorManager); @@ -391,16 +422,8 @@ private void tearDown() lifecycle.stop(); } - private JacksonConfigManager mockConfigManager() + private JacksonConfigManager mockConfigManager(CoordinatorDynamicConfig dynamicConfig) { - final CoordinatorDynamicConfig dynamicConfig = - CoordinatorDynamicConfig - .builder() - .withMaxSegmentsToMove(100) - .withMaxSegmentsInNodeLoadingQueue(0) - .withReplicationThrottleLimit(100000) - .build(); - final JacksonConfigManager jacksonConfigManager = EasyMock.createMock(JacksonConfigManager.class); EasyMock.expect( @@ -423,6 +446,10 @@ private JacksonConfigManager mockConfigManager() } } + /** + * Implementation of {@link ScheduledExecutorFactory} used to create and keep + * a handle on the various executors used inside the coordinator. + */ private static class ExecutorFactory implements ScheduledExecutorFactory { static final String HISTORICAL_LOADER = "historical-loader-%d"; @@ -431,20 +458,30 @@ private static class ExecutorFactory implements ScheduledExecutorFactory static final String COORDINATOR_RUNNER = "Coordinator-Exec--%d"; private final Map blockingExecutors = new HashMap<>(); + private final boolean directExecution; private BlockingExecutorService historicalLoader; private BlockingExecutorService loadQueueExecutor; private BlockingExecutorService loadCallbackExecutor; private BlockingExecutorService coordinatorRunner; + private ExecutorFactory(boolean directExecution) + { + this.directExecution = directExecution; + } + @Override public ScheduledExecutorService create(int corePoolSize, String nameFormat) { boolean isCoordinatorRunner = COORDINATOR_RUNNER.equals(nameFormat); - return blockingExecutors.computeIfAbsent( - nameFormat, - k -> new BlockingExecutorService(k, !isCoordinatorRunner) - ); + + // Coordinator running executor must always be blocked + final ExecutorService executorService = + (directExecution && !isCoordinatorRunner) + ? new DirectExecutorService() + : blockingExecutors.computeIfAbsent(nameFormat, BlockingExecutorService::new); + + return new WrappingScheduledExecutorService(nameFormat, executorService, !isCoordinatorRunner); } private BlockingExecutorService findExecutor(String nameFormat) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java index 661e0a864a68..ad0a025bb804 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java @@ -23,8 +23,10 @@ import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.server.coordinator.CostBalancerStrategyFactory; import org.apache.druid.server.coordinator.CreateDataSegments; +import org.apache.druid.server.coordinator.rules.Rule; import org.apache.druid.timeline.DataSegment; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import java.util.List; @@ -34,44 +36,54 @@ */ public class SegmentBalancingTest extends CoordinatorSimulationBaseTest { + // TODO: randomness and flakiness can creep in if the maxSegmentsToMove < totalLoadedSegments + @Override + public void setUp() + { + + } + @Test - public void testBalancingWithInventoryViewUpdates() + public void testBalancingWithUpdatedInventory() { testBalancingWhenInventoryIsSynced(true); + } - // Fix https://github.com/apache/druid/issues/12881 to enable this test case - // testBalancingWhenInventoryIsSynced(false); + @Test + @Ignore("Fix #12881 to enable this test. " + + "Current impl requires updated inventory for correct callback behaviour.") + public void testBalancingWithStaleInventory() + { + testBalancingWhenInventoryIsSynced(false); } private void testBalancingWhenInventoryIsSynced(boolean syncInventory) { - final String datasource = "wikitest"; - - // Setup servers and segments - final List historicals = createHistoricalTier(Tier.T1, 2, 10_000); + // historicals = 2(T1), segments = 10(1 day) + final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); + final DruidServer historicalT12 = createHistorical(2, Tier.T1, 10_000); final List segments = - CreateDataSegments.ofDatasource(datasource) + CreateDataSegments.ofDatasource(DS.WIKI) .forIntervals(1, Granularities.DAY) .startingAt("2022-01-01") .andPartitionsPerInterval(10) .eachOfSizeMb(500); - // Put all the segments on historicalA - final DruidServer historicalA = historicals.get(0); - final DruidServer historicalB = historicals.get(1); - segments.forEach(historicalA::addDataSegment); - // Build and run the simulation + // strategy = cost, replicas = 1(T1) final CoordinatorSimulation sim = - CoordinatorSimulationImpl.builder() - .balancer(new CostBalancerStrategyFactory()) - .segments(segments) - .servers(historicals) - .rulesForDatasource(datasource, Rules.T1_X1) - .build(); + CoordinatorSimulation.builder() + .segments(segments) + .servers(historicalT11, historicalT12) + .rules(DS.WIKI, Load.on(Tier.T1, 1).forever()) + .balancer(new CostBalancerStrategyFactory()) + .build(); + + // Put all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); startSimulation(sim); - runCycle(); + runCoordinatorCycle(); // Verify that segments have been chosen for balancing verifyLatestMetricValue("segment/moved/count", 5L); @@ -82,23 +94,37 @@ private void testBalancingWhenInventoryIsSynced(boolean syncInventory) } // Verify that segments have now been balanced out - final DruidServer historicalViewA = getInventoryView(historicalA.getName()); - final DruidServer historicalViewB = getInventoryView(historicalB.getName()); - Assert.assertEquals(5, historicalViewA.getTotalSegments()); - Assert.assertEquals(5, historicalViewB.getTotalSegments()); - verifyDatasourceIsFullyLoaded(datasource); + final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); + final DruidServer historicalViewT12 = getInventoryView(historicalT12.getName()); + Assert.assertEquals(5, historicalViewT11.getTotalSegments()); + Assert.assertEquals(5, historicalViewT12.getTotalSegments()); + verifyDatasourceIsFullyLoaded(DS.WIKI); } @Test - public void testBalancingSkipsOvershadowedSegments() + public void testBalancingSkipsUnusedSegments() { + // have a bunch of segments + // mark them all as unused + // verify that nothing is chosen for balancing + } + @Test + public void testBalancingSkipsOvershadowedSegments() + { + // have a bunch of segments + // overshadow all but one + // verify that none of the overshadowed ones are chosen for balancing } @Test public void testBalancingOfFullyReplicatedSegment() { - + // a couple of segments + // all of them are fully loaded + // something will be chosen for balancing + // verify that the segments on the move don't count towards over-replication + // this will require 2 coordinator runs } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java index fef7e0a00379..09bbcd4c089a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java @@ -19,29 +19,179 @@ package org.apache.druid.server.coordinator.simulate; +import org.apache.druid.client.DruidServer; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; +import org.apache.druid.server.coordinator.CostBalancerStrategyFactory; +import org.apache.druid.server.coordinator.CreateDataSegments; +import org.apache.druid.server.coordinator.rules.Rule; +import org.apache.druid.timeline.DataSegment; +import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; +import java.util.List; + /** * Coordinator simulation test to verify behaviour of segment loading. */ public class SegmentLoadingTest extends CoordinatorSimulationBaseTest { + @Override + public void setUp() + { + + } + @Test - public void testPrimaryReplicaOnTierIsNotThrottled() + @Ignore("Fix #12881 to enable this test. " + + "Current behaviour is to throttle globally and not per tier.") + public void testFirstReplicaOnAnyTierIsNotThrottled() { + // historicals = 1(in T1) + 1(in T2), segments = 10*1day + final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); + final DruidServer historicalT21 = createHistorical(1, Tier.T2, 10_000); + final List segments = + CreateDataSegments.ofDatasource(DS.WIKI) + .forIntervals(1, Granularities.DAY) + .startingAt("2022-01-01") + .andPartitionsPerInterval(10) + .eachOfSizeMb(500); + + // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); + + // strategy = cost, replicas = 1(on T1) + 1(on T2) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .segments(segments) + .servers(historicalT11, historicalT21) + .dynamicConfig(dynamicConfig) + .balancer(new CostBalancerStrategyFactory()) + .rules( + DS.WIKI, + Load.on(Tier.T1, 1).andOn(Tier.T2, 1).forever() + ) + .build(); + + // Put the first replica of all the segments on T1 + segments.forEach(historicalT11::addDataSegment); + startSimulation(sim); + runCoordinatorCycle(); + + // Verify that all the missing replicas for T2 have been assigned + verifyLatestMetricValue("segment/assigned/count", 10L); + + loadQueuedSegments(); + syncInventoryView(); + + final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); + final DruidServer historicalViewT21 = getInventoryView(historicalT21.getName()); + Assert.assertEquals(10, historicalViewT11.getTotalSegments()); + Assert.assertEquals(10, historicalViewT21.getTotalSegments()); + verifyDatasourceIsFullyLoaded(DS.WIKI); } @Test - public void testReplicationThrottleLimit() + public void testSecondReplicaOnAnyTierIsThrottled() { + // historicals = 2(in T1), segments = 10*1day + final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); + final DruidServer historicalT12 = createHistorical(2, Tier.T1, 10_000); + final List segments = + CreateDataSegments.ofDatasource(DS.WIKI) + .forIntervals(1, Granularities.DAY) + .startingAt("2022-01-01") + .andPartitionsPerInterval(10) + .eachOfSizeMb(500); + + // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); + + // strategy = cost, replicas = 2(on T1) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .segments(segments) + .servers(historicalT11, historicalT12) + .rules(DS.WIKI, Load.on(Tier.T1, 2).forever()) + .dynamicConfig(dynamicConfig) + .balancer(new CostBalancerStrategyFactory()) + .build(); + + // Put the first replica of all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); + + startSimulation(sim); + runCoordinatorCycle(); + // Verify that that replicationThrottleLimit is honored + verifyLatestMetricValue("segment/assigned/count", 2L); + + loadQueuedSegments(); + syncInventoryView(); + + final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); + final DruidServer historicalViewT12 = getInventoryView(historicalT12.getName()); + Assert.assertEquals(10, historicalViewT11.getTotalSegments()); + Assert.assertEquals(2, historicalViewT12.getTotalSegments()); } @Test - public void testHistoricalsAreNotOverAssigned() + @Ignore("Fix #12881 to enable this test. " + + "Current impl allows throttle limit to be violated if loading is fast.") + public void testReplicationThrottleWithImmediateLoading() { + // historicals = 2(in T1), segments = 10*1day + final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); + final DruidServer historicalT12 = createHistorical(2, Tier.T1, 10_000); + final List segments = + CreateDataSegments.ofDatasource(DS.WIKI) + .forIntervals(1, Granularities.DAY) + .startingAt("2022-01-01") + .andPartitionsPerInterval(10) + .eachOfSizeMb(500); + + // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); + + // strategy = cost, replicas = 2(on T1), immediate segment loading + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .balancer(new CostBalancerStrategyFactory()) + .segments(segments) + .servers(historicalT11, historicalT12) + .rules(DS.WIKI, Load.on(Tier.T1, 2).forever()) + .immediateSegmentLoading() + .dynamicConfig(dynamicConfig) + .build(); + + // Put the first replica of all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); + + startSimulation(sim); + runCoordinatorCycle(); + // Verify that number of replicas assigned is higher than replicationThrottleLimit + verifyLatestMetricValue("segment/assigned/count", 10L); + + syncInventoryView(); + final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); + final DruidServer historicalViewT12 = getInventoryView(historicalT12.getName()); + Assert.assertEquals(10, historicalViewT11.getTotalSegments()); + Assert.assertEquals(10, historicalViewT12.getTotalSegments()); + verifyDatasourceIsFullyLoaded(DS.WIKI); + } + + @Test + public void testHistoricalsAreNotOverAssigned() + { + // some stuff is assigned + // immediate loading + // historical is now full + // but we keep adding stuff to it + // verify that we assign too many + // and that historical size was much less } @Test @@ -53,7 +203,11 @@ public void testDropHappensAfterAllPrimaryLoads() @Test public void testLoadingOfFullyReplicatedSegment() { - + // a couple of segments + // all of them are fully loaded + // something will be chosen for balancing + // verify that the segments in the load queue do count towards over-replication + // this will require 2 coordinator runs } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java index d0c319664058..4e826dad5b26 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java @@ -24,7 +24,6 @@ import org.apache.druid.client.DataSourcesSnapshot; import org.apache.druid.client.ImmutableDruidDataSource; import org.apache.druid.metadata.SegmentsMetadataManager; -import org.apache.druid.metadata.UnknownSegmentIdsException; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.Partitions; import org.apache.druid.timeline.SegmentId; @@ -88,7 +87,6 @@ public int markAsUsedNonOvershadowedSegmentsInInterval(String dataSource, Interv @Override public int markAsUsedNonOvershadowedSegments(String dataSource, Set segmentIds) - throws UnknownSegmentIdsException { return 0; } @@ -96,7 +94,6 @@ public int markAsUsedNonOvershadowedSegments(String dataSource, Set segm @Override public boolean markSegmentAsUsed(String segmentId) { - // TODO return false; } @@ -115,8 +112,13 @@ public int markAsUnusedSegmentsInInterval(String dataSource, Interval interval) @Override public int markSegmentsAsUnused(Set segmentIds) { - segmentIds.forEach(usedSegments::remove); - return 0; + int numModifiedSegments = 0; + for (SegmentId segmentId : segmentIds) { + if (usedSegments.remove(segmentId) != null) { + ++numModifiedSegments; + } + } + return numModifiedSegments; } @Override diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java index bf9ccfe2c28a..fedafc45c9d6 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestServerInventoryView.java @@ -37,7 +37,7 @@ public class TestServerInventoryView implements ServerInventoryView { - private static final Logger LOG = new Logger(TestServerInventoryView.class); + private static final Logger log = new Logger(TestServerInventoryView.class); private final ConcurrentHashMap servers = new ConcurrentHashMap<>(); private final ConcurrentHashMap segmentChangeHandlers = new ConcurrentHashMap<>(); @@ -156,7 +156,7 @@ public void addSegment( @Nullable DataSegmentChangeCallback callback ) { - LOG.info("Adding segment [%s] to server [%s]", segment.getId(), server.getName()); + log.debug("Adding segment [%s] to server [%s]", segment.getId(), server.getName()); if (server.getMaxSize() - server.getCurrSize() >= segment.getSize()) { server.addDataSegment(segment); @@ -181,7 +181,7 @@ public void removeSegment( @Nullable DataSegmentChangeCallback callback ) { - LOG.info("Removing segment [%s] from server [%s]", segment.getId(), server.getName()); + log.debug("Removing segment [%s] from server [%s]", segment.getId(), server.getName()); server.removeDataSegment(segment.getId()); segmentCallbacks.forEach( (segmentCallback, executor) -> executor.execute( diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java new file mode 100644 index 000000000000..334651ee30f5 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.java.util.common.logger.Logger; + +import java.util.Collection; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Delayed; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +/** + * Wraps an {@link ExecutorService} into a {@link ScheduledExecutorService}. + */ +public class WrappingScheduledExecutorService implements ScheduledExecutorService +{ + private static final Logger log = new Logger(WrappingScheduledExecutorService.class); + + private final String nameFormat; + private final ExecutorService delegate; + private final boolean ignoreScheduledTasks; + + public WrappingScheduledExecutorService( + String nameFormat, + ExecutorService delegate, + boolean ignoreScheduledTasks + ) + { + this.nameFormat = nameFormat; + this.delegate = delegate; + this.ignoreScheduledTasks = ignoreScheduledTasks; + } + + @Override + public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) + { + if (ignoreScheduledTasks) { + log.debug("[%s] Ignoring scheduled task", nameFormat); + return new WrappingScheduledFuture<>(CompletableFuture.completedFuture(null)); + } + + // Ignore the delay and just queue the task + return new WrappingScheduledFuture<>(submit(command)); + } + + @Override + public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) + { + if (ignoreScheduledTasks) { + log.debug("[%s] Ignoring scheduled task", nameFormat); + return new WrappingScheduledFuture<>(CompletableFuture.completedFuture(null)); + } + + // Ignore the delay and just queue the task + return new WrappingScheduledFuture<>(submit(callable)); + } + + @Override + public ScheduledFuture scheduleAtFixedRate( + Runnable command, + long initialDelay, + long period, + TimeUnit unit + ) + { + throw new UnsupportedOperationException(); + } + + @Override + public ScheduledFuture scheduleWithFixedDelay( + Runnable command, + long initialDelay, + long delay, + TimeUnit unit + ) + { + throw new UnsupportedOperationException(); + } + + @Override + public void shutdown() + { + delegate.shutdown(); + } + + @Override + public List shutdownNow() + { + return delegate.shutdownNow(); + } + + @Override + public boolean isShutdown() + { + return delegate.isShutdown(); + } + + @Override + public boolean isTerminated() + { + return delegate.isTerminated(); + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException + { + return delegate.awaitTermination(timeout, unit); + } + + @Override + public Future submit(Callable task) + { + return delegate.submit(task); + } + + @Override + public Future submit(Runnable task, T result) + { + return delegate.submit(task, result); + } + + @Override + public Future submit(Runnable task) + { + return delegate.submit(task); + } + + @Override + public List> invokeAll(Collection> tasks) throws InterruptedException + { + return delegate.invokeAll(tasks); + } + + @Override + public List> invokeAll( + Collection> tasks, long timeout, TimeUnit unit + ) throws InterruptedException + { + return delegate.invokeAll(tasks, timeout, unit); + } + + @Override + public T invokeAny(Collection> tasks) throws InterruptedException, ExecutionException + { + return delegate.invokeAny(tasks); + } + + @Override + public T invokeAny(Collection> tasks, long timeout, TimeUnit unit) + throws InterruptedException, ExecutionException, TimeoutException + { + return delegate.invokeAny(tasks, timeout, unit); + } + + @Override + public void execute(Runnable command) + { + delegate.execute(command); + } + + /** + * Wraps a Future into a ScheduledFuture. + */ + private static class WrappingScheduledFuture implements ScheduledFuture + { + private final Future future; + + private WrappingScheduledFuture(Future future) + { + this.future = future; + } + + @Override + public long getDelay(TimeUnit unit) + { + return 0; + } + + @Override + public int compareTo(Delayed o) + { + return 0; + } + + @Override + public boolean cancel(boolean mayInterruptIfRunning) + { + return future.cancel(mayInterruptIfRunning); + } + + @Override + public boolean isCancelled() + { + return future.isCancelled(); + } + + @Override + public boolean isDone() + { + return future.isDone(); + } + + @Override + public V get() throws InterruptedException, ExecutionException + { + return future.get(); + } + + @Override + public V get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException + { + return future.get(timeout, unit); + } + } +} From b9db352a0d008bd5e8e80976ab7602898444d803 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 14 Sep 2022 20:45:38 +0530 Subject: [PATCH 04/10] Add option to auto-sync inventory --- .../coordinator/CreateDataSegments.java | 16 +- .../server/coordinator/RunRulesTest.java | 2 +- .../simulate/BlockingExecutorService.java | 2 +- .../simulate/CoordinatorSimulation.java | 6 + .../CoordinatorSimulationBaseTest.java | 118 +++++--- .../CoordinatorSimulationBuilder.java | 128 +++++--- .../simulate/SegmentBalancingTest.java | 126 ++++---- .../simulate/SegmentLoadingTest.java | 280 +++++++++++++----- 8 files changed, 453 insertions(+), 225 deletions(-) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java index 7663180c9a42..6493aa3f690b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java @@ -40,7 +40,7 @@ public class CreateDataSegments private DateTime startTime; private Granularity granularity; - private int numPartitionsPerInterval; + private int numPartitions; private int numIntervals; public static CreateDataSegments ofDatasource(String datasource) @@ -60,15 +60,15 @@ public CreateDataSegments forIntervals(int numIntervals, Granularity intervalSiz return this; } - public CreateDataSegments andPartitionsPerInterval(int numPartitionsPerInterval) + public CreateDataSegments startingAt(String startOfFirstInterval) { - this.numPartitionsPerInterval = numPartitionsPerInterval; + this.startTime = DateTimes.of(startOfFirstInterval); return this; } - public CreateDataSegments startingAt(String startOfFirstInterval) + public CreateDataSegments withNumPartitions(int numPartitions) { - this.startTime = DateTimes.of(startOfFirstInterval); + this.numPartitions = numPartitions; return this; } @@ -80,12 +80,12 @@ public List eachOfSizeMb(long sizeMb) DateTime nextStart = startTime; for (int numInterval = 0; numInterval < numIntervals; ++numInterval) { Interval nextInterval = new Interval(nextStart, granularity.increment(nextStart)); - for (int numPartition = 0; numPartition < numPartitionsPerInterval; ++numPartition) { + for (int numPartition = 0; numPartition < numPartitions; ++numPartition) { segments.add( new NumberedDataSegment( datasource, nextInterval, - new NumberedShardSpec(numPartition, numPartitionsPerInterval), + new NumberedShardSpec(numPartition, numPartitions), ++uniqueIdInInterval, sizeMb ) @@ -94,7 +94,7 @@ public List eachOfSizeMb(long sizeMb) nextStart = granularity.increment(nextStart); } - return segments; + return Collections.unmodifiableList(segments); } /** diff --git a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java index 1184cb0660e1..ff373254bed0 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java @@ -83,7 +83,7 @@ public void setUp() usedSegments = CreateDataSegments.ofDatasource("test") .forIntervals(24, Granularities.HOUR) .startingAt("2012-01-01") - .andPartitionsPerInterval(1) + .withNumPartitions(1) .eachOfSizeMb(1); ruleRunner = new RunRules(new ReplicationThrottler(24, 1, false), coordinator); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java index efb664eba603..ebd47ccb23e9 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java @@ -153,7 +153,7 @@ private Future addTaskToQueue(Callable callable) @Override public void shutdown() { - + taskQueue.clear(); } @Override diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java index c497258cdfdc..447ec0d3e61a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java @@ -21,6 +21,7 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.emitter.core.Event; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; import java.util.List; @@ -64,6 +65,11 @@ interface CoordinatorState */ void syncInventoryView(); + /** + * Sets the CoordinatorDynamicConfig. + */ + void setDynamicConfig(CoordinatorDynamicConfig dynamicConfig); + /** * Gets the inventory view of the specified server as maintained by the * coordinator. diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java index 3f189017db91..dd2071cdebf4 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -21,7 +21,6 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.granularity.Granularities; -import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.emitter.core.Event; import org.apache.druid.server.coordination.ServerType; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; @@ -34,11 +33,9 @@ import org.junit.Before; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** * Base test for coordinator simulations. @@ -102,6 +99,12 @@ public void syncInventoryView() sim.coordinator().syncInventoryView(); } + @Override + public void setDynamicConfig(CoordinatorDynamicConfig dynamicConfig) + { + sim.coordinator().setDynamicConfig(dynamicConfig); + } + @Override public void loadQueuedSegments() { @@ -120,22 +123,55 @@ void verifyDatasourceIsFullyLoaded(String datasource) Assert.assertEquals(100.0, getLoadPercentage(datasource), DOUBLE_DELTA); } + void verifyNoMetricEvent(String metricName) + { + Assert.assertTrue(getMetricValues(metricName, null).isEmpty()); + } + void verifyLatestMetricValue(String metricName, Number expectedValue) { - final List observedValues = getMetricValues(metricName); - Number latestValue = observedValues.get(observedValues.size() - 1); - Assert.assertEquals(expectedValue, latestValue); + verifyLatestMetricValue(metricName, null, expectedValue); + } + + void verifyLatestMetricValue(String metricName, Map dimensionFilters, Number expectedValue) + { + Assert.assertEquals(expectedValue, getLatestMetricValue(metricName, dimensionFilters)); + } + + Number getLatestMetricValue(String metricName) + { + return getLatestMetricValue(metricName, null); + } + + Number getLatestMetricValue(String metricName, Map dimensionFilters) + { + List values = getMetricValues(metricName, dimensionFilters); + Assert.assertEquals( + "Metric must have been emitted exactly once for the given dimensions.", + 1, + values.size() + ); + return values.get(0); } - private List getMetricValues(String metricName) + private List getMetricValues(String metricName, Map dimensionFilters) { + dimensionFilters = dimensionFilters == null ? new HashMap<>() : dimensionFilters; final List metricValues = new ArrayList<>(); for (Event event : sim.coordinator().getMetricEvents()) { - final Map map = event.toMap(); - final String eventMetricName = (String) map.get("metric"); - if (eventMetricName != null && eventMetricName.equals(metricName)) { - metricValues.add((Number) map.get("value")); + final Map eventMap = event.toMap(); + + boolean match = metricName.equals(eventMap.get("metric")); + for (String dimension : dimensionFilters.keySet()) { + if (!match) { + break; + } + match = dimensionFilters.get(dimension).equals(eventMap.get(dimension)); + } + + if (match) { + metricValues.add((Number) eventMap.get("value")); } } @@ -143,19 +179,6 @@ private List getMetricValues(String metricName) } // Utility methods - static List createWikiSegmentsForIntervals( - int numIntervals, - Granularity granularity, - int partitionsPerInterval - ) - { - return CreateDataSegments.ofDatasource(DS.WIKI) - .forIntervals(1, Granularities.DAY) - .startingAt("2022-01-01") - .andPartitionsPerInterval(10) - .eachOfSizeMb(500); - } - static CoordinatorDynamicConfig createDynamicConfig( int maxSegmentsToMove, int maxSegmentsInNodeLoadingQueue, @@ -171,19 +194,25 @@ static CoordinatorDynamicConfig createDynamicConfig( } /** - * Creates a tier of historicals. + * Creates a map containing dimension key-values to filter out metric events. */ - static List createHistoricalTier(String tier, int numServers, long serverSizeMb) + static Map filter(String... dimensionValues) { - List servers = new ArrayList<>(); - for (int i = 0; i < numServers; ++i) { - servers.add(createHistorical(i, tier, serverSizeMb)); + if (dimensionValues.length < 2 || dimensionValues.length % 2 == 1) { + throw new IllegalArgumentException("Dimension key-values must be specified in pairs."); + } + + final Map filters = new HashMap<>(); + for (int i = 0; i < dimensionValues.length; ) { + filters.put(dimensionValues[i], dimensionValues[i + 1]); + i += 2; } - return servers; + return filters; } /** - * Creates a historical. + * Creates a historical. The {@code uniqueIdInTier} must be correctly specified + * as it is used to identify the historical throughout the simulation. */ static DruidServer createHistorical(int uniqueIdInTier, String tier, long serverSizeMb) { @@ -202,22 +231,35 @@ static class Tier { static final String T1 = "tier_t1"; static final String T2 = "tier_t2"; + static final String T3 = "tier_t3"; } - /** - * Builder for retention rules. - * - * @see Load - */ - interface RuleBuilder + static class Metric { + static final String ASSIGNED_COUNT = "segment/assigned/count"; + static final String MOVED_COUNT = "segment/moved/count"; + static final String DROPPED_COUNT = "segment/dropped/count"; + static final String LOAD_QUEUE_COUNT = "segment/loadQueue/count"; + } + static class Segments + { + /** + * Segments of datasource {@link DS#WIKI}, size 500 MB each, + * spanning 1 day containing 10 partitions. + */ + static final List WIKI_10X1D = + CreateDataSegments.ofDatasource(DS.WIKI) + .forIntervals(1, Granularities.DAY) + .startingAt("2022-01-01") + .withNumPartitions(10) + .eachOfSizeMb(500); } /** * Builder for a load rule. */ - static class Load implements RuleBuilder + static class Load { private final Map tieredReplicants = new HashMap<>(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java index 746eeb9caca8..c815e08e925b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java @@ -40,6 +40,7 @@ import org.apache.druid.server.coordinator.CachingCostBalancerStrategyFactory; import org.apache.druid.server.coordinator.CoordinatorCompactionConfig; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; +import org.apache.druid.server.coordinator.CostBalancerStrategyFactory; import org.apache.druid.server.coordinator.DruidCoordinator; import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.LoadQueueTaskMaster; @@ -84,8 +85,14 @@ public class CoordinatorSimulationBuilder private List servers; private List segments; private final Map> datasourceRules = new HashMap<>(); - private boolean immediateLoading; + private boolean loadImmediately = false; + private boolean autoSyncInventory = true; + /** + * Specifies the balancer strategy to be used. + *

    + * Default: "cost" ({@link CostBalancerStrategyFactory}) + */ public CoordinatorSimulationBuilder balancer(BalancerStrategyFactory strategyFactory) { this.balancerStrategyFactory = strategyFactory; @@ -116,18 +123,32 @@ public CoordinatorSimulationBuilder rules(String datasource, Rule... rules) } /** - * Causes segments to be loaded as soon as they are queued. + * Specifies whether segments should be loaded as soon as they are queued. + *

    + * Default: false */ - public CoordinatorSimulationBuilder immediateSegmentLoading() + public CoordinatorSimulationBuilder loadSegmentsImmediately(boolean loadImmediately) { - this.immediateLoading = true; + this.loadImmediately = loadImmediately; + return this; + } + + /** + * Specifies whether the inventory view maintained by the coordinator + * should be auto-synced as soon as any change is made to the cluster. + *

    + * Default: true + */ + public CoordinatorSimulationBuilder autoSyncInventory(boolean autoSync) + { + this.autoSyncInventory = autoSync; return this; } /** * Specifies the CoordinatorDynamicConfig to be used in the simulation. *

    - * If not specified, the default values are used. + * Default values: Specified in {@link CoordinatorDynamicConfig.Builder}. */ public CoordinatorSimulationBuilder dynamicConfig(CoordinatorDynamicConfig dynamicConfig) { @@ -142,16 +163,6 @@ public CoordinatorSimulation build() "Cannot run simulation for an empty cluster" ); - // Prepare the config - final DruidCoordinatorConfig coordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withCoordinatorStartDelay(new Duration(1L)) - .withCoordinatorPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) - .withCoordinatorKillPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) - .withLoadQueuePeonRepeatDelay(new Duration("PT0S")) - .withLoadQueuePeonType("http") - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - // Prepare the environment final TestServerInventoryView serverInventoryView = new TestServerInventoryView(); servers.forEach(serverInventoryView::addServer); @@ -168,13 +179,12 @@ public CoordinatorSimulation build() ); final Environment env = new Environment( - new TestDruidLeaderSelector(), serverInventoryView, segmentManager, ruleManager, - coordinatorConfig, dynamicConfig, - immediateLoading + loadImmediately, + autoSyncInventory ); // Build the coordinator @@ -183,7 +193,7 @@ public CoordinatorSimulation build() null, env.jacksonConfigManager, env.segmentManager, - env.historicalInventoryView, + env.coordinatorInventoryView, env.ruleManager, () -> null, env.serviceEmitter, @@ -196,7 +206,7 @@ public CoordinatorSimulation build() null, new CoordinatorCustomDutyGroups(Collections.emptySet()), balancerStrategyFactory != null ? balancerStrategyFactory - : buildCachingCostBalancerStrategy(env), + : new CostBalancerStrategyFactory(), env.lookupCoordinatorManager, env.leaderSelector, OBJECT_MAPPER, @@ -278,16 +288,28 @@ public void runCoordinatorCycle() { verifySimulationRunning(); env.serviceEmitter.flush(); - env.executorFactory.coordinatorRunner.finishNextPendingTasks(1); + + // Invoke historical duties and metadata duties + env.executorFactory.coordinatorRunner.finishNextPendingTasks(2); } @Override public void syncInventoryView() { verifySimulationRunning(); + Preconditions.checkState( + !env.autoSyncInventory, + "Cannot invoke syncInventoryView as simulation is running in auto-sync mode." + ); env.coordinatorInventoryView.sync(env.historicalInventoryView); } + @Override + public void setDynamicConfig(CoordinatorDynamicConfig dynamicConfig) + { + env.setDynamicConfig(dynamicConfig); + } + @Override public DruidServer getInventoryView(String serverName) { @@ -299,7 +321,7 @@ public void loadQueuedSegments() { verifySimulationRunning(); Preconditions.checkState( - !env.immediateLoading, + !env.loadImmediately, "Cannot invoke loadQueuedSegments as simulation is running in immediate loading mode." ); @@ -312,6 +334,9 @@ public void loadQueuedSegments() // Load all the queued segments, handle their responses and execute callbacks int loadedSegments = env.executorFactory.historicalLoader.finishAllPendingTasks(); loadQueueExecutor.finishNextPendingTasks(loadedSegments); + + // TODO: sync should happen here?? or should it not?? + env.executorFactory.loadCallbackExecutor.finishAllPendingTasks(); } } @@ -346,7 +371,7 @@ private static class Environment // Executors private final ExecutorFactory executorFactory; - private final TestDruidLeaderSelector leaderSelector; + private final TestDruidLeaderSelector leaderSelector = new TestDruidLeaderSelector(); private final TestSegmentsMetadataManager segmentManager; private final TestMetadataRuleManager ruleManager; private final TestServerInventoryView historicalInventoryView; @@ -356,35 +381,46 @@ private static class Environment = new StubServiceEmitter("coordinator", "coordinator"); private final TestServerInventoryView coordinatorInventoryView; + private final AtomicReference dynamicConfig = new AtomicReference<>(); private final JacksonConfigManager jacksonConfigManager; private final LookupCoordinatorManager lookupCoordinatorManager; private final DruidCoordinatorConfig coordinatorConfig; - private final boolean immediateLoading; + private final boolean loadImmediately; + private final boolean autoSyncInventory; private final List mocks = new ArrayList<>(); private Environment( - TestDruidLeaderSelector leaderSelector, - TestServerInventoryView historicalInventoryView, + TestServerInventoryView clusterInventory, TestSegmentsMetadataManager segmentManager, TestMetadataRuleManager ruleManager, - DruidCoordinatorConfig coordinatorConfig, CoordinatorDynamicConfig dynamicConfig, - boolean immediateLoading + boolean loadImmediately, + boolean autoSyncInventory ) { - this.leaderSelector = leaderSelector; - this.historicalInventoryView = historicalInventoryView; + this.historicalInventoryView = clusterInventory; this.segmentManager = segmentManager; this.ruleManager = ruleManager; - this.coordinatorConfig = coordinatorConfig; - this.immediateLoading = immediateLoading; - - this.executorFactory = new ExecutorFactory(immediateLoading); - this.coordinatorInventoryView = new TestServerInventoryView(); + this.loadImmediately = loadImmediately; + this.autoSyncInventory = autoSyncInventory; + + this.coordinatorConfig = new TestDruidCoordinatorConfig.Builder() + .withCoordinatorStartDelay(new Duration(1L)) + .withCoordinatorPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) + .withCoordinatorKillPeriod(new Duration(DEFAULT_COORDINATOR_PERIOD)) + .withLoadQueuePeonRepeatDelay(new Duration("PT0S")) + .withLoadQueuePeonType("http") + .withCoordinatorKillIgnoreDurationToRetain(false) + .build(); + + this.executorFactory = new ExecutorFactory(loadImmediately); + this.coordinatorInventoryView = autoSyncInventory + ? clusterInventory + : new TestServerInventoryView(); HttpClient httpClient = new TestSegmentLoadingHttpClient( OBJECT_MAPPER, - historicalInventoryView::getChangeHandlerForHost, + clusterInventory::getChangeHandlerForHost, executorFactory.create(1, ExecutorFactory.HISTORICAL_LOADER) ); @@ -398,7 +434,9 @@ private Environment( null ); - this.jacksonConfigManager = mockConfigManager(dynamicConfig); + this.jacksonConfigManager = mockConfigManager(); + setDynamicConfig(dynamicConfig); + this.lookupCoordinatorManager = EasyMock.createNiceMock(LookupCoordinatorManager.class); mocks.add(jacksonConfigManager); mocks.add(lookupCoordinatorManager); @@ -408,7 +446,6 @@ private void setUp() throws Exception { EmittingLogger.registerEmitter(serviceEmitter); historicalInventoryView.setUp(); - coordinatorInventoryView.sync(historicalInventoryView); coordinatorInventoryView.setUp(); lifecycle.start(); executorFactory.setUp(); @@ -419,10 +456,16 @@ private void setUp() throws Exception private void tearDown() { EasyMock.verify(mocks.toArray()); + executorFactory.tearDown(); lifecycle.stop(); } - private JacksonConfigManager mockConfigManager(CoordinatorDynamicConfig dynamicConfig) + private void setDynamicConfig(CoordinatorDynamicConfig dynamicConfig) + { + this.dynamicConfig.set(dynamicConfig); + } + + private JacksonConfigManager mockConfigManager() { final JacksonConfigManager jacksonConfigManager = EasyMock.createMock(JacksonConfigManager.class); @@ -432,7 +475,7 @@ private JacksonConfigManager mockConfigManager(CoordinatorDynamicConfig dynamicC EasyMock.eq(CoordinatorDynamicConfig.class), EasyMock.anyObject() ) - ).andReturn(new AtomicReference<>(dynamicConfig)).anyTimes(); + ).andReturn(dynamicConfig).anyTimes(); EasyMock.expect( jacksonConfigManager.watch( @@ -496,6 +539,11 @@ private void setUp() loadQueueExecutor = findExecutor(LOAD_QUEUE_EXECUTOR); loadCallbackExecutor = findExecutor(LOAD_CALLBACK_EXECUTOR); } + + private void tearDown() + { + blockingExecutors.values().forEach(BlockingExecutorService::shutdown); + } } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java index ad0a025bb804..1b1da0b3431f 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java @@ -20,10 +20,8 @@ package org.apache.druid.server.coordinator.simulate; import org.apache.druid.client.DruidServer; -import org.apache.druid.java.util.common.granularity.Granularities; -import org.apache.druid.server.coordinator.CostBalancerStrategyFactory; -import org.apache.druid.server.coordinator.CreateDataSegments; -import org.apache.druid.server.coordinator.rules.Rule; +import org.apache.druid.query.DruidMetrics; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; import org.apache.druid.timeline.DataSegment; import org.junit.Assert; import org.junit.Ignore; @@ -36,95 +34,113 @@ */ public class SegmentBalancingTest extends CoordinatorSimulationBaseTest { - // TODO: randomness and flakiness can creep in if the maxSegmentsToMove < totalLoadedSegments + private DruidServer historicalT11; + private DruidServer historicalT12; + + private final String datasource = DS.WIKI; + private final List segments = Segments.WIKI_10X1D; + @Override public void setUp() { - + // Setup historicals for 2 tiers, size 10 GB each + historicalT11 = createHistorical(1, Tier.T1, 10_000); + historicalT12 = createHistorical(2, Tier.T1, 10_000); } @Test - public void testBalancingWithUpdatedInventory() + public void testBalancingWithSyncedInventory() { - testBalancingWhenInventoryIsSynced(true); + testBalancingWithInventorySynced(true); } @Test @Ignore("Fix #12881 to enable this test. " + "Current impl requires updated inventory for correct callback behaviour.") - public void testBalancingWithStaleInventory() + public void testBalancingWithUnsyncedInventory() { - testBalancingWhenInventoryIsSynced(false); + testBalancingWithInventorySynced(false); } - private void testBalancingWhenInventoryIsSynced(boolean syncInventory) + private void testBalancingWithInventorySynced(boolean autoSyncInventory) { - // historicals = 2(T1), segments = 10(1 day) - final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); - final DruidServer historicalT12 = createHistorical(2, Tier.T1, 10_000); - final List segments = - CreateDataSegments.ofDatasource(DS.WIKI) - .forIntervals(1, Granularities.DAY) - .startingAt("2022-01-01") - .andPartitionsPerInterval(10) - .eachOfSizeMb(500); - - - // strategy = cost, replicas = 1(T1) + // maxSegmentsToMove = 10, unlimited load queue, replicationThrottleLimit = 10 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(10, 0, 10); + + // historicals = 2(T1), replicas = 1(T1) final CoordinatorSimulation sim = CoordinatorSimulation.builder() .segments(segments) .servers(historicalT11, historicalT12) - .rules(DS.WIKI, Load.on(Tier.T1, 1).forever()) - .balancer(new CostBalancerStrategyFactory()) + .rules(datasource, Load.on(Tier.T1, 1).forever()) + .dynamicConfig(dynamicConfig) + .autoSyncInventory(autoSyncInventory) .build(); // Put all the segments on histT11 segments.forEach(historicalT11::addDataSegment); startSimulation(sim); + if (!autoSyncInventory) { + syncInventoryView(); + } + runCoordinatorCycle(); // Verify that segments have been chosen for balancing - verifyLatestMetricValue("segment/moved/count", 5L); + verifyLatestMetricValue(Metric.MOVED_COUNT, 5L); loadQueuedSegments(); - if (syncInventory) { - syncInventoryView(); - } // Verify that segments have now been balanced out - final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); - final DruidServer historicalViewT12 = getInventoryView(historicalT12.getName()); - Assert.assertEquals(5, historicalViewT11.getTotalSegments()); - Assert.assertEquals(5, historicalViewT12.getTotalSegments()); - verifyDatasourceIsFullyLoaded(DS.WIKI); + Assert.assertEquals(5, historicalT11.getTotalSegments()); + Assert.assertEquals(5, historicalT12.getTotalSegments()); + verifyDatasourceIsFullyLoaded(datasource); } @Test - public void testBalancingSkipsUnusedSegments() + public void testBalancingOfFullyReplicatedSegment() { - // have a bunch of segments - // mark them all as unused - // verify that nothing is chosen for balancing - } + // maxSegmentsToMove = 10, unlimited load queue, replicationThrottleLimit = 10 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(10, 0, 10); - @Test - public void testBalancingSkipsOvershadowedSegments() - { - // have a bunch of segments - // overshadow all but one - // verify that none of the overshadowed ones are chosen for balancing - } + // historicals = 2(in T1), replicas = 1(T1) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .segments(segments) + .servers(historicalT11, historicalT12) + .dynamicConfig(dynamicConfig) + .rules(datasource, Load.on(Tier.T1, 1).forever()) + .build(); - @Test - public void testBalancingOfFullyReplicatedSegment() - { - // a couple of segments - // all of them are fully loaded - // something will be chosen for balancing - // verify that the segments on the move don't count towards over-replication - // this will require 2 coordinator runs - } + // Put all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); + startSimulation(sim); + runCoordinatorCycle(); + + // Verify that there are segments in the load queue for balancing + verifyLatestMetricValue(Metric.MOVED_COUNT, 5L); + verifyLatestMetricValue( + Metric.LOAD_QUEUE_COUNT, + filter(DruidMetrics.SERVER, historicalT12.getName()), + 5 + ); + + runCoordinatorCycle(); + + // Verify that the segments in the load queue are not considered as over-replicated + verifyLatestMetricValue("segment/dropped/count", 0L); + verifyLatestMetricValue( + Metric.LOAD_QUEUE_COUNT, + filter(DruidMetrics.SERVER, historicalT12.getName()), + 5 + ); + + // Finish and verify balancing + loadQueuedSegments(); + Assert.assertEquals(5, historicalT11.getTotalSegments()); + Assert.assertEquals(5, historicalT12.getTotalSegments()); + verifyDatasourceIsFullyLoaded(datasource); + } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java index 09bbcd4c089a..81f3ce2c8b2a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java @@ -20,11 +20,8 @@ package org.apache.druid.server.coordinator.simulate; import org.apache.druid.client.DruidServer; -import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.query.DruidMetrics; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; -import org.apache.druid.server.coordinator.CostBalancerStrategyFactory; -import org.apache.druid.server.coordinator.CreateDataSegments; -import org.apache.druid.server.coordinator.rules.Rule; import org.apache.druid.timeline.DataSegment; import org.junit.Assert; import org.junit.Ignore; @@ -37,10 +34,23 @@ */ public class SegmentLoadingTest extends CoordinatorSimulationBaseTest { + private DruidServer historicalT11; + private DruidServer historicalT12; + private DruidServer historicalT21; + private DruidServer historicalT22; + + private final String datasource = DS.WIKI; + private final List segments = Segments.WIKI_10X1D; + @Override public void setUp() { + // Setup historicals for 2 tiers, size 10 GB each + historicalT11 = createHistorical(1, Tier.T1, 10_000); + historicalT12 = createHistorical(2, Tier.T1, 10_000); + historicalT21 = createHistorical(1, Tier.T2, 10_000); + historicalT22 = createHistorical(2, Tier.T2, 10_000); } @Test @@ -48,28 +58,18 @@ public void setUp() + "Current behaviour is to throttle globally and not per tier.") public void testFirstReplicaOnAnyTierIsNotThrottled() { - // historicals = 1(in T1) + 1(in T2), segments = 10*1day - final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); - final DruidServer historicalT21 = createHistorical(1, Tier.T2, 10_000); - final List segments = - CreateDataSegments.ofDatasource(DS.WIKI) - .forIntervals(1, Granularities.DAY) - .startingAt("2022-01-01") - .andPartitionsPerInterval(10) - .eachOfSizeMb(500); - // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); - // strategy = cost, replicas = 1(on T1) + 1(on T2) + // historicals = 1(in T1) + 1(in T2) + // replicas = 1(on T1) + 1(on T2) final CoordinatorSimulation sim = CoordinatorSimulation.builder() .segments(segments) .servers(historicalT11, historicalT21) .dynamicConfig(dynamicConfig) - .balancer(new CostBalancerStrategyFactory()) .rules( - DS.WIKI, + datasource, Load.on(Tier.T1, 1).andOn(Tier.T2, 1).forever() ) .build(); @@ -80,43 +80,34 @@ public void testFirstReplicaOnAnyTierIsNotThrottled() startSimulation(sim); runCoordinatorCycle(); - // Verify that all the missing replicas for T2 have been assigned - verifyLatestMetricValue("segment/assigned/count", 10L); + // Verify that all the required replicas for T2 have been assigned + verifyLatestMetricValue( + Metric.ASSIGNED_COUNT, + filter(DruidMetrics.TIER, Tier.T2), + 10L + ); loadQueuedSegments(); - syncInventoryView(); - final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); - final DruidServer historicalViewT21 = getInventoryView(historicalT21.getName()); - Assert.assertEquals(10, historicalViewT11.getTotalSegments()); - Assert.assertEquals(10, historicalViewT21.getTotalSegments()); - verifyDatasourceIsFullyLoaded(DS.WIKI); + verifyDatasourceIsFullyLoaded(datasource); + Assert.assertEquals(10, historicalT11.getTotalSegments()); + Assert.assertEquals(10, historicalT21.getTotalSegments()); } @Test public void testSecondReplicaOnAnyTierIsThrottled() { - // historicals = 2(in T1), segments = 10*1day - final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); - final DruidServer historicalT12 = createHistorical(2, Tier.T1, 10_000); - final List segments = - CreateDataSegments.ofDatasource(DS.WIKI) - .forIntervals(1, Granularities.DAY) - .startingAt("2022-01-01") - .andPartitionsPerInterval(10) - .eachOfSizeMb(500); - // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); - // strategy = cost, replicas = 2(on T1) + // historicals = 2(in T1) + // replicas = 2(on T1) final CoordinatorSimulation sim = CoordinatorSimulation.builder() .segments(segments) .servers(historicalT11, historicalT12) - .rules(DS.WIKI, Load.on(Tier.T1, 2).forever()) + .rules(datasource, Load.on(Tier.T1, 2).forever()) .dynamicConfig(dynamicConfig) - .balancer(new CostBalancerStrategyFactory()) .build(); // Put the first replica of all the segments on histT11 @@ -126,43 +117,30 @@ public void testSecondReplicaOnAnyTierIsThrottled() runCoordinatorCycle(); // Verify that that replicationThrottleLimit is honored - verifyLatestMetricValue("segment/assigned/count", 2L); + verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 2L); loadQueuedSegments(); - syncInventoryView(); - final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); - final DruidServer historicalViewT12 = getInventoryView(historicalT12.getName()); - Assert.assertEquals(10, historicalViewT11.getTotalSegments()); - Assert.assertEquals(2, historicalViewT12.getTotalSegments()); + Assert.assertEquals(10, historicalT11.getTotalSegments()); + Assert.assertEquals(2, historicalT12.getTotalSegments()); } @Test @Ignore("Fix #12881 to enable this test. " + "Current impl allows throttle limit to be violated if loading is fast.") - public void testReplicationThrottleWithImmediateLoading() + public void testImmediateLoadingDoesNotViolateThrottleLimit() { - // historicals = 2(in T1), segments = 10*1day - final DruidServer historicalT11 = createHistorical(1, Tier.T1, 10_000); - final DruidServer historicalT12 = createHistorical(2, Tier.T1, 10_000); - final List segments = - CreateDataSegments.ofDatasource(DS.WIKI) - .forIntervals(1, Granularities.DAY) - .startingAt("2022-01-01") - .andPartitionsPerInterval(10) - .eachOfSizeMb(500); - - // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 + // Disable balancing, infinite load queue size, replicationThrottleLimit = 2 CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); - // strategy = cost, replicas = 2(on T1), immediate segment loading + // historicals = 2(in T1), segments = 10*1day + // replicas = 2(on T1), immediate segment loading final CoordinatorSimulation sim = CoordinatorSimulation.builder() - .balancer(new CostBalancerStrategyFactory()) .segments(segments) .servers(historicalT11, historicalT12) - .rules(DS.WIKI, Load.on(Tier.T1, 2).forever()) - .immediateSegmentLoading() + .rules(datasource, Load.on(Tier.T1, 2).forever()) + .loadSegmentsImmediately(true) .dynamicConfig(dynamicConfig) .build(); @@ -172,42 +150,180 @@ public void testReplicationThrottleWithImmediateLoading() startSimulation(sim); runCoordinatorCycle(); - // Verify that number of replicas assigned is higher than replicationThrottleLimit - verifyLatestMetricValue("segment/assigned/count", 10L); + // Verify that number of replicas assigned is equal to replicationThrottleLimit + verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 2L); - syncInventoryView(); - final DruidServer historicalViewT11 = getInventoryView(historicalT11.getName()); - final DruidServer historicalViewT12 = getInventoryView(historicalT12.getName()); - Assert.assertEquals(10, historicalViewT11.getTotalSegments()); - Assert.assertEquals(10, historicalViewT12.getTotalSegments()); - verifyDatasourceIsFullyLoaded(DS.WIKI); + Assert.assertEquals(10, historicalT11.getTotalSegments()); + Assert.assertEquals(2, historicalT12.getTotalSegments()); + verifyDatasourceIsFullyLoaded(datasource); } @Test - public void testHistoricalsAreNotOverAssigned() + public void testLoadingDoesNotOverassignHistorical() { - // some stuff is assigned - // immediate loading - // historical is now full - // but we keep adding stuff to it - // verify that we assign too many - // and that historical size was much less + testHistoricalIsNotOverAssigned(false); } @Test - public void testDropHappensAfterAllPrimaryLoads() + @Ignore("Fix #12881 to enable this test. " + + "Current impl may overassign a historical if loading is fast.") + public void testImmediateLoadingDoesNotOverassignHistorical() { + testHistoricalIsNotOverAssigned(true); + } + + private void testHistoricalIsNotOverAssigned(boolean loadImmediately) + { + // historicals = 1(in T1), size 1 GB + final DruidServer historicalT11 = createHistorical(1, Tier.T1, 1000); + + // disable balancing, unlimited load queue, replicationThrottleLimit = 10 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 10); + + // segments = 10*1day, size 500 MB + // strategy = cost, replicas = 1(T1) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .segments(segments) + .servers(historicalT11) + .dynamicConfig(dynamicConfig) + .rules(datasource, Load.on(Tier.T1, 1).forever()) + .loadSegmentsImmediately(loadImmediately) + .build(); + + startSimulation(sim); + runCoordinatorCycle(); + // Verify that the number of segments assigned is within the historical capacity + verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 2L); + if (!loadImmediately) { + loadQueuedSegments(); + } + Assert.assertEquals(2, historicalT11.getTotalSegments()); } @Test - public void testLoadingOfFullyReplicatedSegment() + public void testDropHappensAfterTargetReplicationOnEveryTier() { - // a couple of segments - // all of them are fully loaded - // something will be chosen for balancing - // verify that the segments in the load queue do count towards over-replication - // this will require 2 coordinator runs + // maxNonPrimaryReplicants = 33 ensures that all target replicas (total 4) + // are assigned for some segments in the first run itself (pigeon-hole) + CoordinatorDynamicConfig dynamicConfig = + CoordinatorDynamicConfig.builder() + .withMaxSegmentsToMove(0) + .withReplicationThrottleLimit(10) + .withMaxNonPrimaryReplicantsToLoad(33) + .build(); + + // historicals = 1(in T1) + 2(in T2) + 2(in T3) + // segments = 10 * 1day, replicas = 2(T2) + 2(T3) + final DruidServer historicalT31 = createHistorical(1, Tier.T3, 10_000); + final DruidServer historicalT32 = createHistorical(2, Tier.T3, 10_000); + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .segments(segments) + .dynamicConfig(dynamicConfig) + .rules(datasource, Load.on(Tier.T2, 2).andOn(Tier.T3, 2).forever()) + .servers( + historicalT11, + historicalT21, + historicalT22, + historicalT31, + historicalT32 + ) + .build(); + + // At the start, T1 has all the segments + segments.forEach(historicalT11::addDataSegment); + + // Run 1: Nothing is dropped from T1 but things are assigned to T2 and T3 + startSimulation(sim); + runCoordinatorCycle(); + + verifyNoMetricEvent(Metric.DROPPED_COUNT); + int totalAssignedInRun1 = + getLatestMetricValue(Metric.ASSIGNED_COUNT, filter(DruidMetrics.TIER, Tier.T2)) + .intValue() + + getLatestMetricValue(Metric.ASSIGNED_COUNT, filter(DruidMetrics.TIER, Tier.T3)) + .intValue(); + Assert.assertTrue(totalAssignedInRun1 > 0 && totalAssignedInRun1 < 40); + + // Run 2: Segments still queued, nothing is dropped from T1 + runCoordinatorCycle(); + loadQueuedSegments(); + + verifyNoMetricEvent(Metric.DROPPED_COUNT); + int totalLoadedAfterRun2 = historicalT21.getTotalSegments() + historicalT22.getTotalSegments() + + historicalT31.getTotalSegments() + historicalT32.getTotalSegments(); + Assert.assertEquals(totalAssignedInRun1, totalLoadedAfterRun2); + + // Run 3: Some segments have been loaded + // segments fully replicated on T2 and T3 will now be dropped from T1 + runCoordinatorCycle(); + loadQueuedSegments(); + + int totalDroppedInRun3 = + getLatestMetricValue(Metric.DROPPED_COUNT, filter(DruidMetrics.TIER, Tier.T1)) + .intValue(); + Assert.assertTrue(totalDroppedInRun3 > 0 && totalDroppedInRun3 < 10); + int totalLoadedAfterRun3 = historicalT21.getTotalSegments() + historicalT22.getTotalSegments() + + historicalT31.getTotalSegments() + historicalT32.getTotalSegments(); + Assert.assertEquals(40, totalLoadedAfterRun3); + + // Run 4: All segments are fully replicated on T2 and T3 + runCoordinatorCycle(); + loadQueuedSegments(); + + int totalDroppedInRun4 = + getLatestMetricValue(Metric.DROPPED_COUNT, filter(DruidMetrics.TIER, Tier.T1)) + .intValue(); + + Assert.assertEquals(10, totalDroppedInRun3 + totalDroppedInRun4); + Assert.assertEquals(0, historicalT11.getTotalSegments()); + verifyDatasourceIsFullyLoaded(datasource); + } + + @Test + @Ignore("Fix #12881 to enable this test. " + + "Current impl does not cancel load even if segment is fully replicated.") + public void testLoadOfFullyReplicatedSegmentGetsCancelled() + { + // disable balancing, unlimited load queue, replicationThrottleLimit = 10 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 10); + + // historicals = 2(in T1), replicas = 2(on T1) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .segments(segments) + .servers(historicalT11, historicalT12) + .dynamicConfig(dynamicConfig) + .rules(datasource, Load.on(Tier.T1, 2).forever()) + .build(); + + // Put the first replica of all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); + + startSimulation(sim); + runCoordinatorCycle(); + + // Verify that there are segments in the load queue + verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 10L); + verifyLatestMetricValue( + Metric.LOAD_QUEUE_COUNT, + filter(DruidMetrics.SERVER, historicalT12.getName()), + 10 + ); + + // Put the second replica of all the segments on histT12 + segments.forEach(historicalT12::addDataSegment); + + runCoordinatorCycle(); + + // Verify that the segments have been cleared from the load queue + verifyLatestMetricValue( + Metric.LOAD_QUEUE_COUNT, + filter(DruidMetrics.SERVER, historicalT12.getName()), + 0 + ); } } From b4c83ddb261ee0663abefebc327d5f0585496e78 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 14 Sep 2022 21:29:08 +0530 Subject: [PATCH 05/10] Minor cleanup --- .../coordinator/duty/BalanceSegments.java | 7 +- .../coordinator/CreateDataSegments.java | 2 +- .../simulate/CoordinatorSimulation.java | 13 ++-- .../CoordinatorSimulationBaseTest.java | 67 ++++++++++++------- .../simulate/SegmentBalancingTest.java | 10 +-- .../simulate/SegmentLoadingTest.java | 46 ++++++------- 6 files changed, 78 insertions(+), 67 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java index d08bb59568fa..198a7cf5e8f6 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java @@ -251,15 +251,14 @@ private Pair balanceServers( if (moveSegment(segmentToMoveHolder, destinationHolder.getServer(), params)) { moved++; } else { - log.info("Segment [%s] cannot be moved. Maybe no space left or already being served or already in queue.", segmentToMove.getId()); unmoved++; } } else { - log.info("Segment [%s] is 'optimally' placed.", segmentToMove.getId()); + log.debug("Segment [%s] is 'optimally' placed.", segmentToMove.getId()); unmoved++; } } else { - log.info("No valid movement destinations for segment [%s].", segmentToMove.getId()); + log.debug("No valid movement destinations for segment [%s].", segmentToMove.getId()); unmoved++; } } @@ -292,7 +291,7 @@ protected boolean moveSegment( if (!toPeon.getSegmentsToLoad().contains(segmentToMove) && (toServer.getSegment(segmentId) == null) && new ServerHolder(toServer, toPeon).getAvailableSize() > segmentToMove.getSize()) { - log.info("Moving [%s] from [%s] to [%s]", segmentId, fromServer.getName(), toServer.getName()); + log.debug("Moving [%s] from [%s] to [%s]", segmentId, fromServer.getName(), toServer.getName()); LoadPeonCallback callback = null; try { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java index 6493aa3f690b..fecc04864226 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java @@ -32,7 +32,7 @@ import java.util.List; /** - * Creates {@link DataSegment}s for a given datasource. + * Test utility to create {@link DataSegment}s for a given datasource. */ public class CreateDataSegments { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java index 447ec0d3e61a..bb2c6f990076 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java @@ -40,8 +40,14 @@ public interface CoordinatorSimulation */ void stop(); + /** + * State of the coordinator during the simulation. + */ CoordinatorState coordinator(); + /** + * State of the cluster during the simulation. + */ ClusterState cluster(); static CoordinatorSimulationBuilder builder() @@ -49,9 +55,6 @@ static CoordinatorSimulationBuilder builder() return new CoordinatorSimulationBuilder(); } - /** - * Represents the state of the coordinator during a simulation. - */ interface CoordinatorState { /** @@ -87,9 +90,6 @@ interface CoordinatorState double getLoadPercentage(String datasource); } - /** - * Represents the state of the cluster during a simulation. - */ interface ClusterState { /** @@ -98,6 +98,5 @@ interface ClusterState * callbacks on the coordinator. */ void loadQueuedSegments(); - } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java index dd2071cdebf4..2b194b241963 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -33,6 +33,7 @@ import org.junit.Before; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -56,6 +57,7 @@ public abstract class CoordinatorSimulationBaseTest static final double DOUBLE_DELTA = 10e-9; private CoordinatorSimulation sim; + private final Map> latestMetricEvents = new HashMap<>(); @Before public abstract void setUp(); @@ -78,7 +80,18 @@ void startSimulation(CoordinatorSimulation simulation) @Override public void runCoordinatorCycle() { + latestMetricEvents.clear(); sim.coordinator().runCoordinatorCycle(); + + // Extract the metric values of this run + for (Event event : sim.coordinator().getMetricEvents()) { + Map eventMap = event.toMap(); + String metricName = (String) eventMap.get("metric"); + if (metricName != null) { + latestMetricEvents.computeIfAbsent(metricName, m -> new ArrayList<>()) + .add(event); + } + } } @Override @@ -123,27 +136,33 @@ void verifyDatasourceIsFullyLoaded(String datasource) Assert.assertEquals(100.0, getLoadPercentage(datasource), DOUBLE_DELTA); } - void verifyNoMetricEvent(String metricName) + void verifyNoEvent(String metricName) { Assert.assertTrue(getMetricValues(metricName, null).isEmpty()); } - void verifyLatestMetricValue(String metricName, Number expectedValue) - { - verifyLatestMetricValue(metricName, null, expectedValue); - } - - void verifyLatestMetricValue(String metricName, Map dimensionFilters, Number expectedValue) + /** + * Verifies the value of the specified metric emitted in the previous run. + */ + void verifyValue(String metricName, Number expectedValue) { - Assert.assertEquals(expectedValue, getLatestMetricValue(metricName, dimensionFilters)); + verifyValue(metricName, null, expectedValue); } - Number getLatestMetricValue(String metricName) + /** + * Verifies the value of the event corresponding to the specified metric and + * dimensionFilters emitted in the previous run. + */ + void verifyValue(String metricName, Map dimensionFilters, Number expectedValue) { - return getLatestMetricValue(metricName, null); + Assert.assertEquals(expectedValue, getValue(metricName, dimensionFilters)); } - Number getLatestMetricValue(String metricName, Map dimensionFilters) + /** + * Gets the value of the event corresponding to the specified metric and + * dimensionFilters emitted in the previous run. + */ + Number getValue(String metricName, Map dimensionFilters) { List values = getMetricValues(metricName, dimensionFilters); Assert.assertEquals( @@ -156,26 +175,22 @@ Number getLatestMetricValue(String metricName, Map dimensionFilt private List getMetricValues(String metricName, Map dimensionFilters) { - dimensionFilters = dimensionFilters == null ? new HashMap<>() : dimensionFilters; - final List metricValues = new ArrayList<>(); - - for (Event event : sim.coordinator().getMetricEvents()) { + final List values = new ArrayList<>(); + final List events = latestMetricEvents.getOrDefault(metricName, Collections.emptyList()); + final Map filters = dimensionFilters == null + ? Collections.emptyMap() : dimensionFilters; + for (Event event : events) { final Map eventMap = event.toMap(); - - boolean match = metricName.equals(eventMap.get("metric")); - for (String dimension : dimensionFilters.keySet()) { - if (!match) { - break; - } - match = dimensionFilters.get(dimension).equals(eventMap.get(dimension)); - } - + boolean match = filters.keySet().stream() + .map(d -> filters.get(d).equals(eventMap.get(d))) + .reduce((a, b) -> a && b) + .orElse(true); if (match) { - metricValues.add((Number) eventMap.get("value")); + values.add((Number) eventMap.get("value")); } } - return metricValues; + return values; } // Utility methods diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java index 1b1da0b3431f..635760695a7c 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java @@ -88,7 +88,7 @@ private void testBalancingWithInventorySynced(boolean autoSyncInventory) runCoordinatorCycle(); // Verify that segments have been chosen for balancing - verifyLatestMetricValue(Metric.MOVED_COUNT, 5L); + verifyValue(Metric.MOVED_COUNT, 5L); loadQueuedSegments(); @@ -120,8 +120,8 @@ public void testBalancingOfFullyReplicatedSegment() runCoordinatorCycle(); // Verify that there are segments in the load queue for balancing - verifyLatestMetricValue(Metric.MOVED_COUNT, 5L); - verifyLatestMetricValue( + verifyValue(Metric.MOVED_COUNT, 5L); + verifyValue( Metric.LOAD_QUEUE_COUNT, filter(DruidMetrics.SERVER, historicalT12.getName()), 5 @@ -130,8 +130,8 @@ public void testBalancingOfFullyReplicatedSegment() runCoordinatorCycle(); // Verify that the segments in the load queue are not considered as over-replicated - verifyLatestMetricValue("segment/dropped/count", 0L); - verifyLatestMetricValue( + verifyValue("segment/dropped/count", 0L); + verifyValue( Metric.LOAD_QUEUE_COUNT, filter(DruidMetrics.SERVER, historicalT12.getName()), 5 diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java index 81f3ce2c8b2a..36e3205035b8 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java @@ -81,7 +81,7 @@ public void testFirstReplicaOnAnyTierIsNotThrottled() runCoordinatorCycle(); // Verify that all the required replicas for T2 have been assigned - verifyLatestMetricValue( + verifyValue( Metric.ASSIGNED_COUNT, filter(DruidMetrics.TIER, Tier.T2), 10L @@ -117,7 +117,7 @@ public void testSecondReplicaOnAnyTierIsThrottled() runCoordinatorCycle(); // Verify that that replicationThrottleLimit is honored - verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 2L); + verifyValue(Metric.ASSIGNED_COUNT, 2L); loadQueuedSegments(); @@ -151,7 +151,7 @@ public void testImmediateLoadingDoesNotViolateThrottleLimit() runCoordinatorCycle(); // Verify that number of replicas assigned is equal to replicationThrottleLimit - verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 2L); + verifyValue(Metric.ASSIGNED_COUNT, 2L); Assert.assertEquals(10, historicalT11.getTotalSegments()); Assert.assertEquals(2, historicalT12.getTotalSegments()); @@ -195,7 +195,7 @@ private void testHistoricalIsNotOverAssigned(boolean loadImmediately) runCoordinatorCycle(); // Verify that the number of segments assigned is within the historical capacity - verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 2L); + verifyValue(Metric.ASSIGNED_COUNT, 2L); if (!loadImmediately) { loadQueuedSegments(); } @@ -239,21 +239,20 @@ public void testDropHappensAfterTargetReplicationOnEveryTier() startSimulation(sim); runCoordinatorCycle(); - verifyNoMetricEvent(Metric.DROPPED_COUNT); - int totalAssignedInRun1 = - getLatestMetricValue(Metric.ASSIGNED_COUNT, filter(DruidMetrics.TIER, Tier.T2)) - .intValue() - + getLatestMetricValue(Metric.ASSIGNED_COUNT, filter(DruidMetrics.TIER, Tier.T3)) - .intValue(); + verifyNoEvent(Metric.DROPPED_COUNT); + int totalAssignedInRun1 + = getValue(Metric.ASSIGNED_COUNT, filter(DruidMetrics.TIER, Tier.T2)).intValue() + + getValue(Metric.ASSIGNED_COUNT, filter(DruidMetrics.TIER, Tier.T3)).intValue(); Assert.assertTrue(totalAssignedInRun1 > 0 && totalAssignedInRun1 < 40); // Run 2: Segments still queued, nothing is dropped from T1 runCoordinatorCycle(); loadQueuedSegments(); - verifyNoMetricEvent(Metric.DROPPED_COUNT); - int totalLoadedAfterRun2 = historicalT21.getTotalSegments() + historicalT22.getTotalSegments() - + historicalT31.getTotalSegments() + historicalT32.getTotalSegments(); + verifyNoEvent(Metric.DROPPED_COUNT); + int totalLoadedAfterRun2 + = historicalT21.getTotalSegments() + historicalT22.getTotalSegments() + + historicalT31.getTotalSegments() + historicalT32.getTotalSegments(); Assert.assertEquals(totalAssignedInRun1, totalLoadedAfterRun2); // Run 3: Some segments have been loaded @@ -261,21 +260,20 @@ public void testDropHappensAfterTargetReplicationOnEveryTier() runCoordinatorCycle(); loadQueuedSegments(); - int totalDroppedInRun3 = - getLatestMetricValue(Metric.DROPPED_COUNT, filter(DruidMetrics.TIER, Tier.T1)) - .intValue(); + int totalDroppedInRun3 + = getValue(Metric.DROPPED_COUNT, filter(DruidMetrics.TIER, Tier.T1)).intValue(); Assert.assertTrue(totalDroppedInRun3 > 0 && totalDroppedInRun3 < 10); - int totalLoadedAfterRun3 = historicalT21.getTotalSegments() + historicalT22.getTotalSegments() - + historicalT31.getTotalSegments() + historicalT32.getTotalSegments(); + int totalLoadedAfterRun3 + = historicalT21.getTotalSegments() + historicalT22.getTotalSegments() + + historicalT31.getTotalSegments() + historicalT32.getTotalSegments(); Assert.assertEquals(40, totalLoadedAfterRun3); // Run 4: All segments are fully replicated on T2 and T3 runCoordinatorCycle(); loadQueuedSegments(); - int totalDroppedInRun4 = - getLatestMetricValue(Metric.DROPPED_COUNT, filter(DruidMetrics.TIER, Tier.T1)) - .intValue(); + int totalDroppedInRun4 + = getValue(Metric.DROPPED_COUNT, filter(DruidMetrics.TIER, Tier.T1)).intValue(); Assert.assertEquals(10, totalDroppedInRun3 + totalDroppedInRun4); Assert.assertEquals(0, historicalT11.getTotalSegments()); @@ -306,8 +304,8 @@ public void testLoadOfFullyReplicatedSegmentGetsCancelled() runCoordinatorCycle(); // Verify that there are segments in the load queue - verifyLatestMetricValue(Metric.ASSIGNED_COUNT, 10L); - verifyLatestMetricValue( + verifyValue(Metric.ASSIGNED_COUNT, 10L); + verifyValue( Metric.LOAD_QUEUE_COUNT, filter(DruidMetrics.SERVER, historicalT12.getName()), 10 @@ -319,7 +317,7 @@ public void testLoadOfFullyReplicatedSegmentGetsCancelled() runCoordinatorCycle(); // Verify that the segments have been cleared from the load queue - verifyLatestMetricValue( + verifyValue( Metric.LOAD_QUEUE_COUNT, filter(DruidMetrics.SERVER, historicalT12.getName()), 0 From 4dc210fdaab1d306b15b783b07f6c89e89e1dc47 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 15 Sep 2022 08:01:27 +0530 Subject: [PATCH 06/10] Fix inspections --- .../simulate/BlockingExecutorService.java | 2 +- .../simulate/TestSegmentsMetadataManager.java | 24 +++++++------ server/src/test/resources/log4j2.xml | 35 ------------------- 3 files changed, 15 insertions(+), 46 deletions(-) delete mode 100644 server/src/test/resources/log4j2.xml diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java index ebd47ccb23e9..fc59a6bd9d43 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/BlockingExecutorService.java @@ -175,7 +175,7 @@ public boolean isTerminated() } @Override - public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException + public boolean awaitTermination(long timeout, TimeUnit unit) { return false; } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java index 4e826dad5b26..43a96d600707 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentsMetadataManager.java @@ -40,19 +40,19 @@ public class TestSegmentsMetadataManager implements SegmentsMetadataManager { - private final ConcurrentMap segments = new ConcurrentHashMap<>(); - private final ConcurrentMap usedSegments = new ConcurrentHashMap<>(); + private final ConcurrentMap segments = new ConcurrentHashMap<>(); + private final ConcurrentMap usedSegments = new ConcurrentHashMap<>(); public void addSegment(DataSegment segment) { - segments.put(segment.getId(), segment); - usedSegments.put(segment.getId(), segment); + segments.put(segment.getId().toString(), segment); + usedSegments.put(segment.getId().toString(), segment); } public void removeSegment(DataSegment segment) { - segments.remove(segment.getId()); - usedSegments.remove(segment.getId()); + segments.remove(segment.getId().toString()); + usedSegments.remove(segment.getId().toString()); } @Override @@ -94,7 +94,12 @@ public int markAsUsedNonOvershadowedSegments(String dataSource, Set segm @Override public boolean markSegmentAsUsed(String segmentId) { - return false; + if (!segments.containsKey(segmentId)) { + return false; + } + + usedSegments.put(segmentId, segments.get(segmentId)); + return true; } @Override @@ -114,7 +119,7 @@ public int markSegmentsAsUnused(Set segmentIds) { int numModifiedSegments = 0; for (SegmentId segmentId : segmentIds) { - if (usedSegments.remove(segmentId) != null) { + if (usedSegments.remove(segmentId.toString()) != null) { ++numModifiedSegments; } } @@ -124,8 +129,7 @@ public int markSegmentsAsUnused(Set segmentIds) @Override public boolean markSegmentAsUnused(SegmentId segmentId) { - usedSegments.remove(segmentId); - return true; + return usedSegments.remove(segmentId.toString()) != null; } @Nullable diff --git a/server/src/test/resources/log4j2.xml b/server/src/test/resources/log4j2.xml deleted file mode 100644 index d335523d0863..000000000000 --- a/server/src/test/resources/log4j2.xml +++ /dev/null @@ -1,35 +0,0 @@ - - - - - - - - - - - - - - - - - - From 32c9f142b35f307c465b5fe4eadce113224f4eab Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Fri, 16 Sep 2022 19:55:37 +0530 Subject: [PATCH 07/10] Add README for simulations, add SegmentLoadingNegativeTest --- .../server/coordinator/DruidCoordinator.java | 2 +- .../coordinator/CreateDataSegments.java | 2 +- .../server/coordinator/RunRulesTest.java | 2 +- .../CoordinatorSimulationBaseTest.java | 5 +- .../CoordinatorSimulationBuilder.java | 24 +- .../server/coordinator/simulate/README.md | 122 ++++++++ .../simulate/SegmentBalancingTest.java | 36 +-- .../simulate/SegmentLoadingNegativeTest.java | 260 ++++++++++++++++++ .../simulate/SegmentLoadingTest.java | 163 +---------- 9 files changed, 423 insertions(+), 193 deletions(-) create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md create mode 100644 server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingNegativeTest.java diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java index 2244d9ab4d45..df13471aa819 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java @@ -483,7 +483,7 @@ public void moveSegment( ); } - final String toLoadQueueSegPath = curator == null ? null : + final String toLoadQueueSegPath = ZKPaths.makePath(zkPaths.getLoadQueuePath(), toServer.getName(), segmentId.toString()); final LoadPeonCallback loadPeonCallback = () -> { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java index fecc04864226..8f2c1238915d 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/CreateDataSegments.java @@ -72,7 +72,7 @@ public CreateDataSegments withNumPartitions(int numPartitions) return this; } - public List eachOfSizeMb(long sizeMb) + public List eachOfSizeInMb(long sizeMb) { final List segments = new ArrayList<>(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java index ff373254bed0..eb3be4c89510 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java @@ -84,7 +84,7 @@ public void setUp() .forIntervals(24, Granularities.HOUR) .startingAt("2012-01-01") .withNumPartitions(1) - .eachOfSizeMb(1); + .eachOfSizeInMb(1); ruleRunner = new RunRules(new ReplicationThrottler(24, 1, false), coordinator); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java index 2b194b241963..c65a0271f294 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -71,6 +71,9 @@ public void tearDown() } } + /** + * This must be called to start the simulation and set the correct state. + */ void startSimulation(CoordinatorSimulation simulation) { this.sim = simulation; @@ -268,7 +271,7 @@ static class Segments .forIntervals(1, Granularities.DAY) .startingAt("2022-01-01") .withNumPartitions(10) - .eachOfSizeMb(500); + .eachOfSizeInMb(500); } /** diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java index c815e08e925b..b60fca7da8c2 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java @@ -47,6 +47,7 @@ import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroups; import org.apache.druid.server.coordinator.rules.Rule; +import org.apache.druid.server.initialization.ZkPathsConfig; import org.apache.druid.server.lookup.cache.LookupCoordinatorManager; import org.apache.druid.timeline.DataSegment; import org.easymock.EasyMock; @@ -93,30 +94,30 @@ public class CoordinatorSimulationBuilder *

    * Default: "cost" ({@link CostBalancerStrategyFactory}) */ - public CoordinatorSimulationBuilder balancer(BalancerStrategyFactory strategyFactory) + public CoordinatorSimulationBuilder withBalancer(BalancerStrategyFactory strategyFactory) { this.balancerStrategyFactory = strategyFactory; return this; } - public CoordinatorSimulationBuilder servers(List servers) + public CoordinatorSimulationBuilder withServers(List servers) { this.servers = servers; return this; } - public CoordinatorSimulationBuilder servers(DruidServer... servers) + public CoordinatorSimulationBuilder withServers(DruidServer... servers) { - return servers(Arrays.asList(servers)); + return withServers(Arrays.asList(servers)); } - public CoordinatorSimulationBuilder segments(List segments) + public CoordinatorSimulationBuilder withSegments(List segments) { this.segments = segments; return this; } - public CoordinatorSimulationBuilder rules(String datasource, Rule... rules) + public CoordinatorSimulationBuilder withRules(String datasource, Rule... rules) { this.datasourceRules.put(datasource, Arrays.asList(rules)); return this; @@ -127,7 +128,7 @@ public CoordinatorSimulationBuilder rules(String datasource, Rule... rules) *

    * Default: false */ - public CoordinatorSimulationBuilder loadSegmentsImmediately(boolean loadImmediately) + public CoordinatorSimulationBuilder withImmediateSegmentLoading(boolean loadImmediately) { this.loadImmediately = loadImmediately; return this; @@ -139,7 +140,7 @@ public CoordinatorSimulationBuilder loadSegmentsImmediately(boolean loadImmediat *

    * Default: true */ - public CoordinatorSimulationBuilder autoSyncInventory(boolean autoSync) + public CoordinatorSimulationBuilder withAutoInventorySync(boolean autoSync) { this.autoSyncInventory = autoSync; return this; @@ -150,7 +151,7 @@ public CoordinatorSimulationBuilder autoSyncInventory(boolean autoSync) *

    * Default values: Specified in {@link CoordinatorDynamicConfig.Builder}. */ - public CoordinatorSimulationBuilder dynamicConfig(CoordinatorDynamicConfig dynamicConfig) + public CoordinatorSimulationBuilder withDynamicConfig(CoordinatorDynamicConfig dynamicConfig) { this.dynamicConfig = dynamicConfig; return this; @@ -190,7 +191,7 @@ public CoordinatorSimulation build() // Build the coordinator final DruidCoordinator coordinator = new DruidCoordinator( env.coordinatorConfig, - null, + new ZkPathsConfig(), env.jacksonConfigManager, env.segmentManager, env.coordinatorInventoryView, @@ -334,9 +335,6 @@ public void loadQueuedSegments() // Load all the queued segments, handle their responses and execute callbacks int loadedSegments = env.executorFactory.historicalLoader.finishAllPendingTasks(); loadQueueExecutor.finishNextPendingTasks(loadedSegments); - - // TODO: sync should happen here?? or should it not?? - env.executorFactory.loadCallbackExecutor.finishAllPendingTasks(); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md b/server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md new file mode 100644 index 000000000000..422ada576901 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md @@ -0,0 +1,122 @@ +# Coordinator simulations + +The simulation framework allows developers to recreate arbitrary cluster setups and verify coordinator behaviour. Tests +written using the framework can also help identify performance bottlenecks or potential bugs in the system and even +compare different balancing strategies. + +As opposed to unit tests, simulations are meant to test the coordinator as a whole and verify the interactions of all +the underlying parts. In that regard, these simulations resemble integration tests more closely. + +## Test targets + +The primary test target is the `DruidCoordinator` itself. The behaviour of the following entities can also be verified +using simulations: + +- `LoadQueuePeon`, `LoadQueueTaskMaster` +- All coordinator duties, e.g. `BalanceSegments`, `RunRules` +- All retention rules + +## Capabilities + +The framework provides control over the following aspects of the setup: + +| Input | Details | Actions | +|-------|---------|---------| +|cluster | server name, type, tier, size | add a server, remove a server| +|segment |datasource, interval, version, partition num, size | add/remove from server, mark used/unused, publish new segments| +|rules | type (foreverLoad, drop, etc), replica count per tier | set rules for a datasource| +|configs |coordinator period, load queue type, load queue size, max segments to balance | set or update a config | + +The above actions can be performed at any point after building the simulation. So, you could even recreate scenarios +where during a coordinator run, a server crashes or the retention rules of a datasource change, and verify the behaviour +of the coordinator in these situations. + +## Design + +1. __Execution__: A tight dependency on time durations such as the period of a repeating task or the delay before a + scheduled task makes it difficult to reliably reproduce a test scenario. As a result, the tests become flaky. Thus, + all the executors required for coordinator operations have been allowed only two possible modes of execution: + - __immediate__: Execute tasks on the calling thread itself. + - __blocked__: Keep tasks in a queue until explicitly invoked. +2. __Internal dependencies__: In order to allow realistic reproductions of the coordinator behaviour, none of the + internal parts of the coordinator have been mocked in the framework and new tests need not mock anything at all. +3. __External dependencies__: Since these tests are meant to verify the behaviour of only the coordinator, the + interfaces to communicate with external dependencies have been provided as simple in-memory implementations: + - communication with metadata store: `SegmentMetadataManager`, `MetadataRuleManager` + - communication with historicals: `HttpClient`, `ServerInventoryView` +4. __Inventory__: The coordinator maintains an inventory view of the cluster state. Simulations can choose from two + modes of inventory update - auto and manual. In auto update mode, any change made to the cluster is immediately + reflected in the inventory view. In manual update mode, the inventory must be explicitly synchronized with the + cluster state. + +## Limitations + +- The framework does not expose the coordinator HTTP endpoints. +- It should not be used to verify the absolute values of execution latencies, e.g. the time taken to compute the + balancing cost of a segment. But the relative values can still be a good indicator while doing comparisons between, + say two balancing strategies. + +## Usage + +Writing a test class: + +- Extend `CoordinatorSimulationBaseTest`. This base test exposes methods to get or set the state of the cluster and + coordinator during a simulation. +- Build a simulation using `CoordinatorSimulation.builder()` with specified segments, servers, rules and configs. +- Start the simulation with `startSimulation(simulation)`. +- Invoke coordinator runs with `runCoordinatorCycle()` +- Verify emitted metrics and current cluster state + +Example: + +```java +public class SimpleSimulationTest extends CoordinatorSimulationBaseTest +{ + @Test + public void testShiftSegmentsToDifferentTier() + { + // Create segments + List segments = + CreateDataSegments.ofDatasource("wiki") + .forIntervals(30, Granularities.DAY) + .startingAt("2022-01-01") + .withNumPartitions(10) + .eachOfSizeInMb(500); + + // Create servers + DruidServer historicalTier1 = createHistoricalTier(1, "tier_1", 10000); + DruidServer historicalTier2 = createHistoricalTier(1, "tier_2", 20000); + + // Build simulation + CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .withServers(historicalTier1, historicalTier2) + .withSegments(segments) + .withRules("wiki".Load.on("tier_2", 1).forever()) + .build(); + + // Start the simulation with all segments loaded on tier_1 + segments.forEach(historicalTier1::addSegment); + startSimulation(sim); + + // Run a few coordinator cycles + int totalLoadedOnT2 = 0; + int totalDroppedFromT1 = 0; + for (int i = 0; i < 10; ++i) { + runCoordinatorCycle(); + loadQueuedSegments(); + totalLoadedOnT2 += getValue("segment/assigned/count", filter("tier", "tier_2")); + totalDroppedFromT1 += getValue("segment/dropped/count", filter("tier", "tier_1")); + } + + // Verify that some segments have been loaded/dropped + Assert.assertTrue(totalLoadedOnT2 > 0 && totalLoadedOnT2 <= segments.size()); + Assert.assertTrue(totalDroppedFromT1 > 0 && totalDroppedFromT1 <= segments.size()); + Assert.assertTrue(totalDroppedFromT1 <= totalLoadedOnT2); + } +} +``` + +## More examples + +See `org.apache.druid.server.coordinator.simulate.SegmentLoadingTest` diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java index 635760695a7c..77b9820a9533 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentBalancingTest.java @@ -24,7 +24,6 @@ import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; import org.apache.druid.timeline.DataSegment; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import java.util.List; @@ -50,19 +49,6 @@ public void setUp() @Test public void testBalancingWithSyncedInventory() - { - testBalancingWithInventorySynced(true); - } - - @Test - @Ignore("Fix #12881 to enable this test. " - + "Current impl requires updated inventory for correct callback behaviour.") - public void testBalancingWithUnsyncedInventory() - { - testBalancingWithInventorySynced(false); - } - - private void testBalancingWithInventorySynced(boolean autoSyncInventory) { // maxSegmentsToMove = 10, unlimited load queue, replicationThrottleLimit = 10 CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(10, 0, 10); @@ -70,21 +56,17 @@ private void testBalancingWithInventorySynced(boolean autoSyncInventory) // historicals = 2(T1), replicas = 1(T1) final CoordinatorSimulation sim = CoordinatorSimulation.builder() - .segments(segments) - .servers(historicalT11, historicalT12) - .rules(datasource, Load.on(Tier.T1, 1).forever()) - .dynamicConfig(dynamicConfig) - .autoSyncInventory(autoSyncInventory) + .withSegments(segments) + .withServers(historicalT11, historicalT12) + .withRules(datasource, Load.on(Tier.T1, 1).forever()) + .withDynamicConfig(dynamicConfig) + .withAutoInventorySync(true) .build(); // Put all the segments on histT11 segments.forEach(historicalT11::addDataSegment); startSimulation(sim); - if (!autoSyncInventory) { - syncInventoryView(); - } - runCoordinatorCycle(); // Verify that segments have been chosen for balancing @@ -107,10 +89,10 @@ public void testBalancingOfFullyReplicatedSegment() // historicals = 2(in T1), replicas = 1(T1) final CoordinatorSimulation sim = CoordinatorSimulation.builder() - .segments(segments) - .servers(historicalT11, historicalT12) - .dynamicConfig(dynamicConfig) - .rules(datasource, Load.on(Tier.T1, 1).forever()) + .withSegments(segments) + .withServers(historicalT11, historicalT12) + .withDynamicConfig(dynamicConfig) + .withRules(datasource, Load.on(Tier.T1, 1).forever()) .build(); // Put all the segments on histT11 diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingNegativeTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingNegativeTest.java new file mode 100644 index 000000000000..52dc7c0933b9 --- /dev/null +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingNegativeTest.java @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.simulate; + +import org.apache.druid.client.DruidServer; +import org.apache.druid.query.DruidMetrics; +import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; +import org.apache.druid.timeline.DataSegment; +import org.junit.Assert; +import org.junit.Test; + +import java.util.List; + +/** + * Contains negative tests that verify existing erroneous behaviour of segment + * loading. The underlying issues should be fixed and the modified tests + * should be migrated to {@link SegmentLoadingTest}. + *

    + * Identified issues: + * Apache #12881 + */ +public class SegmentLoadingNegativeTest extends CoordinatorSimulationBaseTest +{ + private DruidServer historicalT11; + private DruidServer historicalT12; + private DruidServer historicalT21; + + private final String datasource = DS.WIKI; + private final List segments = Segments.WIKI_10X1D; + + @Override + public void setUp() + { + // Setup historicals for 2 tiers, size 10 GB each + historicalT11 = createHistorical(1, Tier.T1, 10_000); + historicalT12 = createHistorical(2, Tier.T1, 10_000); + historicalT21 = createHistorical(1, Tier.T2, 10_000); + } + + /** + * Correct behaviour: replicationThrottleLimit should not be violated even if + * segment loading is fast. + *

    + * Fix Apache #12881 to fix this test. + */ + @Test + public void testImmediateLoadingViolatesThrottleLimit() + { + // Disable balancing, infinite load queue size, replicationThrottleLimit = 2 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); + + // historicals = 2(in T1), segments = 10*1day + // replicas = 2(on T1), immediate segment loading + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .withSegments(segments) + .withServers(historicalT11, historicalT12) + .withRules(datasource, Load.on(Tier.T1, 2).forever()) + .withImmediateSegmentLoading(true) + .withDynamicConfig(dynamicConfig) + .build(); + + // Put the first replica of all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); + + startSimulation(sim); + runCoordinatorCycle(); + + // Verify that number of replicas assigned exceeds the replicationThrottleLimit + verifyValue(Metric.ASSIGNED_COUNT, 10L); + + Assert.assertEquals(10, historicalT11.getTotalSegments()); + Assert.assertEquals(10, historicalT12.getTotalSegments()); + verifyDatasourceIsFullyLoaded(datasource); + } + + /** + * Correct behaviour: The first replica on any tier should not be throttled. + *

    + * Fix Apache #12881 to fix this test. + */ + @Test + public void testFirstReplicaOnAnyTierIsThrottled() + { + // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); + + // historicals = 1(in T1) + 1(in T2) + // replicas = 1(on T1) + 1(on T2) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .withSegments(segments) + .withServers(historicalT11, historicalT21) + .withDynamicConfig(dynamicConfig) + .withRules( + datasource, + Load.on(Tier.T1, 1).andOn(Tier.T2, 1).forever() + ) + .build(); + + // Put the first replica of all the segments on T1 + segments.forEach(historicalT11::addDataSegment); + + startSimulation(sim); + runCoordinatorCycle(); + + // Verify that num replicas assigned to T2 are equal to the replicationthrottleLimit + verifyValue( + Metric.ASSIGNED_COUNT, + filter(DruidMetrics.TIER, Tier.T2), + 2L + ); + + loadQueuedSegments(); + + verifyDatasourceIsFullyLoaded(datasource); + Assert.assertEquals(10, historicalT11.getTotalSegments()); + Assert.assertEquals(2, historicalT21.getTotalSegments()); + } + + /** + * Correct behaviour: Historical should not get overassigned even if loading is fast. + *

    + * Fix Apache #12881 to fix this test. + */ + @Test + public void testImmediateLoadingOverassignsHistorical() + { + // historicals = 1(in T1), size 1 GB + final DruidServer historicalT11 = createHistorical(1, Tier.T1, 1000); + + // disable balancing, unlimited load queue, replicationThrottleLimit = 10 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 10); + + // segments = 10*1day, size 500 MB + // strategy = cost, replicas = 1(T1) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .withSegments(segments) + .withServers(historicalT11) + .withDynamicConfig(dynamicConfig) + .withRules(datasource, Load.on(Tier.T1, 1).forever()) + .withImmediateSegmentLoading(true) + .build(); + + startSimulation(sim); + runCoordinatorCycle(); + + // The historical is assigned several segments but loads only upto its capacity + verifyValue(Metric.ASSIGNED_COUNT, 10L); + Assert.assertEquals(2, historicalT11.getTotalSegments()); + } + + /** + * Correct behaviour: For a fully replicated segment, items that are in the load + * queue should get cancelled so that the coordinator does not have to wait + * for the loads to finish and then take remedial action. + *

    + * Fix Apache #12881 to fix this test case. + */ + @Test + public void testLoadOfFullyReplicatedSegmentIsNotCancelled() + { + // disable balancing, unlimited load queue, replicationThrottleLimit = 10 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 10); + + // historicals = 2(in T1), replicas = 2(on T1) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .withSegments(segments) + .withServers(historicalT11, historicalT12) + .withDynamicConfig(dynamicConfig) + .withRules(datasource, Load.on(Tier.T1, 2).forever()) + .build(); + + // Put the first replica of all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); + + startSimulation(sim); + runCoordinatorCycle(); + + // Verify that there are segments in the load queue + verifyValue(Metric.ASSIGNED_COUNT, 10L); + verifyValue( + Metric.LOAD_QUEUE_COUNT, + filter(DruidMetrics.SERVER, historicalT12.getName()), + 10 + ); + + // Put the second replica of all the segments on histT12 + segments.forEach(historicalT12::addDataSegment); + + runCoordinatorCycle(); + + // Verify that the segments are still in the load queue + verifyValue( + Metric.LOAD_QUEUE_COUNT, + filter(DruidMetrics.SERVER, historicalT12.getName()), + 10 + ); + } + + /** + * Correct behaviour: Balancing should never cause over-replication, even when + * the inventory view is not updated. + *

    + * Fix Apache #12881 to fix this test. + */ + @Test + public void testBalancingWithStaleInventoryCausesOverReplication() + { + // maxSegmentsToMove = 10, unlimited load queue, replicationThrottleLimit = 10 + CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(10, 0, 10); + + // historicals = 2(T1), replicas = 1(T1) + final CoordinatorSimulation sim = + CoordinatorSimulation.builder() + .withSegments(segments) + .withServers(historicalT11, historicalT12) + .withRules(datasource, Load.on(Tier.T1, 1).forever()) + .withDynamicConfig(dynamicConfig) + .withAutoInventorySync(false) + .build(); + + // Put all the segments on histT11 + segments.forEach(historicalT11::addDataSegment); + + startSimulation(sim); + syncInventoryView(); + runCoordinatorCycle(); + + // Verify that segments have been chosen for balancing + verifyValue(Metric.MOVED_COUNT, 5L); + + loadQueuedSegments(); + + // Verify that segments have now been balanced out + Assert.assertEquals(10, historicalT11.getTotalSegments()); + Assert.assertEquals(5, historicalT12.getTotalSegments()); + verifyDatasourceIsFullyLoaded(datasource); + } + +} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java index 36e3205035b8..1edeab8a370f 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/SegmentLoadingTest.java @@ -24,7 +24,6 @@ import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; import org.apache.druid.timeline.DataSegment; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import java.util.List; @@ -53,47 +52,6 @@ public void setUp() historicalT22 = createHistorical(2, Tier.T2, 10_000); } - @Test - @Ignore("Fix #12881 to enable this test. " - + "Current behaviour is to throttle globally and not per tier.") - public void testFirstReplicaOnAnyTierIsNotThrottled() - { - // Disable balancing, infinite load queue size, replicateThrottleLimit = 2 - CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); - - // historicals = 1(in T1) + 1(in T2) - // replicas = 1(on T1) + 1(on T2) - final CoordinatorSimulation sim = - CoordinatorSimulation.builder() - .segments(segments) - .servers(historicalT11, historicalT21) - .dynamicConfig(dynamicConfig) - .rules( - datasource, - Load.on(Tier.T1, 1).andOn(Tier.T2, 1).forever() - ) - .build(); - - // Put the first replica of all the segments on T1 - segments.forEach(historicalT11::addDataSegment); - - startSimulation(sim); - runCoordinatorCycle(); - - // Verify that all the required replicas for T2 have been assigned - verifyValue( - Metric.ASSIGNED_COUNT, - filter(DruidMetrics.TIER, Tier.T2), - 10L - ); - - loadQueuedSegments(); - - verifyDatasourceIsFullyLoaded(datasource); - Assert.assertEquals(10, historicalT11.getTotalSegments()); - Assert.assertEquals(10, historicalT21.getTotalSegments()); - } - @Test public void testSecondReplicaOnAnyTierIsThrottled() { @@ -104,10 +62,10 @@ public void testSecondReplicaOnAnyTierIsThrottled() // replicas = 2(on T1) final CoordinatorSimulation sim = CoordinatorSimulation.builder() - .segments(segments) - .servers(historicalT11, historicalT12) - .rules(datasource, Load.on(Tier.T1, 2).forever()) - .dynamicConfig(dynamicConfig) + .withSegments(segments) + .withServers(historicalT11, historicalT12) + .withRules(datasource, Load.on(Tier.T1, 2).forever()) + .withDynamicConfig(dynamicConfig) .build(); // Put the first replica of all the segments on histT11 @@ -120,59 +78,12 @@ public void testSecondReplicaOnAnyTierIsThrottled() verifyValue(Metric.ASSIGNED_COUNT, 2L); loadQueuedSegments(); - Assert.assertEquals(10, historicalT11.getTotalSegments()); Assert.assertEquals(2, historicalT12.getTotalSegments()); } - @Test - @Ignore("Fix #12881 to enable this test. " - + "Current impl allows throttle limit to be violated if loading is fast.") - public void testImmediateLoadingDoesNotViolateThrottleLimit() - { - // Disable balancing, infinite load queue size, replicationThrottleLimit = 2 - CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 2); - - // historicals = 2(in T1), segments = 10*1day - // replicas = 2(on T1), immediate segment loading - final CoordinatorSimulation sim = - CoordinatorSimulation.builder() - .segments(segments) - .servers(historicalT11, historicalT12) - .rules(datasource, Load.on(Tier.T1, 2).forever()) - .loadSegmentsImmediately(true) - .dynamicConfig(dynamicConfig) - .build(); - - // Put the first replica of all the segments on histT11 - segments.forEach(historicalT11::addDataSegment); - - startSimulation(sim); - runCoordinatorCycle(); - - // Verify that number of replicas assigned is equal to replicationThrottleLimit - verifyValue(Metric.ASSIGNED_COUNT, 2L); - - Assert.assertEquals(10, historicalT11.getTotalSegments()); - Assert.assertEquals(2, historicalT12.getTotalSegments()); - verifyDatasourceIsFullyLoaded(datasource); - } - @Test public void testLoadingDoesNotOverassignHistorical() - { - testHistoricalIsNotOverAssigned(false); - } - - @Test - @Ignore("Fix #12881 to enable this test. " - + "Current impl may overassign a historical if loading is fast.") - public void testImmediateLoadingDoesNotOverassignHistorical() - { - testHistoricalIsNotOverAssigned(true); - } - - private void testHistoricalIsNotOverAssigned(boolean loadImmediately) { // historicals = 1(in T1), size 1 GB final DruidServer historicalT11 = createHistorical(1, Tier.T1, 1000); @@ -184,11 +95,11 @@ private void testHistoricalIsNotOverAssigned(boolean loadImmediately) // strategy = cost, replicas = 1(T1) final CoordinatorSimulation sim = CoordinatorSimulation.builder() - .segments(segments) - .servers(historicalT11) - .dynamicConfig(dynamicConfig) - .rules(datasource, Load.on(Tier.T1, 1).forever()) - .loadSegmentsImmediately(loadImmediately) + .withSegments(segments) + .withServers(historicalT11) + .withDynamicConfig(dynamicConfig) + .withRules(datasource, Load.on(Tier.T1, 1).forever()) + .withImmediateSegmentLoading(false) .build(); startSimulation(sim); @@ -196,9 +107,7 @@ private void testHistoricalIsNotOverAssigned(boolean loadImmediately) // Verify that the number of segments assigned is within the historical capacity verifyValue(Metric.ASSIGNED_COUNT, 2L); - if (!loadImmediately) { - loadQueuedSegments(); - } + loadQueuedSegments(); Assert.assertEquals(2, historicalT11.getTotalSegments()); } @@ -220,10 +129,10 @@ public void testDropHappensAfterTargetReplicationOnEveryTier() final DruidServer historicalT32 = createHistorical(2, Tier.T3, 10_000); final CoordinatorSimulation sim = CoordinatorSimulation.builder() - .segments(segments) - .dynamicConfig(dynamicConfig) - .rules(datasource, Load.on(Tier.T2, 2).andOn(Tier.T3, 2).forever()) - .servers( + .withSegments(segments) + .withDynamicConfig(dynamicConfig) + .withRules(datasource, Load.on(Tier.T2, 2).andOn(Tier.T3, 2).forever()) + .withServers( historicalT11, historicalT21, historicalT22, @@ -280,48 +189,4 @@ public void testDropHappensAfterTargetReplicationOnEveryTier() verifyDatasourceIsFullyLoaded(datasource); } - @Test - @Ignore("Fix #12881 to enable this test. " - + "Current impl does not cancel load even if segment is fully replicated.") - public void testLoadOfFullyReplicatedSegmentGetsCancelled() - { - // disable balancing, unlimited load queue, replicationThrottleLimit = 10 - CoordinatorDynamicConfig dynamicConfig = createDynamicConfig(0, 0, 10); - - // historicals = 2(in T1), replicas = 2(on T1) - final CoordinatorSimulation sim = - CoordinatorSimulation.builder() - .segments(segments) - .servers(historicalT11, historicalT12) - .dynamicConfig(dynamicConfig) - .rules(datasource, Load.on(Tier.T1, 2).forever()) - .build(); - - // Put the first replica of all the segments on histT11 - segments.forEach(historicalT11::addDataSegment); - - startSimulation(sim); - runCoordinatorCycle(); - - // Verify that there are segments in the load queue - verifyValue(Metric.ASSIGNED_COUNT, 10L); - verifyValue( - Metric.LOAD_QUEUE_COUNT, - filter(DruidMetrics.SERVER, historicalT12.getName()), - 10 - ); - - // Put the second replica of all the segments on histT12 - segments.forEach(historicalT12::addDataSegment); - - runCoordinatorCycle(); - - // Verify that the segments have been cleared from the load queue - verifyValue( - Metric.LOAD_QUEUE_COUNT, - filter(DruidMetrics.SERVER, historicalT12.getName()), - 0 - ); - } - } From 3a3d2c4a3bfdf6f7ee3b518e10166d46cd57b162 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Mon, 19 Sep 2022 10:52:52 +0530 Subject: [PATCH 08/10] Add license to simulate/README --- .../server/coordinator/simulate/README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md b/server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md index 422ada576901..a1562c518754 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/README.md @@ -1,3 +1,22 @@ + + # Coordinator simulations The simulation framework allows developers to recreate arbitrary cluster setups and verify coordinator behaviour. Tests From 2479e4e31299f045ba4e9bd5276e5d06ec89bd4e Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Tue, 20 Sep 2022 12:40:31 +0530 Subject: [PATCH 09/10] Collect ServiceMetricEvents in StubServiceEmitter --- .../java/util/metrics/StubServiceEmitter.java | 32 +++++++++-------- .../simulate/CoordinatorSimulation.java | 3 +- .../CoordinatorSimulationBaseTest.java | 34 +++++++++++-------- .../CoordinatorSimulationBuilder.java | 15 +++++--- .../TestSegmentLoadingHttpClient.java | 7 ++-- 5 files changed, 53 insertions(+), 38 deletions(-) diff --git a/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java b/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java index cb8926500a72..653dc8a08aae 100644 --- a/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java +++ b/core/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java @@ -19,19 +19,17 @@ package org.apache.druid.java.util.metrics; -import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.java.util.emitter.core.Event; -import org.apache.druid.java.util.emitter.service.AlertEvent; import org.apache.druid.java.util.emitter.service.ServiceEmitter; +import org.apache.druid.java.util.emitter.service.ServiceMetricEvent; import java.util.ArrayList; import java.util.List; public class StubServiceEmitter extends ServiceEmitter { - private static final Logger log = new Logger(StubServiceEmitter.class); - private final List events = new ArrayList<>(); + private final List metricEvents = new ArrayList<>(); public StubServiceEmitter(String service, String host) { @@ -41,25 +39,28 @@ public StubServiceEmitter(String service, String host) @Override public void emit(Event event) { - if (event instanceof AlertEvent) { - final AlertEvent alertEvent = (AlertEvent) event; - log.warn( - "[%s] [%s] [%s]: %s%n", - alertEvent.getSeverity(), - alertEvent.getService(), - alertEvent.getFeed(), - alertEvent.getDescription() - ); - } else { - events.add(event); + if (event instanceof ServiceMetricEvent) { + metricEvents.add((ServiceMetricEvent) event); } + events.add(event); } + /** + * Gets all the events emitted since the previous {@link #flush()}. + */ public List getEvents() { return events; } + /** + * Gets all the metric events emitted since the previous {@link #flush()}. + */ + public List getMetricEvents() + { + return metricEvents; + } + @Override public void start() { @@ -69,6 +70,7 @@ public void start() public void flush() { events.clear(); + metricEvents.clear(); } @Override diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java index bb2c6f990076..ea5fa80badab 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java @@ -21,6 +21,7 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.emitter.core.Event; +import org.apache.druid.java.util.emitter.service.ServiceMetricEvent; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; import java.util.List; @@ -82,7 +83,7 @@ interface CoordinatorState /** * Returns the metric events emitted in the previous coordinator run. */ - List getMetricEvents(); + List getMetricEvents(); /** * Gets the load percentage of the specified datasource as seen by the coordinator. diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java index c65a0271f294..3dee651f205a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -22,6 +22,7 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.emitter.core.Event; +import org.apache.druid.java.util.emitter.service.ServiceMetricEvent; import org.apache.druid.server.coordination.ServerType; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; import org.apache.druid.server.coordinator.CreateDataSegments; @@ -57,7 +58,7 @@ public abstract class CoordinatorSimulationBaseTest static final double DOUBLE_DELTA = 10e-9; private CoordinatorSimulation sim; - private final Map> latestMetricEvents = new HashMap<>(); + private final Map> latestMetricEvents = new HashMap<>(); @Before public abstract void setUp(); @@ -87,18 +88,14 @@ public void runCoordinatorCycle() sim.coordinator().runCoordinatorCycle(); // Extract the metric values of this run - for (Event event : sim.coordinator().getMetricEvents()) { - Map eventMap = event.toMap(); - String metricName = (String) eventMap.get("metric"); - if (metricName != null) { - latestMetricEvents.computeIfAbsent(metricName, m -> new ArrayList<>()) - .add(event); - } + for (ServiceMetricEvent event : sim.coordinator().getMetricEvents()) { + latestMetricEvents.computeIfAbsent(event.getMetric(), m -> new ArrayList<>()) + .add(event); } } @Override - public List getMetricEvents() + public List getMetricEvents() { return sim.coordinator().getMetricEvents(); } @@ -179,17 +176,17 @@ Number getValue(String metricName, Map dimensionFilters) private List getMetricValues(String metricName, Map dimensionFilters) { final List values = new ArrayList<>(); - final List events = latestMetricEvents.getOrDefault(metricName, Collections.emptyList()); + final List events = latestMetricEvents.getOrDefault(metricName, Collections.emptyList()); final Map filters = dimensionFilters == null ? Collections.emptyMap() : dimensionFilters; - for (Event event : events) { - final Map eventMap = event.toMap(); + for (ServiceMetricEvent event : events) { + final Map userDims = event.getUserDims(); boolean match = filters.keySet().stream() - .map(d -> filters.get(d).equals(eventMap.get(d))) + .map(d -> filters.get(d).equals(userDims.get(d))) .reduce((a, b) -> a && b) .orElse(true); if (match) { - values.add((Number) eventMap.get("value")); + values.add(event.getValue()); } } @@ -197,6 +194,15 @@ private List getMetricValues(String metricName, Map dime } // Utility methods + + /** + * Creates a {@link CoordinatorDynamicConfig} with the specified values of: + * {@code maxSegmentsToMove, maxSegmentsInNodeLoadingQueue and replicationThrottleLimit}. + * The created config always has {@code useBatchedSegmentSampler=true} to avoid + * flakiness in tests. + * + * @see CoordinatorSimulationBaseTest + */ static CoordinatorDynamicConfig createDynamicConfig( int maxSegmentsToMove, int maxSegmentsInNodeLoadingQueue, diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java index b60fca7da8c2..29301ea033d4 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java @@ -32,7 +32,7 @@ import org.apache.druid.java.util.common.concurrent.ScheduledExecutorFactory; import org.apache.druid.java.util.common.lifecycle.Lifecycle; import org.apache.druid.java.util.emitter.EmittingLogger; -import org.apache.druid.java.util.emitter.core.Event; +import org.apache.druid.java.util.emitter.service.ServiceMetricEvent; import org.apache.druid.java.util.http.client.HttpClient; import org.apache.druid.java.util.metrics.StubServiceEmitter; import org.apache.druid.server.coordinator.BalancerStrategyFactory; @@ -149,7 +149,14 @@ public CoordinatorSimulationBuilder withAutoInventorySync(boolean autoSync) /** * Specifies the CoordinatorDynamicConfig to be used in the simulation. *

    - * Default values: Specified in {@link CoordinatorDynamicConfig.Builder}. + * Default values: {@code useBatchedSegmentSampler = true}, other params as + * specified in {@link CoordinatorDynamicConfig.Builder}. + *

    + * Tests that verify balancing behaviour should set + * {@link CoordinatorDynamicConfig#useBatchedSegmentSampler()} to true. + * Otherwise, the segment sampling is random and can produce repeated values + * leading to flakiness in the tests. The simulation sets this field to true by + * default. */ public CoordinatorSimulationBuilder withDynamicConfig(CoordinatorDynamicConfig dynamicConfig) { @@ -353,9 +360,9 @@ public double getLoadPercentage(String datasource) } @Override - public List getMetricEvents() + public List getMetricEvents() { - return new ArrayList<>(env.serviceEmitter.getEvents()); + return new ArrayList<>(env.serviceEmitter.getMetricEvents()); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java index 9f340a298246..0b91e7009026 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestSegmentLoadingHttpClient.java @@ -74,7 +74,7 @@ public ListenableFuture go( HttpResponseHandler handler ) { - throw new UnsupportedOperationException(); + return go(request, handler, null); } @Override @@ -84,13 +84,12 @@ public ListenableFuture go( Duration readTimeout ) { - return executorService.submit(() -> processRequest(request, handler, readTimeout)); + return executorService.submit(() -> processRequest(request, handler)); } private Final processRequest( Request request, - HttpResponseHandler handler, - Duration readTimeout + HttpResponseHandler handler ) { try { From 2bf2985156e5fe337b19d4e7f69c04c4ae60a564 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Tue, 20 Sep 2022 12:43:28 +0530 Subject: [PATCH 10/10] Fix checkstyle --- .../druid/server/coordinator/simulate/CoordinatorSimulation.java | 1 - .../coordinator/simulate/CoordinatorSimulationBaseTest.java | 1 - 2 files changed, 2 deletions(-) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java index ea5fa80badab..6822419f79a0 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulation.java @@ -20,7 +20,6 @@ package org.apache.druid.server.coordinator.simulate; import org.apache.druid.client.DruidServer; -import org.apache.druid.java.util.emitter.core.Event; import org.apache.druid.java.util.emitter.service.ServiceMetricEvent; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java index 3dee651f205a..60e8e428244c 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -21,7 +21,6 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.granularity.Granularities; -import org.apache.druid.java.util.emitter.core.Event; import org.apache.druid.java.util.emitter.service.ServiceMetricEvent; import org.apache.druid.server.coordination.ServerType; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig;