Core: Make sure deletes are unscheduled and no npe is returned when a alert is deleted twice

Core: When an alert searches in an index that doesn't exist it shouldn't fail

Original commit: elastic/x-pack-elasticsearch@fc4ab8f823
This commit is contained in:
Martijn van Groningen 2014-11-08 02:13:59 +01:00
parent 4b089cb157
commit b62da0691b
5 changed files with 54 additions and 12 deletions

View File

@ -73,12 +73,11 @@ public class AlertManager extends AbstractComponent {
public DeleteResponse deleteAlert(String name) throws InterruptedException, ExecutionException {
ensureStarted();
if (alertsStore.hasAlert(name)) {
assert scheduler.remove(name);
return alertsStore.deleteAlert(name);
} else {
return null;
DeleteResponse deleteResponse = alertsStore.deleteAlert(name);
if (deleteResponse.isFound()) {
scheduler.remove(name);
}
return deleteResponse;
}
public IndexResponse addAlert(String alertName, BytesReference alertSource) {

View File

@ -13,6 +13,7 @@ import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.alerts.actions.AlertAction;
import org.elasticsearch.alerts.actions.AlertActionRegistry;
import org.elasticsearch.alerts.triggers.TriggerManager;
@ -26,6 +27,7 @@ import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.BytesStreamInput;
import org.elasticsearch.common.joda.time.DateTime;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
@ -131,7 +133,7 @@ public class AlertsStore extends AbstractComponent {
public DeleteResponse deleteAlert(String name) {
Alert alert = alertMap.remove(name);
if (alert == null) {
return null;
return new DeleteResponse(ALERT_INDEX, ALERT_TYPE, name, Versions.MATCH_ANY, false);
}
DeleteResponse deleteResponse = client.prepareDelete(ALERT_INDEX, ALERT_TYPE, name)
@ -269,6 +271,7 @@ public class AlertsStore extends AbstractComponent {
} else if (REQUEST_FIELD.match(currentFieldName)) {
String searchRequestFieldName = null;
SearchRequest searchRequest = new SearchRequest();
searchRequest.indicesOptions(IndicesOptions.lenientExpandOpen()); // TODO: make options configurable
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
searchRequestFieldName = parser.currentName();

View File

@ -30,16 +30,11 @@ public class DeleteAlertResponse extends ActionResponse {
return deleteResponse;
}
public void deleteResponse(DeleteResponse deleteResponse){
this.deleteResponse = deleteResponse;
}
@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
deleteResponse = new DeleteResponse();
if (in.readBoolean()) {
deleteResponse = new DeleteResponse();
deleteResponse.readFrom(in);
}
}

View File

@ -113,4 +113,26 @@ public abstract class AbstractAlertingTests extends ElasticsearchIntegrationTest
});
}
protected void assertNoAlertTrigger(final String alertName) throws Exception {
assertBusy(new Runnable() {
@Override
public void run() {
// The alerthistory index gets created in the background when the first alert fires, so we to check first is this index is created and shards are started
IndicesExistsResponse indicesExistsResponse = client().admin().indices().prepareExists(AlertActionManager.ALERT_HISTORY_INDEX).get();
assertThat(indicesExistsResponse.isExists(), is(true));
ClusterState state = client().admin().cluster().prepareState().get().getState();
IndexRoutingTable routingTable = state.getRoutingTable().index(AlertActionManager.ALERT_HISTORY_INDEX);
assertThat(routingTable, notNullValue());
assertThat(routingTable.allPrimaryShardsActive(), is(true));
SearchResponse searchResponse = client().prepareSearch(AlertActionManager.ALERT_HISTORY_INDEX)
.setIndicesOptions(IndicesOptions.lenientExpandOpen())
.setQuery(boolQuery().must(matchQuery("alert_name", alertName)).must(matchQuery("state", AlertActionState.NO_ACTION_NEEDED.toString())))
.setSize(1)
.get();
assertThat(searchResponse.getHits().getHits().length, equalTo(1));
}
});
}
}

View File

@ -39,6 +39,23 @@ public class BasicAlertingTest extends AbstractAlertingTests {
assertAlertTriggered("my-first-alert");
}
@Test
public void testIndexAlert_registerAlertBeforeTargetIndex() throws Exception {
AlertsClientInterface alertsClient = alertClient();
SearchRequest searchRequest = new SearchRequest("my-index").source(searchSource().query(termQuery("field", "value")));
BytesReference alertSource = createAlertSource("0/5 * * * * ? *", searchRequest, "hits.total == 1");
alertsClient.prepareIndexAlert("my-first-alert")
.setAlertSource(alertSource)
.get();
// The alert can't trigger because there is no data that matches with the query
assertNoAlertTrigger("my-first-alert");
// Index sample doc after we register the alert and the alert should get triggered
client().prepareIndex("my-index", "my-type").setSource("field", "value").get();
assertAlertTriggered("my-first-alert");
}
@Test
public void testDeleteAlert() throws Exception {
AlertsClientInterface alertsClient = alertClient();
@ -59,6 +76,12 @@ public class BasicAlertingTest extends AbstractAlertingTests {
refresh();
assertHitCount(client().prepareCount(AlertsStore.ALERT_INDEX).get(), 0l);
// Deleting the same alert for the second time
deleteAlertRequest = new DeleteAlertRequest("my-first-alert");
deleteAlertResponse = alertsClient.deleteAlert(deleteAlertRequest).actionGet();
assertNotNull(deleteAlertResponse.deleteResponse());
assertFalse(deleteAlertResponse.deleteResponse().isFound());
}
}