Skip to content

Commit 08e3ceb

Browse files
authored
Allow dropping documents with auto-generated ID (#46773)
When using auto-generated IDs + the ingest drop processor (which looks to be used by filebeat as well) + coordinating nodes that do not have the ingest processor functionality, this can lead to a NullPointerException. The issue is that markCurrentItemAsDropped() is creating an UpdateResponse with no id when the request contains auto-generated IDs. The response serialization is lenient for our REST/XContent format (i.e. we will send "id" : null) but the internal transport format (used for communication between nodes) assumes for this field to be non-null, which means that it can't be serialized between nodes. Bulk requests with ingest functionality are processed on the coordinating node if the node has the ingest capability, and only otherwise sent to a different node. This means that, in order to reproduce this, one needs two nodes, with the coordinating node not having the ingest functionality. Closes #46678
1 parent 8b764a5 commit 08e3ceb

File tree

17 files changed

+92
-37
lines changed

17 files changed

+92
-37
lines changed

modules/reindex/src/test/java/org/elasticsearch/index/reindex/AsyncBulkByScrollActionTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,7 @@ void doExecute(ActionType<Response> action, Request request, ActionListener<Resp
878878
new IndexResponse(
879879
shardId,
880880
index.type(),
881-
index.id(),
881+
index.id() == null ? "dummy_id" : index.id(),
882882
randomInt(20),
883883
randomIntBetween(1, 16),
884884
randomIntBetween(0, Integer.MAX_VALUE),

server/src/main/java/org/elasticsearch/action/DocWriteResponse.java

+5-4
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.io.UnsupportedEncodingException;
4141
import java.net.URLEncoder;
4242
import java.util.Locale;
43+
import java.util.Objects;
4344

4445
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
4546
import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_PRIMARY_TERM;
@@ -121,13 +122,13 @@ public void writeTo(StreamOutput out) throws IOException {
121122
protected final Result result;
122123

123124
public DocWriteResponse(ShardId shardId, String type, String id, long seqNo, long primaryTerm, long version, Result result) {
124-
this.shardId = shardId;
125-
this.type = type;
126-
this.id = id;
125+
this.shardId = Objects.requireNonNull(shardId);
126+
this.type = Objects.requireNonNull(type);
127+
this.id = Objects.requireNonNull(id);
127128
this.seqNo = seqNo;
128129
this.primaryTerm = primaryTerm;
129130
this.version = version;
130-
this.result = result;
131+
this.result = Objects.requireNonNull(result);
131132
}
132133

133134
// needed for deserialization

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ public class TransportBulkAction extends HandledTransportAction<BulkRequest, Bul
100100
private final IngestActionForwarder ingestForwarder;
101101
private final NodeClient client;
102102
private final IndexNameExpressionResolver indexNameExpressionResolver;
103+
private static final String DROPPED_ITEM_WITH_AUTO_GENERATED_ID = "auto-generated";
103104

104105
@Inject
105106
public TransportBulkAction(ThreadPool threadPool, TransportService transportService,
@@ -672,11 +673,12 @@ ActionListener<BulkResponse> wrapActionListenerIfNeeded(long ingestTookInMillis,
672673
void markCurrentItemAsDropped() {
673674
IndexRequest indexRequest = getIndexWriteRequest(bulkRequest.requests().get(currentSlot));
674675
failedSlots.set(currentSlot);
676+
final String id = indexRequest.id() == null ? DROPPED_ITEM_WITH_AUTO_GENERATED_ID : indexRequest.id();
675677
itemResponses.add(
676678
new BulkItemResponse(currentSlot, indexRequest.opType(),
677679
new UpdateResponse(
678680
new ShardId(indexRequest.index(), IndexMetaData.INDEX_UUID_NA_VALUE, 0),
679-
indexRequest.type(), indexRequest.id(), SequenceNumbers.UNASSIGNED_SEQ_NO, SequenceNumbers.UNASSIGNED_PRIMARY_TERM,
681+
indexRequest.type(), id, SequenceNumbers.UNASSIGNED_SEQ_NO, SequenceNumbers.UNASSIGNED_PRIMARY_TERM,
680682
indexRequest.version(), DocWriteResponse.Result.NOOP
681683
)
682684
)

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.elasticsearch.action.update.UpdateRequest;
2727
import org.elasticsearch.common.unit.TimeValue;
2828
import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
29+
import org.elasticsearch.index.shard.ShardId;
2930
import org.elasticsearch.test.ESTestCase;
3031
import org.elasticsearch.test.client.NoOpClient;
3132
import org.junit.After;
@@ -226,7 +227,8 @@ public void bulk(BulkRequest request, ActionListener<BulkResponse> listener) {
226227
}
227228

228229
private BulkItemResponse successfulResponse() {
229-
return new BulkItemResponse(1, OpType.DELETE, new DeleteResponse(null, null, null, 0, 0, 0, false));
230+
return new BulkItemResponse(1, OpType.DELETE, new DeleteResponse(
231+
new ShardId("test", "test", 0), "_doc", "test", 0, 0, 0, false));
230232
}
231233

232234
private BulkItemResponse failedResponse() {

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ public void testExecuteBulkIndexRequestWithMappingUpdates() throws Exception {
240240
Engine.IndexResult success = new FakeIndexResult(1, 1, 13, true, resultLocation);
241241

242242
IndexShard shard = mock(IndexShard.class);
243+
when(shard.shardId()).thenReturn(shardId);
243244
when(shard.applyIndexOperationOnPrimary(anyLong(), any(), any(), anyLong(), anyLong(), anyLong(), anyBoolean()))
244245
.thenReturn(mappingUpdate);
245246

@@ -583,6 +584,7 @@ public void testUpdateRequestWithSuccess() throws Exception {
583584
when(shard.applyIndexOperationOnPrimary(anyLong(), any(), any(), anyLong(), anyLong(), anyLong(), anyBoolean()))
584585
.thenReturn(indexResult);
585586
when(shard.indexSettings()).thenReturn(indexSettings);
587+
when(shard.shardId()).thenReturn(shardId);
586588

587589
UpdateHelper updateHelper = mock(UpdateHelper.class);
588590
when(updateHelper.prepare(any(), eq(shard), any())).thenReturn(
@@ -629,6 +631,7 @@ public void testUpdateWithDelete() throws Exception {
629631
IndexShard shard = mock(IndexShard.class);
630632
when(shard.applyDeleteOperationOnPrimary(anyLong(), any(), any(), any(), anyLong(), anyLong())).thenReturn(deleteResult);
631633
when(shard.indexSettings()).thenReturn(indexSettings);
634+
when(shard.shardId()).thenReturn(shardId);
632635

633636
UpdateHelper updateHelper = mock(UpdateHelper.class);
634637
when(updateHelper.prepare(any(), eq(shard), any())).thenReturn(
@@ -783,6 +786,7 @@ public void testRetries() throws Exception {
783786
}
784787
});
785788
when(shard.indexSettings()).thenReturn(indexSettings);
789+
when(shard.shardId()).thenReturn(shardId);
786790

787791
UpdateHelper updateHelper = mock(UpdateHelper.class);
788792
when(updateHelper.prepare(any(), eq(shard), any())).thenReturn(
@@ -814,7 +818,7 @@ public void testRetries() throws Exception {
814818
private void randomlySetIgnoredPrimaryResponse(BulkItemRequest primaryRequest) {
815819
if (randomBoolean()) {
816820
// add a response to the request and thereby check that it is ignored for the primary.
817-
primaryRequest.setPrimaryResponse(new BulkItemResponse(0, DocWriteRequest.OpType.INDEX, new IndexResponse(null, "_doc",
821+
primaryRequest.setPrimaryResponse(new BulkItemResponse(0, DocWriteRequest.OpType.INDEX, new IndexResponse(shardId, "_doc",
818822
"ignore-primary-response-on-primary", 42, 42, 42, false)));
819823
}
820824
}

server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java

+3-5
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,7 @@ public void testExecuteItem() throws Exception {
101101

102102
public void testExecuteVerboseItemExceptionWithoutOnFailure() throws Exception {
103103
TestProcessor processor1 = new TestProcessor("processor_0", "mock", ingestDocument -> {});
104-
TestProcessor processor2 = new TestProcessor("processor_1", "mock",
105-
ingestDocument -> { throw new RuntimeException("processor failed"); });
104+
TestProcessor processor2 = new TestProcessor("processor_1", "mock", new RuntimeException("processor failed"));
106105
TestProcessor processor3 = new TestProcessor("processor_2", "mock", ingestDocument -> {});
107106
Pipeline pipeline = new Pipeline("_id", "_description", version, new CompoundProcessor(processor1, processor2, processor3));
108107
SimulateDocumentResult actualItemResponse = executionService.executeDocument(pipeline, ingestDocument, true);
@@ -126,8 +125,7 @@ public void testExecuteVerboseItemExceptionWithoutOnFailure() throws Exception {
126125
}
127126

128127
public void testExecuteVerboseItemWithOnFailure() throws Exception {
129-
TestProcessor processor1 = new TestProcessor("processor_0", "mock",
130-
ingestDocument -> { throw new RuntimeException("processor failed"); });
128+
TestProcessor processor1 = new TestProcessor("processor_0", "mock", new RuntimeException("processor failed"));
131129
TestProcessor processor2 = new TestProcessor("processor_1", "mock", ingestDocument -> {});
132130
TestProcessor processor3 = new TestProcessor("processor_2", "mock", ingestDocument -> {});
133131
Pipeline pipeline = new Pipeline("_id", "_description", version,
@@ -165,7 +163,7 @@ public void testExecuteVerboseItemWithOnFailure() throws Exception {
165163

166164
public void testExecuteVerboseItemExceptionWithIgnoreFailure() throws Exception {
167165
RuntimeException exception = new RuntimeException("processor failed");
168-
TestProcessor testProcessor = new TestProcessor("processor_0", "mock", ingestDocument -> { throw exception; });
166+
TestProcessor testProcessor = new TestProcessor("processor_0", "mock", exception);
169167
CompoundProcessor processor = new CompoundProcessor(true, Collections.singletonList(testProcessor), Collections.emptyList());
170168
Pipeline pipeline = new Pipeline("_id", "_description", version, new CompoundProcessor(processor));
171169
SimulateDocumentResult actualItemResponse = executionService.executeDocument(pipeline, ingestDocument, true);

server/src/test/java/org/elasticsearch/ingest/CompoundProcessorTests.java

+14-13
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.HashMap;
2929
import java.util.Map;
3030
import java.util.concurrent.TimeUnit;
31+
import java.util.function.Consumer;
3132
import java.util.function.LongSupplier;
3233

3334
import static org.hamcrest.CoreMatchers.equalTo;
@@ -74,7 +75,7 @@ public void testSingleProcessor() throws Exception {
7475
}
7576

7677
public void testSingleProcessorWithException() throws Exception {
77-
TestProcessor processor = new TestProcessor(ingestDocument -> {throw new RuntimeException("error");});
78+
TestProcessor processor = new TestProcessor(new RuntimeException("error"));
7879
LongSupplier relativeTimeProvider = mock(LongSupplier.class);
7980
when(relativeTimeProvider.getAsLong()).thenReturn(0L);
8081
CompoundProcessor compoundProcessor = new CompoundProcessor(relativeTimeProvider, processor);
@@ -93,7 +94,7 @@ public void testSingleProcessorWithException() throws Exception {
9394
}
9495

9596
public void testIgnoreFailure() throws Exception {
96-
TestProcessor processor1 = new TestProcessor(ingestDocument -> {throw new RuntimeException("error");});
97+
TestProcessor processor1 = new TestProcessor(new RuntimeException("error"));
9798
TestProcessor processor2 = new TestProcessor(ingestDocument -> {ingestDocument.setFieldValue("field", "value");});
9899
LongSupplier relativeTimeProvider = mock(LongSupplier.class);
99100
when(relativeTimeProvider.getAsLong()).thenReturn(0L);
@@ -108,7 +109,7 @@ public void testIgnoreFailure() throws Exception {
108109
}
109110

110111
public void testSingleProcessorWithOnFailureProcessor() throws Exception {
111-
TestProcessor processor1 = new TestProcessor("id", "first", ingestDocument -> {throw new RuntimeException("error");});
112+
TestProcessor processor1 = new TestProcessor("id", "first", new RuntimeException("error"));
112113
TestProcessor processor2 = new TestProcessor(ingestDocument -> {
113114
Map<String, Object> ingestMetadata = ingestDocument.getIngestMetadata();
114115
assertThat(ingestMetadata.size(), equalTo(3));
@@ -130,7 +131,7 @@ public void testSingleProcessorWithOnFailureProcessor() throws Exception {
130131
}
131132

132133
public void testSingleProcessorWithOnFailureDropProcessor() throws Exception {
133-
TestProcessor processor1 = new TestProcessor("id", "first", ingestDocument -> {throw new RuntimeException("error");});
134+
TestProcessor processor1 = new TestProcessor("id", "first", new RuntimeException("error"));
134135
Processor processor2 = new Processor() {
135136
@Override
136137
public IngestDocument execute(IngestDocument ingestDocument) throws Exception {
@@ -159,8 +160,8 @@ public String getTag() {
159160
}
160161

161162
public void testSingleProcessorWithNestedFailures() throws Exception {
162-
TestProcessor processor = new TestProcessor("id", "first", ingestDocument -> {throw new RuntimeException("error");});
163-
TestProcessor processorToFail = new TestProcessor("id2", "second", ingestDocument -> {
163+
TestProcessor processor = new TestProcessor("id", "first", new RuntimeException("error"));
164+
TestProcessor processorToFail = new TestProcessor("id2", "second", (Consumer<IngestDocument>) ingestDocument -> {
164165
Map<String, Object> ingestMetadata = ingestDocument.getIngestMetadata();
165166
assertThat(ingestMetadata.size(), equalTo(3));
166167
assertThat(ingestMetadata.get(CompoundProcessor.ON_FAILURE_MESSAGE_FIELD), equalTo("error"));
@@ -189,7 +190,7 @@ public void testSingleProcessorWithNestedFailures() throws Exception {
189190
}
190191

191192
public void testCompoundProcessorExceptionFailWithoutOnFailure() throws Exception {
192-
TestProcessor firstProcessor = new TestProcessor("id1", "first", ingestDocument -> {throw new RuntimeException("error");});
193+
TestProcessor firstProcessor = new TestProcessor("id1", "first", new RuntimeException("error"));
193194
TestProcessor secondProcessor = new TestProcessor("id3", "second", ingestDocument -> {
194195
Map<String, Object> ingestMetadata = ingestDocument.getIngestMetadata();
195196
assertThat(ingestMetadata.entrySet(), hasSize(3));
@@ -212,9 +213,9 @@ public void testCompoundProcessorExceptionFailWithoutOnFailure() throws Exceptio
212213
}
213214

214215
public void testCompoundProcessorExceptionFail() throws Exception {
215-
TestProcessor firstProcessor = new TestProcessor("id1", "first", ingestDocument -> {throw new RuntimeException("error");});
216+
TestProcessor firstProcessor = new TestProcessor("id1", "first", new RuntimeException("error"));
216217
TestProcessor failProcessor =
217-
new TestProcessor("tag_fail", "fail", ingestDocument -> {throw new RuntimeException("custom error message");});
218+
new TestProcessor("tag_fail", "fail", new RuntimeException("custom error message"));
218219
TestProcessor secondProcessor = new TestProcessor("id3", "second", ingestDocument -> {
219220
Map<String, Object> ingestMetadata = ingestDocument.getIngestMetadata();
220221
assertThat(ingestMetadata.entrySet(), hasSize(3));
@@ -238,9 +239,9 @@ public void testCompoundProcessorExceptionFail() throws Exception {
238239
}
239240

240241
public void testCompoundProcessorExceptionFailInOnFailure() throws Exception {
241-
TestProcessor firstProcessor = new TestProcessor("id1", "first", ingestDocument -> {throw new RuntimeException("error");});
242+
TestProcessor firstProcessor = new TestProcessor("id1", "first", new RuntimeException("error"));
242243
TestProcessor failProcessor =
243-
new TestProcessor("tag_fail", "fail", ingestDocument -> {throw new RuntimeException("custom error message");});
244+
new TestProcessor("tag_fail", "fail", new RuntimeException("custom error message"));
244245
TestProcessor secondProcessor = new TestProcessor("id3", "second", ingestDocument -> {
245246
Map<String, Object> ingestMetadata = ingestDocument.getIngestMetadata();
246247
assertThat(ingestMetadata.entrySet(), hasSize(3));
@@ -264,8 +265,8 @@ public void testCompoundProcessorExceptionFailInOnFailure() throws Exception {
264265
}
265266

266267
public void testBreakOnFailure() throws Exception {
267-
TestProcessor firstProcessor = new TestProcessor("id1", "first", ingestDocument -> {throw new RuntimeException("error1");});
268-
TestProcessor secondProcessor = new TestProcessor("id2", "second", ingestDocument -> {throw new RuntimeException("error2");});
268+
TestProcessor firstProcessor = new TestProcessor("id1", "first", new RuntimeException("error1"));
269+
TestProcessor secondProcessor = new TestProcessor("id2", "second", new RuntimeException("error2"));
269270
TestProcessor onFailureProcessor = new TestProcessor("id2", "on_failure", ingestDocument -> {});
270271
LongSupplier relativeTimeProvider = mock(LongSupplier.class);
271272
when(relativeTimeProvider.getAsLong()).thenReturn(0L);

server/src/test/java/org/elasticsearch/ingest/IngestClientIT.java

+21
Original file line numberDiff line numberDiff line change
@@ -272,4 +272,25 @@ public void testPutWithPipelineFactoryError() throws Exception {
272272
GetPipelineResponse response = client().admin().cluster().prepareGetPipeline("_id2").get();
273273
assertFalse(response.isFound());
274274
}
275+
276+
public void testWithDedicatedMaster() throws Exception {
277+
String masterOnlyNode = internalCluster().startMasterOnlyNode();
278+
BytesReference source = BytesReference.bytes(jsonBuilder().startObject()
279+
.field("description", "my_pipeline")
280+
.startArray("processors")
281+
.startObject()
282+
.startObject("test")
283+
.endObject()
284+
.endObject()
285+
.endArray()
286+
.endObject());
287+
PutPipelineRequest putPipelineRequest = new PutPipelineRequest("_id", source, XContentType.JSON);
288+
client().admin().cluster().putPipeline(putPipelineRequest).get();
289+
290+
BulkItemResponse item = client(masterOnlyNode).prepareBulk().add(
291+
client().prepareIndex("test", "type").setSource("field", "value2", "drop", true).setPipeline("_id")).get()
292+
.getItems()[0];
293+
assertFalse(item.isFailed());
294+
assertEquals("auto-generated", item.getResponse().getId());
295+
}
275296
}

server/src/test/java/org/elasticsearch/ingest/TrackingResultProcessorTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public void testActualCompoundProcessorWithoutOnFailure() throws Exception {
101101

102102
public void testActualCompoundProcessorWithOnFailure() throws Exception {
103103
RuntimeException exception = new RuntimeException("fail");
104-
TestProcessor failProcessor = new TestProcessor("fail", "test", ingestDocument -> { throw exception; });
104+
TestProcessor failProcessor = new TestProcessor("fail", "test", exception);
105105
TestProcessor onFailureProcessor = new TestProcessor("success", "test", ingestDocument -> {});
106106
CompoundProcessor actualProcessor = new CompoundProcessor(false,
107107
Arrays.asList(new CompoundProcessor(false,

test/framework/src/main/java/org/elasticsearch/ingest/IngestTestPlugin.java

+4
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ public Map<String, Processor.Factory> getProcessors(Processor.Parameters paramet
3737
if (doc.hasField("fail") && doc.getFieldValue("fail", Boolean.class)) {
3838
throw new IllegalArgumentException("test processor failed");
3939
}
40+
if (doc.hasField("drop") && doc.getFieldValue("drop", Boolean.class)) {
41+
return null;
42+
}
43+
return doc;
4044
}));
4145
}
4246
}

test/framework/src/main/java/org/elasticsearch/ingest/TestProcessor.java

+19-4
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Map;
2323
import java.util.concurrent.atomic.AtomicInteger;
2424
import java.util.function.Consumer;
25+
import java.util.function.Function;
2526

2627
/**
2728
* Processor used for testing, keeps track of how many times it is invoked and
@@ -31,24 +32,38 @@ public class TestProcessor implements Processor {
3132

3233
private final String type;
3334
private final String tag;
34-
private final Consumer<IngestDocument> ingestDocumentConsumer;
35+
private final Function<IngestDocument, IngestDocument> ingestDocumentMapper;
3536
private final AtomicInteger invokedCounter = new AtomicInteger();
3637

3738
public TestProcessor(Consumer<IngestDocument> ingestDocumentConsumer) {
3839
this(null, "test-processor", ingestDocumentConsumer);
3940
}
4041

42+
public TestProcessor(RuntimeException e) {
43+
this(null, "test-processor", e);
44+
}
45+
46+
public TestProcessor(String tag, String type, RuntimeException e) {
47+
this(tag, type, (Consumer<IngestDocument>) i -> { throw e; });
48+
}
49+
4150
public TestProcessor(String tag, String type, Consumer<IngestDocument> ingestDocumentConsumer) {
42-
this.ingestDocumentConsumer = ingestDocumentConsumer;
51+
this(tag, type, id -> {
52+
ingestDocumentConsumer.accept(id);
53+
return id;
54+
});
55+
}
56+
57+
public TestProcessor(String tag, String type, Function<IngestDocument, IngestDocument> ingestDocumentMapper) {
58+
this.ingestDocumentMapper = ingestDocumentMapper;
4359
this.type = type;
4460
this.tag = tag;
4561
}
4662

4763
@Override
4864
public IngestDocument execute(IngestDocument ingestDocument) throws Exception {
4965
invokedCounter.incrementAndGet();
50-
ingestDocumentConsumer.accept(ingestDocument);
51-
return ingestDocument;
66+
return ingestDocumentMapper.apply(ingestDocument);
5267
}
5368

5469
@Override

test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java

+1
Original file line numberDiff line numberDiff line change
@@ -2002,6 +2002,7 @@ public String startMasterOnlyNode(Settings settings) {
20022002
.put(settings)
20032003
.put(Node.NODE_MASTER_SETTING.getKey(), true)
20042004
.put(Node.NODE_DATA_SETTING.getKey(), false)
2005+
.put(Node.NODE_INGEST_SETTING.getKey(), false)
20052006
.build();
20062007
return startNode(settings1);
20072008
}

0 commit comments

Comments
 (0)