From 0071f1dcaa42e48cea8f7f170e8bc36dc84012d2 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Wed, 18 Apr 2018 13:09:25 -0700 Subject: [PATCH 1/5] Fix coordinator's dataSource api with full parameter --- .../java/io/druid/client/DruidDataSource.java | 1 + .../client/ImmutableDruidDataSource.java | 26 +++++++- .../server/http/DatasourcesResource.java | 2 +- .../client/ImmutableDruidDataSourceTest.java | 63 +++++++++++++++++++ 4 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java diff --git a/server/src/main/java/io/druid/client/DruidDataSource.java b/server/src/main/java/io/druid/client/DruidDataSource.java index 281d51223a76..34657b8d4f30 100644 --- a/server/src/main/java/io/druid/client/DruidDataSource.java +++ b/server/src/main/java/io/druid/client/DruidDataSource.java @@ -102,6 +102,7 @@ public String toString() @Override public boolean equals(Object o) { + //noinspection Contract throw new UnsupportedOperationException("Use ImmutableDruidDataSource instead"); } diff --git a/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java b/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java index b08f9862ae14..343d0065e239 100644 --- a/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java +++ b/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java @@ -19,11 +19,15 @@ package io.druid.client; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import io.druid.timeline.DataSegment; import java.util.Collection; +import java.util.Map; import java.util.Objects; /** @@ -34,10 +38,11 @@ public class ImmutableDruidDataSource private final ImmutableMap properties; private final ImmutableMap idToSegments; + @JsonCreator public ImmutableDruidDataSource( - String name, - ImmutableMap properties, - ImmutableMap idToSegments + @JsonProperty("name") String name, + @JsonProperty("properties") ImmutableMap properties, + @JsonProperty("idToSegments") ImmutableMap idToSegments ) { this.name = Preconditions.checkNotNull(name); @@ -45,16 +50,31 @@ public ImmutableDruidDataSource( this.idToSegments = idToSegments; } + @JsonProperty public String getName() { return name; } + @JsonProperty + public Map getProperties() + { + return properties; + } + + @JsonProperty + public Map getIdToSegments() + { + return idToSegments; + } + + @JsonIgnore public Collection getSegments() { return idToSegments.values(); } + @JsonIgnore public DataSegment getSegment(String segmentIdentifier) { return idToSegments.get(segmentIdentifier); diff --git a/server/src/main/java/io/druid/server/http/DatasourcesResource.java b/server/src/main/java/io/druid/server/http/DatasourcesResource.java index c53e37158fbd..a358b8d329f3 100644 --- a/server/src/main/java/io/druid/server/http/DatasourcesResource.java +++ b/server/src/main/java/io/druid/server/http/DatasourcesResource.java @@ -143,7 +143,7 @@ public Response getTheDataSource( @QueryParam("full") final String full ) { - ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); + final ImmutableDruidDataSource dataSource = getDataSource(dataSourceName); if (dataSource == null) { return Response.noContent().build(); diff --git a/server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java b/server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java new file mode 100644 index 000000000000..b549c585e758 --- /dev/null +++ b/server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java @@ -0,0 +1,63 @@ +/* + * 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.client; + +import com.fasterxml.jackson.databind.InjectableValues.Std; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.druid.jackson.DefaultObjectMapper; +import io.druid.java.util.common.Intervals; +import io.druid.timeline.DataSegment; +import io.druid.timeline.DataSegment.PruneLoadSpecHolder; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; + +public class ImmutableDruidDataSourceTest +{ + @Test + public void testSerde() throws IOException + { + final DataSegment segment = new DataSegment( + "test", + Intervals.of("2017/2018"), + "version", + null, + ImmutableList.of("dim1", "dim2"), + ImmutableList.of("met1", "met2"), + null, + 1, + 100L, + PruneLoadSpecHolder.DEFAULT + ); + final ImmutableDruidDataSource dataSource = new ImmutableDruidDataSource( + "test", + ImmutableMap.of("prop1", "val1", "prop2", "val2"), + ImmutableMap.of(segment.getIdentifier(), segment) + ); + + final ObjectMapper objectMapper = new DefaultObjectMapper() + .setInjectableValues(new Std().addValue(PruneLoadSpecHolder.class, PruneLoadSpecHolder.DEFAULT)); + final String json = objectMapper.writeValueAsString(dataSource); + Assert.assertEquals(dataSource, objectMapper.readValue(json, ImmutableDruidDataSource.class)); + } +} From f0fbd6d4edfe6d9766fac41dad7260f3775565a1 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Wed, 18 Apr 2018 18:06:26 -0700 Subject: [PATCH 2/5] address comment --- .../java/io/druid/server/http/DatasourcesResource.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/io/druid/server/http/DatasourcesResource.java b/server/src/main/java/io/druid/server/http/DatasourcesResource.java index a358b8d329f3..170f10652249 100644 --- a/server/src/main/java/io/druid/server/http/DatasourcesResource.java +++ b/server/src/main/java/io/druid/server/http/DatasourcesResource.java @@ -150,7 +150,15 @@ public Response getTheDataSource( } if (full != null) { - return Response.ok(dataSource).build(); + final Map fullResult = ImmutableMap.of( + "name", + dataSource.getName(), + "properties", + dataSource.getProperties(), + "segments", + dataSource.getIdToSegments().values() + ); + return Response.ok(fullResult).build(); } return Response.ok(getSimpleDatasource(dataSourceName)).build(); From 966fd51adfbe706832417052989806818832cc75 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 19 Apr 2018 11:33:28 -0700 Subject: [PATCH 3/5] Add a constructor for json serde and fix result order --- .../java/io/druid/client/DruidDataSource.java | 13 +++----- .../client/ImmutableDruidDataSource.java | 31 ++++++++++++------- .../server/http/DatasourcesResource.java | 21 ++++--------- .../client/ImmutableDruidDataSourceTest.java | 3 +- .../server/coordinator/DruidClusterTest.java | 9 +++--- .../server/coordinator/ServerHolderTest.java | 10 +++--- .../server/http/DatasourcesResourceTest.java | 2 +- 7 files changed, 44 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/io/druid/client/DruidDataSource.java b/server/src/main/java/io/druid/client/DruidDataSource.java index 34657b8d4f30..1308c43a3b58 100644 --- a/server/src/main/java/io/druid/client/DruidDataSource.java +++ b/server/src/main/java/io/druid/client/DruidDataSource.java @@ -21,13 +21,12 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; import io.druid.timeline.DataSegment; import java.util.Collection; import java.util.Collections; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentSkipListMap; /** */ @@ -35,7 +34,7 @@ public class DruidDataSource { private final String name; private final Map properties; - private final ConcurrentHashMap idToSegmentMap; + private final ConcurrentSkipListMap idToSegmentMap; public DruidDataSource( String name, @@ -44,7 +43,7 @@ public DruidDataSource( { this.name = Preconditions.checkNotNull(name); this.properties = properties; - this.idToSegmentMap = new ConcurrentHashMap<>(); + this.idToSegmentMap = new ConcurrentSkipListMap<>(); } @JsonProperty @@ -83,11 +82,7 @@ public boolean isEmpty() public ImmutableDruidDataSource toImmutableDruidDataSource() { - return new ImmutableDruidDataSource( - name, - ImmutableMap.copyOf(properties), - ImmutableMap.copyOf(idToSegmentMap) - ); + return new ImmutableDruidDataSource(name, properties, idToSegmentMap); } @Override diff --git a/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java b/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java index 343d0065e239..3431115c2b64 100644 --- a/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java +++ b/server/src/main/java/io/druid/client/ImmutableDruidDataSource.java @@ -24,11 +24,13 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSortedMap; import io.druid.timeline.DataSegment; import java.util.Collection; import java.util.Map; import java.util.Objects; +import java.util.SortedMap; /** */ @@ -36,18 +38,31 @@ public class ImmutableDruidDataSource { private final String name; private final ImmutableMap properties; - private final ImmutableMap idToSegments; + private final ImmutableSortedMap idToSegments; + + public ImmutableDruidDataSource( + String name, + Map properties, + SortedMap idToSegments + ) + { + this.name = Preconditions.checkNotNull(name); + this.properties = ImmutableMap.copyOf(properties); + this.idToSegments = ImmutableSortedMap.copyOfSorted(idToSegments); + } @JsonCreator public ImmutableDruidDataSource( @JsonProperty("name") String name, - @JsonProperty("properties") ImmutableMap properties, - @JsonProperty("idToSegments") ImmutableMap idToSegments + @JsonProperty("properties") Map properties, + @JsonProperty("segments") Collection segments ) { this.name = Preconditions.checkNotNull(name); - this.properties = properties; - this.idToSegments = idToSegments; + this.properties = ImmutableMap.copyOf(properties); + final ImmutableSortedMap.Builder builder = ImmutableSortedMap.naturalOrder(); + segments.forEach(segment -> builder.put(segment.getIdentifier(), segment)); + this.idToSegments = builder.build(); } @JsonProperty @@ -63,12 +78,6 @@ public Map getProperties() } @JsonProperty - public Map getIdToSegments() - { - return idToSegments; - } - - @JsonIgnore public Collection getSegments() { return idToSegments.values(); diff --git a/server/src/main/java/io/druid/server/http/DatasourcesResource.java b/server/src/main/java/io/druid/server/http/DatasourcesResource.java index 170f10652249..2e4003321473 100644 --- a/server/src/main/java/io/druid/server/http/DatasourcesResource.java +++ b/server/src/main/java/io/druid/server/http/DatasourcesResource.java @@ -65,12 +65,15 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import java.util.Collections; import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.stream.Collectors; /** @@ -150,15 +153,7 @@ public Response getTheDataSource( } if (full != null) { - final Map fullResult = ImmutableMap.of( - "name", - dataSource.getName(), - "properties", - dataSource.getProperties(), - "segments", - dataSource.getIdToSegments().values() - ); - return Response.ok(fullResult).build(); + return Response.ok(dataSource).build(); } return Response.ok(getSimpleDatasource(dataSourceName)).build(); @@ -516,7 +511,7 @@ private ImmutableDruidDataSource getDataSource(final String dataSourceName) return null; } - Map segmentMap = Maps.newHashMap(); + SortedMap segmentMap = new TreeMap<>(); for (ImmutableDruidDataSource dataSource : dataSources) { Iterable segments = dataSource.getSegments(); for (DataSegment segment : segments) { @@ -524,11 +519,7 @@ private ImmutableDruidDataSource getDataSource(final String dataSourceName) } } - return new ImmutableDruidDataSource( - dataSourceName, - ImmutableMap.of(), - ImmutableMap.copyOf(segmentMap) - ); + return new ImmutableDruidDataSource(dataSourceName, Collections.emptyMap(), segmentMap); } private Pair> getSegment(String segmentId) diff --git a/server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java b/server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java index b549c585e758..ad101dbeea31 100644 --- a/server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java +++ b/server/src/test/java/io/druid/client/ImmutableDruidDataSourceTest.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSortedMap; import io.druid.jackson.DefaultObjectMapper; import io.druid.java.util.common.Intervals; import io.druid.timeline.DataSegment; @@ -52,7 +53,7 @@ public void testSerde() throws IOException final ImmutableDruidDataSource dataSource = new ImmutableDruidDataSource( "test", ImmutableMap.of("prop1", "val1", "prop2", "val2"), - ImmutableMap.of(segment.getIdentifier(), segment) + ImmutableSortedMap.of(segment.getIdentifier(), segment) ); final ObjectMapper objectMapper = new DefaultObjectMapper() diff --git a/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java b/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java index 90eb5d49e305..933c61e060b0 100644 --- a/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java +++ b/server/src/test/java/io/druid/server/coordinator/DruidClusterTest.java @@ -39,6 +39,7 @@ import java.util.Map; import java.util.NavigableSet; import java.util.Set; +import java.util.TreeMap; import java.util.TreeSet; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -74,14 +75,14 @@ public class DruidClusterTest "src1", new ImmutableDruidDataSource( "src1", - ImmutableMap.of(), - ImmutableMap.of() + Collections.emptyMap(), + new TreeMap<>() ), "src2", new ImmutableDruidDataSource( "src2", - ImmutableMap.of(), - ImmutableMap.of() + Collections.emptyMap(), + new TreeMap<>() ) ); diff --git a/server/src/test/java/io/druid/server/coordinator/ServerHolderTest.java b/server/src/test/java/io/druid/server/coordinator/ServerHolderTest.java index 669bf10cc602..23da9c34a189 100644 --- a/server/src/test/java/io/druid/server/coordinator/ServerHolderTest.java +++ b/server/src/test/java/io/druid/server/coordinator/ServerHolderTest.java @@ -31,8 +31,10 @@ import org.junit.Assert; import org.junit.Test; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.TreeMap; public class ServerHolderTest { @@ -65,14 +67,14 @@ public class ServerHolderTest "src1", new ImmutableDruidDataSource( "src1", - ImmutableMap.of(), - ImmutableMap.of() + Collections.emptyMap(), + new TreeMap<>() ), "src2", new ImmutableDruidDataSource( "src2", - ImmutableMap.of(), - ImmutableMap.of() + Collections.emptyMap(), + new TreeMap<>() ) ); diff --git a/server/src/test/java/io/druid/server/http/DatasourcesResourceTest.java b/server/src/test/java/io/druid/server/http/DatasourcesResourceTest.java index 1c3e31bd2709..1eb0455e5c24 100644 --- a/server/src/test/java/io/druid/server/http/DatasourcesResourceTest.java +++ b/server/src/test/java/io/druid/server/http/DatasourcesResourceTest.java @@ -314,7 +314,7 @@ public void testGetSimpleQueryableDataSources() @Test public void testFullGetTheDataSource() { - DruidDataSource dataSource1 = new DruidDataSource("datasource1", new HashMap()); + DruidDataSource dataSource1 = new DruidDataSource("datasource1", new HashMap<>()); EasyMock.expect(server.getDataSource("datasource1")).andReturn( dataSource1 ).atLeastOnce(); From 891a75a90d26b09d4d3aaeaac7fc3e524bcfbe43 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 19 Apr 2018 11:52:56 -0700 Subject: [PATCH 4/5] Change to immutableSortedMap --- .../java/io/druid/server/http/DatasourcesResource.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/io/druid/server/http/DatasourcesResource.java b/server/src/main/java/io/druid/server/http/DatasourcesResource.java index 2e4003321473..4ffc19ef3813 100644 --- a/server/src/main/java/io/druid/server/http/DatasourcesResource.java +++ b/server/src/main/java/io/druid/server/http/DatasourcesResource.java @@ -20,6 +20,7 @@ package io.druid.server.http; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -72,8 +73,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.SortedMap; -import java.util.TreeMap; import java.util.stream.Collectors; /** @@ -511,15 +510,15 @@ private ImmutableDruidDataSource getDataSource(final String dataSourceName) return null; } - SortedMap segmentMap = new TreeMap<>(); + final ImmutableSortedMap.Builder builder = ImmutableSortedMap.naturalOrder(); for (ImmutableDruidDataSource dataSource : dataSources) { Iterable segments = dataSource.getSegments(); for (DataSegment segment : segments) { - segmentMap.put(segment.getIdentifier(), segment); + builder.put(segment.getIdentifier(), segment); } } - return new ImmutableDruidDataSource(dataSourceName, Collections.emptyMap(), segmentMap); + return new ImmutableDruidDataSource(dataSourceName, Collections.emptyMap(), builder.build()); } private Pair> getSegment(String segmentId) From a4005493788ef12d188a0265ff5e2b40c3ea03b5 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 19 Apr 2018 13:45:21 -0700 Subject: [PATCH 5/5] Revert immutableSortedMap to treeMap --- .../java/io/druid/server/http/DatasourcesResource.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/io/druid/server/http/DatasourcesResource.java b/server/src/main/java/io/druid/server/http/DatasourcesResource.java index 4ffc19ef3813..a763c613cc56 100644 --- a/server/src/main/java/io/druid/server/http/DatasourcesResource.java +++ b/server/src/main/java/io/druid/server/http/DatasourcesResource.java @@ -20,7 +20,6 @@ package io.druid.server.http; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -73,6 +72,8 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.stream.Collectors; /** @@ -510,15 +511,15 @@ private ImmutableDruidDataSource getDataSource(final String dataSourceName) return null; } - final ImmutableSortedMap.Builder builder = ImmutableSortedMap.naturalOrder(); + final SortedMap segmentMap = new TreeMap<>(); for (ImmutableDruidDataSource dataSource : dataSources) { Iterable segments = dataSource.getSegments(); for (DataSegment segment : segments) { - builder.put(segment.getIdentifier(), segment); + segmentMap.put(segment.getIdentifier(), segment); } } - return new ImmutableDruidDataSource(dataSourceName, Collections.emptyMap(), builder.build()); + return new ImmutableDruidDataSource(dataSourceName, Collections.emptyMap(), segmentMap); } private Pair> getSegment(String segmentId)