Merge pull request apache#2821 from metamx/review-comments-2784

handle review comments for PR 2784
cynepiaadmin · Apr 12, 2016 · 886ee4e · 886ee4e
2 parents b486eff + deb6ecf
commit 886ee4e
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 52 deletions.
diff --git a/docs/content/configuration/index.md b/docs/content/configuration/index.md
@@ -296,7 +296,6 @@ In current Druid, multiple data segments may be announced under the same Znode.
 |--------|-----------|-------|
 |`druid.announcer.segmentsPerNode`|Each Znode contains info for up to this many segments.|50|
 |`druid.announcer.maxBytesPerNode`|Max byte size for Znode.|524288|
-|`druid.announcer.skipDimensions`|Skip Dimension list from segment announcements. NOTE: Enabling this will also remove the dimensions list from coordinator and broker endpoints.|false|
-|`druid.announcer.skipMetrics`|Skip Metrics list from segment announcements. NOTE: Enabling this will also remove the metrics list from coordinator and broker endpoints.|false|
+|`druid.announcer.skipDimensionsAndMetrics`|Skip Dimensions and Metrics list from segment announcements. NOTE: Enabling this will also remove the dimensions and metrics list from coordinator and broker endpoints.|false|
 |`druid.announcer.skipLoadSpec`|Skip segment LoadSpec from segment announcements. NOTE: Enabling this will also remove the loadspec from coordinator and broker endpoints.|false|
 
diff --git a/server/src/main/java/io/druid/server/coordination/BatchDataSegmentAnnouncer.java b/server/src/main/java/io/druid/server/coordination/BatchDataSegmentAnnouncer.java
@@ -85,11 +85,8 @@ public BatchDataSegmentAnnouncer(
       public DataSegment apply(DataSegment input)
       {
         DataSegment rv = input;
-        if (config.isSkipDimensions()) {
-          rv = rv.withDimensions(null);
-        }
-        if (config.isSkipMetrics()) {
-          rv = rv.withMetrics(null);
+        if (config.isSkipDimensionsAndMetrics()) {
+          rv = rv.withDimensions(null).withMetrics(null);
         }
         if (config.isSkipLoadSpec()) {
           rv = rv.withLoadSpec(null);

diff --git a/server/src/main/java/io/druid/server/initialization/BatchDataSegmentAnnouncerConfig.java b/server/src/main/java/io/druid/server/initialization/BatchDataSegmentAnnouncerConfig.java
@@ -43,11 +43,7 @@ public class BatchDataSegmentAnnouncerConfig
 
   // Skip dimension list from segment announcements
   @JsonProperty
-  private boolean skipDimensions = false;
-
-  // Skip metrics list from segment announcements
-  @JsonProperty
-  private boolean skipMetrics = false;
+  private boolean skipDimensionsAndMetrics = false;
 
   public int getSegmentsPerNode()
   {
@@ -64,13 +60,9 @@ public boolean isSkipLoadSpec()
     return skipLoadSpec;
   }
 
-  public boolean isSkipDimensions()
+  public boolean isSkipDimensionsAndMetrics()
   {
-    return skipDimensions;
+    return skipDimensionsAndMetrics;
   }
 
-  public boolean isSkipMetrics()
-  {
-    return skipMetrics;
-  }
 }
diff --git a/...rc/test/java/io/druid/server/coordination/coordination/BatchDataSegmentAnnouncerTest.java b/...rc/test/java/io/druid/server/coordination/coordination/BatchDataSegmentAnnouncerTest.java
@@ -69,8 +69,7 @@ public class BatchDataSegmentAnnouncerTest
   private Set<DataSegment> testSegments;
 
   private final AtomicInteger maxBytesPerNode = new AtomicInteger(512 * 1024);
-  private Boolean skipDimensions;
-  private Boolean skipMetrics;
+  private Boolean skipDimensionsAndMetrics;
   private Boolean skipLoadSpec;
 
 
@@ -98,8 +97,7 @@ public void setUp() throws Exception
     announcer.start();
 
     segmentReader = new SegmentReader(cf, jsonMapper);
-    skipDimensions = false;
-    skipMetrics = false;
+    skipDimensionsAndMetrics = false;
     skipLoadSpec = false;
     segmentAnnouncer = new BatchDataSegmentAnnouncer(
         new DruidServerMetadata(
@@ -125,15 +123,9 @@ public long getMaxBytesPerNode()
           }
 
           @Override
-          public boolean isSkipDimensions()
+          public boolean isSkipDimensionsAndMetrics()
           {
-            return skipDimensions;
-          }
-
-          @Override
-          public boolean isSkipMetrics()
-          {
-            return skipMetrics;
+            return skipDimensionsAndMetrics;
           }
 
           @Override
@@ -208,7 +200,7 @@ public void testSingleAnnounce() throws Exception
   @Test
   public void testSkipDimensions() throws Exception
   {
-    skipDimensions = true;
+    skipDimensionsAndMetrics = true;
     Iterator<DataSegment> segIter = testSegments.iterator();
     DataSegment firstSegment = segIter.next();
 
@@ -220,27 +212,6 @@ public void testSkipDimensions() throws Exception
       DataSegment announcedSegment = Iterables.getOnlyElement(segmentReader.read(joiner.join(testSegmentsPath, zNode)));
       Assert.assertEquals(announcedSegment, firstSegment);
       Assert.assertTrue(announcedSegment.getDimensions().isEmpty());
-    }
-
-    segmentAnnouncer.unannounceSegment(firstSegment);
-
-    Assert.assertTrue(cf.getChildren().forPath(testSegmentsPath).isEmpty());
-  }
-
-  @Test
-  public void testSkipMetrics() throws Exception
-  {
-    skipMetrics = true;
-    Iterator<DataSegment> segIter = testSegments.iterator();
-    DataSegment firstSegment = segIter.next();
-
-    segmentAnnouncer.announceSegment(firstSegment);
-
-    List<String> zNodes = cf.getChildren().forPath(testSegmentsPath);
-
-    for (String zNode : zNodes) {
-      DataSegment announcedSegment = Iterables.getOnlyElement(segmentReader.read(joiner.join(testSegmentsPath, zNode)));
-      Assert.assertEquals(announcedSegment, firstSegment);
       Assert.assertTrue(announcedSegment.getMetrics().isEmpty());
     }