From dee1736e43cb20e2a18ecd8e9c7747c104319d78 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 11 Apr 2018 16:14:44 -0700 Subject: [PATCH 1/6] Optimize coordinator loadStatus --- .../druid/benchmark/LoadStatusBenchmark.java | 122 ++++++++++++++++++ .../server/coordinator/DruidCoordinator.java | 4 +- 2 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java diff --git a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java new file mode 100644 index 000000000000..1465007ac6b4 --- /dev/null +++ b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java @@ -0,0 +1,122 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.benchmark; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import io.druid.java.util.common.StringUtils; +import io.druid.timeline.DataSegment; +import io.druid.timeline.partition.NoneShardSpec; +import org.joda.time.Interval; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; + +@State(Scope.Benchmark) +@Fork(value = 1) +@Warmup(iterations = 15) +@Measurement(iterations = 30) +public class LoadStatusBenchmark +{ + // Number of total data segments + @Param({"10000"}) + int totalSegmentsCount; + + @Param({"true", "false"}) + private boolean serverHasAllSegments; + + private Set datasourceSegments; + private Collection serverSegments; + + @Setup(Level.Invocation) + public void setup() throws IOException + { + Map immutableDatasourceSegmentsMap; + ConcurrentHashMap serverSegmentsMap; + + HashMap datasourceSegmentsMap = Maps.newHashMap(); + serverSegmentsMap = new ConcurrentHashMap<>(); + + for (int i = 0; i < totalSegmentsCount; i++) { + DataSegment segment = new DataSegment( + "benchmarkDatasource", + new Interval(StringUtils.format("%s-01-01/%s-12-31", i + 1970, i + 1970)), + "1", + null, + null, + null, + NoneShardSpec.instance(), + 1, + 1 + ); + + datasourceSegmentsMap.put(segment.getIdentifier(), segment); + + if (serverHasAllSegments || i % 2 == 0) { + serverSegmentsMap.put(segment.getIdentifier(), segment); + } + } + + immutableDatasourceSegmentsMap = ImmutableMap.copyOf(datasourceSegmentsMap); + + datasourceSegments = Sets.newHashSet(immutableDatasourceSegmentsMap.values()); + serverSegments = Collections.unmodifiableCollection(serverSegmentsMap.values()); + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.MICROSECONDS) + public void oldVersion(Blackhole blackhole) + { + datasourceSegments.removeAll(serverSegments); + blackhole.consume(datasourceSegments); + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.MICROSECONDS) + public void newVersion(Blackhole blackhole) + { + for (DataSegment segment : serverSegments) { + datasourceSegments.remove(segment); + } + blackhole.consume(datasourceSegments); + } +} diff --git a/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java index d8d49b1a2ba8..3919ae39d331 100644 --- a/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java +++ b/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java @@ -297,7 +297,9 @@ public Map getLoadStatus() for (DruidServer druidServer : serverInventoryView.getInventory()) { final DruidDataSource loadedView = druidServer.getDataSource(dataSource.getName()); if (loadedView != null) { - segments.removeAll(loadedView.getSegments()); + for (DataSegment serverSegment : loadedView.getSegments()) { + segments.remove(serverSegment); + } } } final int unloadedSegmentSize = segments.size(); From 9012a08fff74cde2ccbb5f19f3f888a5df5b7275 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 11 Apr 2018 17:36:33 -0700 Subject: [PATCH 2/6] Add comment --- .../main/java/io/druid/server/coordinator/DruidCoordinator.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java index 3919ae39d331..8868c7673316 100644 --- a/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java +++ b/server/src/main/java/io/druid/server/coordinator/DruidCoordinator.java @@ -297,6 +297,8 @@ public Map getLoadStatus() for (DruidServer druidServer : serverInventoryView.getInventory()) { final DruidDataSource loadedView = druidServer.getDataSource(dataSource.getName()); if (loadedView != null) { + // This does not use segments.removeAll(loadedView.getSegments()) for performance reasons. + // Please see https://github.com/druid-io/druid/pull/5632 and LoadStatusBenchmark for more info. for (DataSegment serverSegment : loadedView.getSegments()) { segments.remove(serverSegment); } From 6a153ea9cd794206214a45c2f5ea2a25d2c781e6 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 11 Apr 2018 19:43:56 -0700 Subject: [PATCH 3/6] Fix teamcity --- .../src/main/java/io/druid/benchmark/LoadStatusBenchmark.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java index 1465007ac6b4..07994d801776 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java @@ -66,7 +66,7 @@ public class LoadStatusBenchmark private Collection serverSegments; @Setup(Level.Invocation) - public void setup() throws IOException + public void setup() { Map immutableDatasourceSegmentsMap; ConcurrentHashMap serverSegmentsMap; From 5e68520b7cbc2645d6de25f1199ba9f764487e4b Mon Sep 17 00:00:00 2001 From: jon-wei Date: Thu, 12 Apr 2018 11:48:50 -0700 Subject: [PATCH 4/6] Checkstyle --- .../src/main/java/io/druid/benchmark/LoadStatusBenchmark.java | 1 - 1 file changed, 1 deletion(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java index 07994d801776..7d0f543b62d6 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java @@ -40,7 +40,6 @@ import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.infra.Blackhole; -import java.io.IOException; import java.util.Collection; import java.util.Collections; import java.util.HashMap; From 555df63b756825f7010ab3a931eb02f1670ac144 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Thu, 12 Apr 2018 12:51:17 -0700 Subject: [PATCH 5/6] More checkstyle --- .../src/main/java/io/druid/benchmark/LoadStatusBenchmark.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java index 7d0f543b62d6..1cc8f238ded0 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import io.druid.java.util.common.Intervals; import io.druid.java.util.common.StringUtils; import io.druid.timeline.DataSegment; import io.druid.timeline.partition.NoneShardSpec; @@ -76,7 +77,7 @@ public void setup() for (int i = 0; i < totalSegmentsCount; i++) { DataSegment segment = new DataSegment( "benchmarkDatasource", - new Interval(StringUtils.format("%s-01-01/%s-12-31", i + 1970, i + 1970)), + Intervals.of(StringUtils.format("%s-01-01/%s-12-31", i + 1970, i + 1970)), "1", null, null, From c9c17751c7e6e9a0f089a1aafdca527f0ffcb318 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Thu, 12 Apr 2018 13:34:28 -0700 Subject: [PATCH 6/6] Checkstyle --- .../src/main/java/io/druid/benchmark/LoadStatusBenchmark.java | 1 - 1 file changed, 1 deletion(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java index 1cc8f238ded0..e1b877d481d8 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/LoadStatusBenchmark.java @@ -26,7 +26,6 @@ import io.druid.java.util.common.StringUtils; import io.druid.timeline.DataSegment; import io.druid.timeline.partition.NoneShardSpec; -import org.joda.time.Interval; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork;