Skip to content

Commit 1b1505b

Browse files
author
Hendrik Muhs
committed
apply review suggestions
1 parent 6df1d70 commit 1b1505b

File tree

3 files changed

+162
-15
lines changed

3 files changed

+162
-15
lines changed

‎x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformIndexer.java

+13-12
Original file line numberDiff line numberDiff line change
@@ -509,18 +509,19 @@ synchronized void handleFailure(Exception e) {
509509
+ " failures; latest failure: "
510510
+ ExceptionRootCauseFinder.getDetailedMessage(unwrappedException)
511511
);
512-
} else {
513-
// Since our schedule fires again very quickly after failures it is possible to run into the same failure numerous
514-
// times in a row, very quickly. We do not want to spam the audit log with repeated failures, so only record the first one
515-
if (e.getMessage().equals(lastAuditedExceptionMessage) == false) {
516-
String message = ExceptionRootCauseFinder.getDetailedMessage(unwrappedException);
517-
518-
auditor.warning(
519-
getJobId(),
520-
"Transform encountered an exception: " + message + " Will attempt again at next scheduled trigger."
521-
);
522-
lastAuditedExceptionMessage = message;
523-
}
512+
return;
513+
}
514+
515+
// Since our schedule fires again very quickly after failures it is possible to run into the same failure numerous
516+
// times in a row, very quickly. We do not want to spam the audit log with repeated failures, so only record the first one
517+
if (e.getMessage().equals(lastAuditedExceptionMessage) == false) {
518+
String message = ExceptionRootCauseFinder.getDetailedMessage(unwrappedException);
519+
520+
auditor.warning(
521+
getJobId(),
522+
"Transform encountered an exception: " + message + " Will attempt again at next scheduled trigger."
523+
);
524+
lastAuditedExceptionMessage = message;
524525
}
525526
}
526527

