From 5f656d77cae91208173a93b8bc476f02a0b51e46 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 85ce28d2d52db..5f61d90d500e7 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 @@ public void markOperationAsExecuted(Engine.Result result) { /** 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 8cb0ba8ceff83..8fac0b91cd6d6 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 void testUpdateDelete() { 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 829f551da2634..8ec822f63a0fe 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 void testNoopUpdateRequest() throws Exception { 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)); }