Skip to content

Commit f6ca4e1

Browse files
authored
Improve logging of put-mapping failures (#121372)
No sense in converting to a list just to convert to a string, we may as well convert directly to a string. Also removes the unnecessary extra `[]` wrapper.
1 parent 0393e56 commit f6ca4e1

File tree

3 files changed

+71
-21
lines changed

3 files changed

+71
-21
lines changed
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
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+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.action.admin.indices.mapping.put;
11+
12+
import org.apache.logging.log4j.Level;
13+
import org.elasticsearch.action.support.master.AcknowledgedResponse;
14+
import org.elasticsearch.test.ESSingleNodeTestCase;
15+
import org.elasticsearch.test.MockLog;
16+
import org.elasticsearch.test.junit.annotations.TestLogging;
17+
18+
import static org.hamcrest.Matchers.equalTo;
19+
20+
public class PutMappingIT extends ESSingleNodeTestCase {
21+
22+
@TestLogging(
23+
reason = "testing DEBUG logging",
24+
value = "org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction:DEBUG"
25+
)
26+
public void testFailureLogging() {
27+
final var indexName = randomIdentifier();
28+
createIndex(indexName);
29+
final var fieldName = randomIdentifier();
30+
safeGet(client().execute(TransportPutMappingAction.TYPE, new PutMappingRequest(indexName).source(fieldName, "type=keyword")));
31+
MockLog.assertThatLogger(
32+
() -> assertThat(
33+
asInstanceOf(
34+
IllegalArgumentException.class,
35+
safeAwaitFailure(
36+
AcknowledgedResponse.class,
37+
l -> client().execute(
38+
TransportPutMappingAction.TYPE,
39+
new PutMappingRequest(indexName).source(fieldName, "type=long"),
40+
l
41+
)
42+
)
43+
).getMessage(),
44+
equalTo("mapper [" + fieldName + "] cannot be changed from type [keyword] to [long]")
45+
),
46+
TransportPutMappingAction.class,
47+
new MockLog.SeenEventExpectation(
48+
"failure message",
49+
TransportPutMappingAction.class.getCanonicalName(),
50+
Level.DEBUG,
51+
"failed to put mappings on indices [[" + indexName
52+
)
53+
);
54+
}
55+
}

server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.elasticsearch.threadpool.ThreadPool;
3737
import org.elasticsearch.transport.TransportService;
3838

39-
import java.io.IOException;
4039
import java.util.ArrayList;
4140
import java.util.Arrays;
4241
import java.util.List;
@@ -126,7 +125,7 @@ protected void masterOperation(
126125

127126
performMappingUpdate(concreteIndices, request, listener, metadataMappingService, false);
128127
} catch (IndexNotFoundException ex) {
129-
logger.debug(() -> "failed to put mappings on indices [" + Arrays.asList(request.indices() + "]"), ex);
128+
logger.debug(() -> "failed to put mappings on indices " + Arrays.toString(request.indices()), ex);
130129
throw ex;
131130
}
132131
}
@@ -162,25 +161,21 @@ static void performMappingUpdate(
162161
MetadataMappingService metadataMappingService,
163162
boolean autoUpdate
164163
) {
165-
final ActionListener<AcknowledgedResponse> wrappedListener = listener.delegateResponse((l, e) -> {
166-
logger.debug(() -> "failed to put mappings on indices [" + Arrays.asList(concreteIndices) + "]", e);
164+
ActionListener.run(listener.delegateResponse((l, e) -> {
165+
logger.debug(() -> "failed to put mappings on indices " + Arrays.toString(concreteIndices), e);
167166
l.onFailure(e);
168-
});
169-
final PutMappingClusterStateUpdateRequest updateRequest;
170-
try {
171-
updateRequest = new PutMappingClusterStateUpdateRequest(
172-
request.masterNodeTimeout(),
173-
request.ackTimeout(),
174-
request.source(),
175-
autoUpdate,
176-
concreteIndices
177-
);
178-
} catch (IOException e) {
179-
wrappedListener.onFailure(e);
180-
return;
181-
}
182-
183-
metadataMappingService.putMapping(updateRequest, wrappedListener);
167+
}),
168+
wrappedListener -> metadataMappingService.putMapping(
169+
new PutMappingClusterStateUpdateRequest(
170+
request.masterNodeTimeout(),
171+
request.ackTimeout(),
172+
request.source(),
173+
autoUpdate,
174+
concreteIndices
175+
),
176+
wrappedListener
177+
)
178+
);
184179
}
185180

186181
static String checkForFailureStoreViolations(ClusterState clusterState, Index[] concreteIndices, PutMappingRequest request) {

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataMappingService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public class MetadataMappingService {
5656
public MetadataMappingService(ClusterService clusterService, IndicesService indicesService) {
5757
this.clusterService = clusterService;
5858
this.indicesService = indicesService;
59-
taskQueue = clusterService.createTaskQueue("put-mapping", Priority.HIGH, new PutMappingExecutor());
59+
this.taskQueue = clusterService.createTaskQueue("put-mapping", Priority.HIGH, new PutMappingExecutor());
6060
}
6161

6262
record PutMappingClusterStateUpdateTask(PutMappingClusterStateUpdateRequest request, ActionListener<AcknowledgedResponse> listener)

0 commit comments

Comments
 (0)