‎x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/utils/ExceptionRootCauseFinder.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public final class ExceptionRootCauseFinder {
2525
/**
2626
* List of rest statuses that we consider irrecoverable
2727
*/
28-
public static Set<RestStatus> IRRECOVERABLE_REST_STATUSES = new HashSet<>(
28+
public static final Set<RestStatus> IRRECOVERABLE_REST_STATUSES = new HashSet<>(
2929
Arrays.asList(
3030
RestStatus.GONE,
3131
RestStatus.NOT_IMPLEMENTED,
@@ -79,7 +79,7 @@ public static String getDetailedMessage(Throwable t) {
7979
/**
8080
* Return the first irrecoverableException from a collection of bulk responses if there are any.
8181
*
82-
* @param failures a collection of bulk item responses
82+
* @param failures a collection of bulk item responses with failures
8383
* @return The first exception considered irrecoverable if there are any, null if no irrecoverable exception found
8484
*/
8585
public static Throwable getFirstIrrecoverableExceptionFromBulkResponses(Collection<BulkItemResponse> failures) {
@@ -91,7 +91,7 @@ public static Throwable getFirstIrrecoverableExceptionFromBulkResponses(Collecti
9191

9292
if (unwrappedThrowable instanceof ElasticsearchException) {
9393
ElasticsearchException elasticsearchException = (ElasticsearchException) unwrappedThrowable;
94-
if (ExceptionRootCauseFinder.IRRECOVERABLE_REST_STATUSES.contains(elasticsearchException.status())) {
94+
if (IRRECOVERABLE_REST_STATUSES.contains(elasticsearchException.status())) {
9595
return elasticsearchException;
9696
}
9797
}

‎x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/utils/ExceptionRootCauseFinderTests.java

+146
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.transform.utils;
8+
9+
import org.elasticsearch.ElasticsearchSecurityException;
10+
import org.elasticsearch.ResourceNotFoundException;
11+
import org.elasticsearch.action.DocWriteRequest.OpType;
12+
import org.elasticsearch.action.bulk.BulkItemResponse;
13+
import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
14+
import org.elasticsearch.index.mapper.MapperParsingException;
15+
import org.elasticsearch.index.shard.ShardId;
16+
import org.elasticsearch.index.translog.TranslogException;
17+
import org.elasticsearch.rest.RestStatus;
18+
import org.elasticsearch.test.ESTestCase;
19+
20+
import java.util.HashMap;
21+
import java.util.Map;
22+
23+
public class ExceptionRootCauseFinderTests extends ESTestCase {
24+
public void testFetFirstIrrecoverableExceptionFromBulkResponses() {
25+
Map<Integer, BulkItemResponse> bulkItemResponses = new HashMap<>();
26+
27+
int id = 1;
28+
// 1
29+
bulkItemResponses.put(
30+
id,
31+
new BulkItemResponse(
32+
id++,
33+
OpType.INDEX,
34+
new BulkItemResponse.Failure("the_index", "id", new MapperParsingException("mapper parsing error"))
35+
)
36+
);
37+
// 2
38+
bulkItemResponses.put(
39+
id,
40+
new BulkItemResponse(
41+
id++,
42+
OpType.INDEX,
43+
new BulkItemResponse.Failure("the_index", "id", new ResourceNotFoundException("resource not found error"))
44+
)
45+
);
46+
// 3
47+
bulkItemResponses.put(
48+
id,
49+
new BulkItemResponse(
50+
id++,
51+
OpType.INDEX,
52+
new BulkItemResponse.Failure("the_index", "id", new IllegalArgumentException("illegal argument error"))
53+
)
54+
);
55+
// 4 not irrecoverable
56+
bulkItemResponses.put(
57+
id,
58+
new BulkItemResponse(
59+
id++,
60+
OpType.INDEX,
61+
new BulkItemResponse.Failure("the_index", "id", new EsRejectedExecutionException("es rejected execution"))
62+
)
63+
);
64+
// 5 not irrecoverable
65+
bulkItemResponses.put(
66+
id,
67+
new BulkItemResponse(
68+
id++,
69+
OpType.INDEX,
70+
new BulkItemResponse.Failure("the_index", "id", new TranslogException(new ShardId("the_index", "uid", 0), "translog error"))
71+
)
72+
);
73+
// 6
74+
bulkItemResponses.put(
75+
id,
76+
new BulkItemResponse(
77+
id++,
78+
OpType.INDEX,
79+
new BulkItemResponse.Failure(
80+
"the_index",
81+
"id",
82+
new ElasticsearchSecurityException("Authentication required", RestStatus.UNAUTHORIZED)
83+
)
84+
)
85+
);
86+
// 7
87+
bulkItemResponses.put(
88+
id,
89+
new BulkItemResponse(
90+
id++,
91+
OpType.INDEX,
92+
new BulkItemResponse.Failure(
93+
"the_index",
94+
"id",
95+
new ElasticsearchSecurityException("current license is non-compliant for [transform]", RestStatus.FORBIDDEN)
96+
)
97+
)
98+
);
99+
// 8 not irrecoverable
100+
bulkItemResponses.put(
101+
id,
102+
new BulkItemResponse(
103+
id++,
104+
OpType.INDEX,
105+
new BulkItemResponse.Failure(
106+
"the_index",
107+
"id",
108+
new ElasticsearchSecurityException("overloaded, to many requests", RestStatus.TOO_MANY_REQUESTS)
109+
)
110+
)
111+
);
112+
// 9 not irrecoverable
113+
bulkItemResponses.put(
114+
id,
115+
new BulkItemResponse(
116+
id++,
117+
OpType.INDEX,
118+
new BulkItemResponse.Failure(
119+
"the_index",
120+
"id",
121+
new ElasticsearchSecurityException("internal error", RestStatus.INTERNAL_SERVER_ERROR)
122+
)
123+
)
124+
);
125+
126+
assertFirstException(bulkItemResponses, MapperParsingException.class, "mapper parsing error");
127+
bulkItemResponses.remove(1);
128+
assertFirstException(bulkItemResponses, ResourceNotFoundException.class, "resource not found error");
129+
bulkItemResponses.remove(2);
130+
assertFirstException(bulkItemResponses, IllegalArgumentException.class, "illegal argument error");
131+
bulkItemResponses.remove(3);
132+
assertFirstException(bulkItemResponses, ElasticsearchSecurityException.class, "Authentication required");
133+
bulkItemResponses.remove(6);
134+
assertFirstException(bulkItemResponses, ElasticsearchSecurityException.class, "current license is non-compliant for [transform]");
135+
bulkItemResponses.remove(7);
136+
137+
assertNull(ExceptionRootCauseFinder.getFirstIrrecoverableExceptionFromBulkResponses(bulkItemResponses.values()));
138+
}
139+
140+
private static void assertFirstException(Map<Integer, BulkItemResponse> bulkItemResponses, Class<?> expectedClass, String message) {
141+
Throwable t = ExceptionRootCauseFinder.getFirstIrrecoverableExceptionFromBulkResponses(bulkItemResponses.values());
142+
assertNotNull(t);
143+
assertEquals(t.getClass(), expectedClass);
144+
assertEquals(t.getMessage(), message);
145+
}
146+
}

0 commit comments

Comments
 (0)