From aa69ed52511e9913231247ae71bdc56694eb0c92 Mon Sep 17 00:00:00 2001 From: Fangjin Yang Date: Fri, 14 Dec 2012 09:05:25 -0800 Subject: [PATCH] alert when no rules match; exceptions in rules no longer block --- .../com/metamx/druid/master/DruidMaster.java | 5 +- .../druid/master/DruidMasterRuleRunner.java | 5 +- .../metamx/druid/master/rules/LoadRule.java | 2 +- .../master/DruidMasterRuleRunnerTest.java | 57 ++++++++++++++++--- 4 files changed, 56 insertions(+), 13 deletions(-) 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 ee2549ac709..80172c432fa 100644 --- a/server/src/main/java/com/metamx/druid/master/DruidMaster.java +++ b/server/src/main/java/com/metamx/druid/master/DruidMaster.java @@ -44,6 +44,7 @@ import com.metamx.druid.client.ServerInventoryManager; import com.metamx.druid.coordination.DruidClusterInfo; import com.metamx.druid.db.DatabaseRuleManager; import com.metamx.druid.db.DatabaseSegmentManager; +import com.metamx.emitter.EmittingLogger; import com.metamx.emitter.service.ServiceEmitter; import com.metamx.emitter.service.ServiceMetricEvent; import com.metamx.phonebook.PhoneBook; @@ -69,7 +70,7 @@ public class DruidMaster { public static final String MASTER_OWNER_NODE = "_MASTER"; - private static final Logger log = new Logger(DruidMaster.class); + private static final EmittingLogger log = new EmittingLogger(DruidMaster.class); private final Object lock = new Object(); @@ -575,7 +576,7 @@ public class DruidMaster } } catch (Exception e) { - log.error(e, "Caught exception, ignoring so that schedule keeps going."); + log.makeAlert(e, "Caught exception, ignoring so that schedule keeps going.").emit(); } } } diff --git a/server/src/main/java/com/metamx/druid/master/DruidMasterRuleRunner.java b/server/src/main/java/com/metamx/druid/master/DruidMasterRuleRunner.java index e33701c66cd..87f16a76f8a 100644 --- a/server/src/main/java/com/metamx/druid/master/DruidMasterRuleRunner.java +++ b/server/src/main/java/com/metamx/druid/master/DruidMasterRuleRunner.java @@ -25,6 +25,7 @@ import com.metamx.druid.client.DataSegment; import com.metamx.druid.db.DatabaseRuleManager; import com.metamx.druid.master.rules.Rule; import com.metamx.druid.master.rules.RuleMap; +import com.metamx.emitter.EmittingLogger; import java.util.List; @@ -32,7 +33,7 @@ import java.util.List; */ public class DruidMasterRuleRunner implements DruidMasterHelper { - private static final Logger log = new Logger(DruidMasterRuleRunner.class); + private static final EmittingLogger log = new EmittingLogger(DruidMasterRuleRunner.class); private final DruidMaster master; @@ -67,7 +68,7 @@ public class DruidMasterRuleRunner implements DruidMasterHelper } if (!foundMatchingRule) { - throw new ISE("Unable to find a matching rule for segment[%s]", segment.getIdentifier()); + log.makeAlert("Unable to find a matching rule for segment[%s]", segment.getIdentifier()).emit(); } } diff --git a/server/src/main/java/com/metamx/druid/master/rules/LoadRule.java b/server/src/main/java/com/metamx/druid/master/rules/LoadRule.java index 82e7a3acf88..b3d256d2066 100644 --- a/server/src/main/java/com/metamx/druid/master/rules/LoadRule.java +++ b/server/src/main/java/com/metamx/druid/master/rules/LoadRule.java @@ -52,7 +52,7 @@ public abstract class LoadRule implements Rule MinMaxPriorityQueue serverQueue = params.getDruidCluster().getServersByTier(getTier()); if (serverQueue == null) { log.makeAlert("Tier[%s] has no servers! Check your cluster configuration!", getTier()).emit(); - throw new ISE("Tier[%s] has no servers! Check your cluster configuration!", getTier()); + return stats; } stats.accumulate(assign(expectedReplicants, actualReplicants, serverQueue, segment)); diff --git a/server/src/test/java/com/metamx/druid/master/DruidMasterRuleRunnerTest.java b/server/src/test/java/com/metamx/druid/master/DruidMasterRuleRunnerTest.java index 66b928f0bde..5076ffbefe1 100644 --- a/server/src/test/java/com/metamx/druid/master/DruidMasterRuleRunnerTest.java +++ b/server/src/test/java/com/metamx/druid/master/DruidMasterRuleRunnerTest.java @@ -397,20 +397,61 @@ public class DruidMasterRuleRunnerTest .withSegmentReplicantLookup(SegmentReplicantLookup.make(new DruidCluster())) .build(); - boolean exceptionOccurred = false; - try { - ruleRunner.run(params); - } - catch (Exception e) { - exceptionOccurred = true; - } - Assert.assertTrue(exceptionOccurred); + ruleRunner.run(params); EasyMock.verify(emitter); EasyMock.verify(mockPeon); } + @Test + public void testRunRuleDoesNotExist() throws Exception + { + emitter.emit(EasyMock.anyObject()); + EasyMock.expectLastCall().atLeastOnce(); + EasyMock.replay(emitter); + + EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( + Lists.newArrayList( + new IntervalLoadRule(new Interval("2012-01-02T00:00:00.000Z/2012-01-03T00:00:00.000Z"), 1, "normal") + ) + ).atLeastOnce(); + EasyMock.replay(databaseRuleManager); + + DruidCluster druidCluster = new DruidCluster( + ImmutableMap.of( + "normal", + MinMaxPriorityQueue.orderedBy(Ordering.natural().reverse()).create( + Arrays.asList( + new ServerHolder( + new DruidServer( + "serverNorm", + "hostNorm", + 1000, + "historical", + "normal" + ), + mockPeon + ) + ) + ) + ) + ); + + DruidMasterRuntimeParams params = + new DruidMasterRuntimeParams.Builder() + .withEmitter(emitter) + .withDruidCluster(druidCluster) + .withAvailableSegments(availableSegments) + .withDatabaseRuleManager(databaseRuleManager) + .withSegmentReplicantLookup(SegmentReplicantLookup.make(new DruidCluster())) + .build(); + + ruleRunner.run(params); + + EasyMock.verify(emitter); + } + @Test public void testDropRemove() throws Exception {