Skip to content

Commit 0c733c0

Browse files
committed
Remove immediate operation retry after mapping update (#38873)
Prior to this commit, when an indexing operation resulted in an `Engine.Result.Type.MAPPING_UPDATE_REQUIRED`, TransportShardBulkAction immediately retries the indexing operation to see if it succeeds. In the event that it succeeds the context does not wait until the mapping update has propagated through the cluster state before finishing the indexing. In some of our tests we rely on mappings being available as soon as they've been introduced in a document that indexed correctly. By removing the immediate retry we always wait for this to be the case. Resolves #38428 Supercedes #38579 Relates to #38711
1 parent d9c255d commit 0c733c0

File tree

2 files changed

+6
-16
lines changed

2 files changed

+6
-16
lines changed

server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -483,26 +483,16 @@ private static <T extends Engine.Result> void executeOnPrimaryWhileHandlingMappi
483483
throws IOException {
484484
T result = toExecute.get();
485485
if (result.getResultType() == Engine.Result.Type.MAPPING_UPDATE_REQUIRED) {
486-
// try to update the mappings and try again.
486+
// try to update the mappings and mark the context as needing to try again.
487487
try {
488488
mappingUpdater.accept(result.getRequiredMappingUpdate());
489+
context.markAsRequiringMappingUpdate();
489490
} catch (Exception e) {
490491
// failure to update the mapping should translate to a failure of specific requests. Other requests
491492
// still need to be executed and replicated.
492493
onComplete.accept(exceptionToResult.apply(e));
493494
return;
494495
}
495-
496-
// TODO - we can fall back to a wait for cluster state update but I'm keeping the logic the same for now
497-
result = toExecute.get();
498-
499-
if (result.getResultType() == Engine.Result.Type.MAPPING_UPDATE_REQUIRED) {
500-
// double mapping update. We assume that the successful mapping update wasn't yet processed on the node
501-
// and retry the entire request again.
502-
context.markAsRequiringMappingUpdate();
503-
} else {
504-
onComplete.accept(result);
505-
}
506496
} else {
507497
onComplete.accept(result);
508498
}

server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,8 @@ public void testExecuteBulkIndexRequestWithMappingUpdates() throws Exception {
285285

286286
assertThat("mappings were \"updated\" once", updateCalled.get(), equalTo(1));
287287

288-
// Verify that the shard "executed" the operation twice
289-
verify(shard, times(2)).applyIndexOperationOnPrimary(anyLong(), any(), any(), anyLong(), anyLong(), anyLong(), anyBoolean());
288+
// Verify that the shard "executed" the operation once
289+
verify(shard, times(1)).applyIndexOperationOnPrimary(anyLong(), any(), any(), anyLong(), anyLong(), anyLong(), anyBoolean());
290290

291291
when(shard.applyIndexOperationOnPrimary(anyLong(), any(), any(), anyLong(), anyLong(), anyLong(), anyBoolean()))
292292
.thenReturn(success);
@@ -295,9 +295,9 @@ public void testExecuteBulkIndexRequestWithMappingUpdates() throws Exception {
295295
(update, shardId, type) -> fail("should not have had to update the mappings"), () -> {});
296296

297297

298-
// Verify that the shard "executed" the operation only once (2 for previous invocations plus
298+
// Verify that the shard "executed" the operation only once (1 for previous invocations plus
299299
// 1 for this execution)
300-
verify(shard, times(3)).applyIndexOperationOnPrimary(anyLong(), any(), any(), anyLong(), anyLong(), anyLong(), anyBoolean());
300+
verify(shard, times(2)).applyIndexOperationOnPrimary(anyLong(), any(), any(), anyLong(), anyLong(), anyLong(), anyBoolean());
301301

302302

303303
BulkItemResponse primaryResponse = bulkShardRequest.items()[0].getPrimaryResponse();

0 commit comments

Comments
 (0)