From cce62cbd8b3d5338f8ffa80d4a47d442822691ad Mon Sep 17 00:00:00 2001 From: fjy Date: Wed, 26 Jun 2013 17:27:04 -0700 Subject: [PATCH 1/5] The highly important commit of changing a log line --- server/src/main/java/com/metamx/druid/master/DruidMaster.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/metamx/druid/master/DruidMaster.java b/server/src/main/java/com/metamx/druid/master/DruidMaster.java index 13fe5a58c1e..36b0b0b1596 100644 --- a/server/src/main/java/com/metamx/druid/master/DruidMaster.java +++ b/server/src/main/java/com/metamx/druid/master/DruidMaster.java @@ -668,7 +668,7 @@ public class DruidMaster synchronized (lock) { final LeaderLatch latch = leaderLatch.get(); if (latch == null || !latch.hasLeadership()) { - log.info("[%s] is master, not me. Phooey.", latch == null ? null : latch.getLeader().getId()); + log.info("LEGGO MY EGGO. [%s] is master.", latch == null ? null : latch.getLeader().getId()); stopBeingMaster(); return; } From 5a57539736770af6e581622f48e0d2617f57dd1f Mon Sep 17 00:00:00 2001 From: fjy Date: Fri, 28 Jun 2013 10:18:10 -0700 Subject: [PATCH 2/5] fix hdfs config --- .../java/com/metamx/druid/loading/HdfsDataSegmentPusher.java | 2 +- .../com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java index 52ac15129d4..09fdf72f863 100644 --- a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java +++ b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java @@ -43,7 +43,7 @@ public class HdfsDataSegmentPusher implements DataSegmentPusher public DataSegment push(File inDir, DataSegment segment) throws IOException { final String storageDir = DataSegmentPusherUtil.getStorageDir(segment); - Path outFile = new Path(String.format("%s/%s/index.zip", config.getStorageDirectory(), storageDir)); + Path outFile = config.getStorageDirectory().suffix(String.format("/%s/index.zip", storageDir)); FileSystem fs = outFile.getFileSystem(hadoopConfig); fs.mkdirs(outFile.getParent()); diff --git a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java index c3f6d603ccb..70f3c979388 100644 --- a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java +++ b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java @@ -19,6 +19,7 @@ package com.metamx.druid.loading; +import org.apache.hadoop.fs.Path; import org.skife.config.Config; import java.io.File; @@ -28,5 +29,5 @@ import java.io.File; public abstract class HdfsDataSegmentPusherConfig { @Config("druid.pusher.hdfs.storageDirectory") - public abstract File getStorageDirectory(); + public abstract Path getStorageDirectory(); } From d02f152498750f213c38e35a41e0d5e2d6a3f55a Mon Sep 17 00:00:00 2001 From: fjy Date: Fri, 28 Jun 2013 14:57:32 -0700 Subject: [PATCH 3/5] fix NPE --- .../druid/curator/inventory/CuratorInventoryManager.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java b/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java index ab1e31bbc49..bfd75488be8 100644 --- a/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java +++ b/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java @@ -288,6 +288,10 @@ public class CuratorInventoryManager final String inventoryKey = ZKPaths.getNodeFromPath(child.getPath()); + if (inventoryKey == null) { + return; + } + switch (event.getType()) { case CHILD_ADDED: case CHILD_UPDATED: From f0b0d70ddaa1c326f8847e62366b803487d313f2 Mon Sep 17 00:00:00 2001 From: fjy Date: Mon, 1 Jul 2013 10:25:38 -0700 Subject: [PATCH 4/5] filter nulls in CEQR --- .../com/metamx/druid/query/ChainedExecutionQueryRunner.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/src/main/java/com/metamx/druid/query/ChainedExecutionQueryRunner.java b/client/src/main/java/com/metamx/druid/query/ChainedExecutionQueryRunner.java index 0dd878c7aad..8acc43a7585 100644 --- a/client/src/main/java/com/metamx/druid/query/ChainedExecutionQueryRunner.java +++ b/client/src/main/java/com/metamx/druid/query/ChainedExecutionQueryRunner.java @@ -20,6 +20,7 @@ package com.metamx.druid.query; import com.google.common.base.Function; +import com.google.common.base.Predicates; import com.google.common.base.Throwables; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -76,7 +77,7 @@ public class ChainedExecutionQueryRunner implements QueryRunner { this.exec = exec; this.ordering = ordering; - this.queryables = Iterables.unmodifiableIterable(queryables); + this.queryables = Iterables.unmodifiableIterable(Iterables.filter(queryables, Predicates.notNull())); } @Override From fde874ea568f640d768431f26e0e7a5f72f22ad3 Mon Sep 17 00:00:00 2001 From: fjy Date: Tue, 2 Jul 2013 16:11:12 -0700 Subject: [PATCH 5/5] fix according to code review --- .../curator/inventory/CuratorInventoryManager.java | 4 ---- .../druid/aggregation/CountAggregatorFactory.java | 2 +- .../druid/aggregation/DoubleSumAggregatorFactory.java | 4 ++-- .../druid/aggregation/HistogramAggregatorFactory.java | 4 ++-- .../druid/aggregation/JavaScriptAggregatorFactory.java | 10 +++++----- .../druid/aggregation/LongSumAggregatorFactory.java | 4 ++-- .../metamx/druid/aggregation/MaxAggregatorFactory.java | 4 ++-- .../metamx/druid/aggregation/MinAggregatorFactory.java | 4 ++-- .../metamx/druid/loading/HdfsDataSegmentPusher.java | 2 +- .../druid/loading/HdfsDataSegmentPusherConfig.java | 5 +---- 10 files changed, 18 insertions(+), 25 deletions(-) diff --git a/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java b/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java index bfd75488be8..ab1e31bbc49 100644 --- a/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java +++ b/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java @@ -288,10 +288,6 @@ public class CuratorInventoryManager final String inventoryKey = ZKPaths.getNodeFromPath(child.getPath()); - if (inventoryKey == null) { - return; - } - switch (event.getType()) { case CHILD_ADDED: case CHILD_UPDATED: diff --git a/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java index f4e2265745c..55900394860 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java @@ -41,7 +41,7 @@ public class CountAggregatorFactory implements AggregatorFactory @JsonProperty("name") String name ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); this.name = name; } diff --git a/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java index 0f47561d1d7..ec89a79f39d 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java @@ -45,8 +45,8 @@ public class DoubleSumAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, nonl null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java index 5bc4a40a420..6c65cd95f28 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java @@ -51,8 +51,8 @@ public class HistogramAggregatorFactory implements AggregatorFactory @JsonProperty("breaks") final List breaksList ) { - Preconditions.checkNotNull(name, "Must have a valid, nonl null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java index 7e4c1a66c6a..a8375f294fa 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java @@ -63,11 +63,11 @@ public class JavaScriptAggregatorFactory implements AggregatorFactory @JsonProperty("fnCombine") final String fnCombine ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); - Preconditions.checkNotNull(fieldNames, "Must have a valid, non null fieldNames"); - Preconditions.checkNotNull(fnAggregate, "Must have a valid, non null fnAggregate"); - Preconditions.checkNotNull(fnReset, "Must have a valid, non null fnReset"); - Preconditions.checkNotNull(fnCombine, "Must have a valid, non null fnCombine"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldNames, "Must have a valid, non-null fieldNames"); + Preconditions.checkNotNull(fnAggregate, "Must have a valid, non-null fnAggregate"); + Preconditions.checkNotNull(fnReset, "Must have a valid, non-null fnReset"); + Preconditions.checkNotNull(fnCombine, "Must have a valid, non-null fnCombine"); this.name = name; this.fieldNames = fieldNames; diff --git a/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java index 1aeb5e6471d..07e04254f76 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java @@ -45,8 +45,8 @@ public class LongSumAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, nonl null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java index dda25b355d7..45cd85257f7 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java @@ -45,8 +45,8 @@ public class MaxAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java index 5b92072be07..a6d19ebd8e5 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java @@ -45,8 +45,8 @@ public class MinAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java index 09fdf72f863..52ac15129d4 100644 --- a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java +++ b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java @@ -43,7 +43,7 @@ public class HdfsDataSegmentPusher implements DataSegmentPusher public DataSegment push(File inDir, DataSegment segment) throws IOException { final String storageDir = DataSegmentPusherUtil.getStorageDir(segment); - Path outFile = config.getStorageDirectory().suffix(String.format("/%s/index.zip", storageDir)); + Path outFile = new Path(String.format("%s/%s/index.zip", config.getStorageDirectory(), storageDir)); FileSystem fs = outFile.getFileSystem(hadoopConfig); fs.mkdirs(outFile.getParent()); diff --git a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java index 70f3c979388..b27d03672bc 100644 --- a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java +++ b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java @@ -19,15 +19,12 @@ package com.metamx.druid.loading; -import org.apache.hadoop.fs.Path; import org.skife.config.Config; -import java.io.File; - /** */ public abstract class HdfsDataSegmentPusherConfig { @Config("druid.pusher.hdfs.storageDirectory") - public abstract Path getStorageDirectory(); + public abstract String getStorageDirectory(); }