From d916f9980066870d100567bbbf94b98dae5d8363 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 23 Feb 2015 23:24:28 +0100 Subject: [PATCH] The lock used during executing a fired alert should also encapsulate the updating of the fired alert. During stopping alerts we wait until all ongoing operations are completed, such as adding, deleting alerts and executing fired alerts. For fired alerts we should also let the update of fired alert happen under a lock. We could miss an ongoing operation if we would shutdown. This can be a reason why BasicAlertingTest.testDeleteAlert failed: https://build.elasticsearch.com/job/es_alerting/187/testReport/junit/org.elasticsearch.alerts/BasicAlertingTest/testDeleteAlert/ Original commit: elastic/x-pack-elasticsearch@732b213cf20b6d448c11968688f7b53f39045a0b --- .../alerts/history/HistoryService.java | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/elasticsearch/alerts/history/HistoryService.java b/src/main/java/org/elasticsearch/alerts/history/HistoryService.java index 7682e2da5cd..82b36879975 100644 --- a/src/main/java/org/elasticsearch/alerts/history/HistoryService.java +++ b/src/main/java/org/elasticsearch/alerts/history/HistoryService.java @@ -214,6 +214,7 @@ public class HistoryService extends AbstractComponent { if (!started.get()) { throw new ElasticsearchIllegalStateException("not started"); } + AlertLockService.Lock lock = alertLockService.acquire(alert.name()); try { firedAlert.update(FiredAlert.State.CHECKING, null); logger.debug("checking alert [{}]", firedAlert.name()); @@ -234,6 +235,8 @@ public class HistoryService extends AbstractComponent { } else { logger.debug("failed to execute fired alert [{}] after shutdown", e, firedAlert); } + } finally { + lock.release(); } } @@ -249,31 +252,24 @@ public class HistoryService extends AbstractComponent { */ AlertExecution execute(ExecutionContext ctx) throws IOException { - AlertLockService.Lock lock = alertLockService.acquire(alert.name()); - try { + Condition.Result conditionResult = alert.condition().execute(ctx); + ctx.onConditionResult(conditionResult); - Condition.Result conditionResult = alert.condition().execute(ctx); - ctx.onConditionResult(conditionResult); + if (conditionResult.met()) { + Throttler.Result throttleResult = alert.throttler().throttle(ctx, conditionResult); + ctx.onThrottleResult(throttleResult); - if (conditionResult.met()) { - Throttler.Result throttleResult = alert.throttler().throttle(ctx, conditionResult); - ctx.onThrottleResult(throttleResult); + if (!throttleResult.throttle()) { + Transform.Result result = alert.transform().apply(ctx, conditionResult.payload()); + ctx.onTransformResult(result); - if (!throttleResult.throttle()) { - Transform.Result result = alert.transform().apply(ctx, conditionResult.payload()); - ctx.onTransformResult(result); - - for (Action action : alert.actions()) { - Action.Result actionResult = action.execute(ctx, result.payload()); - ctx.onActionResult(actionResult); - } + for (Action action : alert.actions()) { + Action.Result actionResult = action.execute(ctx, result.payload()); + ctx.onActionResult(actionResult); } } - return ctx.finish(); - - } finally { - lock.release(); } + return ctx.finish(); } }