From a8bfa466b2f2a68c2384bc730a0b8c7c9ce7ea87 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 14 Aug 2018 08:20:35 +0200 Subject: [PATCH] Fix NOOP bulk updates (#32819) #31821 introduced an unreleased bug where NOOP updates were incorrectly mutating the bulk shard request, inserting null item to be replicated, which would result in NullPointerExceptions when serializing the request to be shipped to the replicas. Closes #32808 --- .../action/bulk/BulkPrimaryExecutionContext.java | 4 ++-- .../test/java/org/elasticsearch/action/IndicesRequestIT.java | 1 - .../action/bulk/TransportShardBulkActionTests.java | 2 ++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkPrimaryExecutionContext.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkPrimaryExecutionContext.java index 85ce28d2d52..5f61d90d500 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkPrimaryExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkPrimaryExecutionContext.java @@ -290,10 +290,10 @@ class BulkPrimaryExecutionContext { /** finishes the execution of the current request, with the response that should be returned to the user */ public void markAsCompleted(BulkItemResponse translatedResponse) { assertInvariants(ItemProcessingState.EXECUTED); - assert executionResult == null || translatedResponse.getItemId() == executionResult.getItemId(); + assert executionResult != null && translatedResponse.getItemId() == executionResult.getItemId(); assert translatedResponse.getItemId() == getCurrentItem().id(); - if (translatedResponse.isFailed() == false && requestToExecute != getCurrent()) { + if (translatedResponse.isFailed() == false && requestToExecute != null && requestToExecute != getCurrent()) { request.items()[currentIndex] = new BulkItemRequest(request.items()[currentIndex].id(), requestToExecute); } getCurrentItem().setPrimaryResponse(translatedResponse); diff --git a/server/src/test/java/org/elasticsearch/action/IndicesRequestIT.java b/server/src/test/java/org/elasticsearch/action/IndicesRequestIT.java index bf28dc2ae90..40795bff730 100644 --- a/server/src/test/java/org/elasticsearch/action/IndicesRequestIT.java +++ b/server/src/test/java/org/elasticsearch/action/IndicesRequestIT.java @@ -284,7 +284,6 @@ public class IndicesRequestIT extends ESIntegTestCase { assertSameIndices(updateRequest, updateShardActions); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/32808") public void testBulk() { String[] bulkShardActions = new String[]{BulkAction.NAME + "[s][p]", BulkAction.NAME + "[s][r]"}; interceptTransportActions(bulkShardActions); diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java index e60ee1395a8..ca19dcc2509 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java @@ -472,6 +472,8 @@ public class TransportShardBulkActionTests extends IndexShardTestCase { assertThat(primaryResponse.getResponse(), equalTo(noopUpdateResponse)); assertThat(primaryResponse.getResponse().getResult(), equalTo(DocWriteResponse.Result.NOOP)); + assertThat(bulkShardRequest.items().length, equalTo(1)); + assertEquals(primaryRequest, bulkShardRequest.items()[0]); // check that bulk item was not mutated assertThat(primaryResponse.getResponse().getSeqNo(), equalTo(SequenceNumbers.UNASSIGNED_SEQ_NO)); }