Skip to content

Commit 737376d

Browse files
authored
Make ingest pipeline resolution logic unit testable (#47026)
Extracted ingest pipeline resolution logic into a static method and added unit tests for pipeline resolution logic. Followup from #46847
1 parent 65d4124 commit 737376d

File tree

2 files changed

+267
-106
lines changed

2 files changed

+267
-106
lines changed

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

+109-106
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
import org.elasticsearch.cluster.metadata.MetaData;
5555
import org.elasticsearch.cluster.metadata.MetaDataIndexTemplateService;
5656
import org.elasticsearch.cluster.service.ClusterService;
57-
import org.elasticsearch.common.collect.ImmutableOpenMap;
5857
import org.elasticsearch.common.inject.Inject;
5958
import org.elasticsearch.common.settings.Settings;
6059
import org.elasticsearch.common.unit.TimeValue;
@@ -155,115 +154,13 @@ protected void doExecute(Task task, BulkRequest bulkRequest, ActionListener<Bulk
155154

156155
boolean hasIndexRequestsWithPipelines = false;
157156
final MetaData metaData = clusterService.state().getMetaData();
158-
ImmutableOpenMap<String, IndexMetaData> indicesMetaData = metaData.indices();
159157
for (DocWriteRequest<?> actionRequest : bulkRequest.requests) {
160158
IndexRequest indexRequest = getIndexWriteRequest(actionRequest);
161-
162159
if (indexRequest != null) {
163-
if (indexRequest.isPipelineResolved() == false) {
164-
final String requestPipeline = indexRequest.getPipeline();
165-
indexRequest.setPipeline(IngestService.NOOP_PIPELINE_NAME);
166-
boolean requestCanOverridePipeline = true;
167-
String requiredPipeline = null;
168-
// start to look for default or required pipelines via settings found in the index meta data
169-
IndexMetaData indexMetaData = indicesMetaData.get(actionRequest.index());
170-
// check the alias for the index request (this is how normal index requests are modeled)
171-
if (indexMetaData == null && indexRequest.index() != null) {
172-
AliasOrIndex indexOrAlias = metaData.getAliasAndIndexLookup().get(indexRequest.index());
173-
if (indexOrAlias != null && indexOrAlias.isAlias()) {
174-
AliasOrIndex.Alias alias = (AliasOrIndex.Alias) indexOrAlias;
175-
indexMetaData = alias.getWriteIndex();
176-
}
177-
}
178-
// check the alias for the action request (this is how upserts are modeled)
179-
if (indexMetaData == null && actionRequest.index() != null) {
180-
AliasOrIndex indexOrAlias = metaData.getAliasAndIndexLookup().get(actionRequest.index());
181-
if (indexOrAlias != null && indexOrAlias.isAlias()) {
182-
AliasOrIndex.Alias alias = (AliasOrIndex.Alias) indexOrAlias;
183-
indexMetaData = alias.getWriteIndex();
184-
}
185-
}
186-
if (indexMetaData != null) {
187-
final Settings indexSettings = indexMetaData.getSettings();
188-
if (IndexSettings.REQUIRED_PIPELINE.exists(indexSettings)) {
189-
// find the required pipeline if one is defined from an existing index
190-
requiredPipeline = IndexSettings.REQUIRED_PIPELINE.get(indexSettings);
191-
assert IndexSettings.DEFAULT_PIPELINE.get(indexSettings).equals(IngestService.NOOP_PIPELINE_NAME) :
192-
IndexSettings.DEFAULT_PIPELINE.get(indexSettings);
193-
indexRequest.setPipeline(requiredPipeline);
194-
requestCanOverridePipeline = false;
195-
} else {
196-
// find the default pipeline if one is defined from an existing index
197-
String defaultPipeline = IndexSettings.DEFAULT_PIPELINE.get(indexSettings);
198-
indexRequest.setPipeline(defaultPipeline);
199-
}
200-
} else if (indexRequest.index() != null) {
201-
// the index does not exist yet (and is valid request), so match index templates to look for a default pipeline
202-
List<IndexTemplateMetaData> templates = MetaDataIndexTemplateService.findTemplates(metaData, indexRequest.index());
203-
assert (templates != null);
204-
// order of templates are highest order first, we have to iterate through them all though
205-
String defaultPipeline = null;
206-
for (IndexTemplateMetaData template : templates) {
207-
final Settings settings = template.settings();
208-
if (requiredPipeline == null && IndexSettings.REQUIRED_PIPELINE.exists(settings)) {
209-
requiredPipeline = IndexSettings.REQUIRED_PIPELINE.get(settings);
210-
requestCanOverridePipeline = false;
211-
// we can not break in case a lower-order template has a default pipeline that we need to reject
212-
} else if (defaultPipeline == null && IndexSettings.DEFAULT_PIPELINE.exists(settings)) {
213-
defaultPipeline = IndexSettings.DEFAULT_PIPELINE.get(settings);
214-
// we can not break in case a lower-order template has a required pipeline that we need to reject
215-
}
216-
}
217-
if (requiredPipeline != null && defaultPipeline != null) {
218-
// we can not have picked up a required and a default pipeline from applying templates
219-
final String message = String.format(
220-
Locale.ROOT,
221-
"required pipeline [%s] and default pipeline [%s] can not both be set",
222-
requiredPipeline,
223-
defaultPipeline);
224-
throw new IllegalArgumentException(message);
225-
}
226-
final String pipeline;
227-
if (requiredPipeline != null) {
228-
pipeline = requiredPipeline;
229-
} else {
230-
pipeline = Objects.requireNonNullElse(defaultPipeline, IngestService.NOOP_PIPELINE_NAME);
231-
}
232-
indexRequest.setPipeline(pipeline);
233-
}
234-
235-
if (requestPipeline != null) {
236-
if (requestCanOverridePipeline == false) {
237-
final String message = String.format(
238-
Locale.ROOT,
239-
"request pipeline [%s] can not override required pipeline [%s]",
240-
requestPipeline,
241-
requiredPipeline);
242-
throw new IllegalArgumentException(message);
243-
} else {
244-
indexRequest.setPipeline(requestPipeline);
245-
}
246-
}
247-
248-
if (IngestService.NOOP_PIPELINE_NAME.equals(indexRequest.getPipeline()) == false) {
249-
hasIndexRequestsWithPipelines = true;
250-
}
251-
/*
252-
* We have to track whether or not the pipeline for this request has already been resolved. It can happen that the
253-
* pipeline for this request has already been derived yet we execute this loop again. That occurs if the bulk request
254-
* has been forwarded by a non-ingest coordinating node to an ingest node. In this case, the coordinating node will have
255-
* already resolved the pipeline for this request. It is important that we are able to distinguish this situation as we
256-
* can not double-resolve the pipeline because we will not be able to distinguish the case of the pipeline having been
257-
* set from a request pipeline parameter versus having been set by the resolution. We need to be able to distinguish
258-
* these cases as we need to reject the request if the pipeline was set by a required pipeline and there is a request
259-
* pipeline parameter too.
260-
*/
261-
indexRequest.isPipelineResolved(true);
262-
} else if (IngestService.NOOP_PIPELINE_NAME.equals(indexRequest.getPipeline()) == false) {
263-
hasIndexRequestsWithPipelines = true;
264-
}
160+
// Each index request needs to be evaluated, because this method also modifies the IndexRequest
161+
boolean indexRequestHasPipeline = resolveRequiredOrDefaultPipeline(actionRequest, indexRequest, metaData);
162+
hasIndexRequestsWithPipelines |= indexRequestHasPipeline;
265163
}
266-
267164
}
268165

269166
if (hasIndexRequestsWithPipelines) {
@@ -359,6 +256,112 @@ public void onFailure(Exception e) {
359256
}
360257
}
361258

259+
static boolean resolveRequiredOrDefaultPipeline(DocWriteRequest<?> originalRequest,
260+
IndexRequest indexRequest,
261+
MetaData metaData) {
262+
263+
if (indexRequest.isPipelineResolved() == false) {
264+
final String requestPipeline = indexRequest.getPipeline();
265+
indexRequest.setPipeline(IngestService.NOOP_PIPELINE_NAME);
266+
boolean requestCanOverridePipeline = true;
267+
String requiredPipeline = null;
268+
// start to look for default or required pipelines via settings found in the index meta data
269+
IndexMetaData indexMetaData = metaData.indices().get(originalRequest.index());
270+
// check the alias for the index request (this is how normal index requests are modeled)
271+
if (indexMetaData == null && indexRequest.index() != null) {
272+
AliasOrIndex indexOrAlias = metaData.getAliasAndIndexLookup().get(indexRequest.index());
273+
if (indexOrAlias != null && indexOrAlias.isAlias()) {
274+
AliasOrIndex.Alias alias = (AliasOrIndex.Alias) indexOrAlias;
275+
indexMetaData = alias.getWriteIndex();
276+
}
277+
}
278+
// check the alias for the action request (this is how upserts are modeled)
279+
if (indexMetaData == null && originalRequest.index() != null) {
280+
AliasOrIndex indexOrAlias = metaData.getAliasAndIndexLookup().get(originalRequest.index());
281+
if (indexOrAlias != null && indexOrAlias.isAlias()) {
282+
AliasOrIndex.Alias alias = (AliasOrIndex.Alias) indexOrAlias;
283+
indexMetaData = alias.getWriteIndex();
284+
}
285+
}
286+
if (indexMetaData != null) {
287+
final Settings indexSettings = indexMetaData.getSettings();
288+
if (IndexSettings.REQUIRED_PIPELINE.exists(indexSettings)) {
289+
// find the required pipeline if one is defined from an existing index
290+
requiredPipeline = IndexSettings.REQUIRED_PIPELINE.get(indexSettings);
291+
assert IndexSettings.DEFAULT_PIPELINE.get(indexSettings).equals(IngestService.NOOP_PIPELINE_NAME) :
292+
IndexSettings.DEFAULT_PIPELINE.get(indexSettings);
293+
indexRequest.setPipeline(requiredPipeline);
294+
requestCanOverridePipeline = false;
295+
} else {
296+
// find the default pipeline if one is defined from an existing index
297+
String defaultPipeline = IndexSettings.DEFAULT_PIPELINE.get(indexSettings);
298+
indexRequest.setPipeline(defaultPipeline);
299+
}
300+
} else if (indexRequest.index() != null) {
301+
// the index does not exist yet (and is valid request), so match index templates to look for a default pipeline
302+
List<IndexTemplateMetaData> templates = MetaDataIndexTemplateService.findTemplates(metaData, indexRequest.index());
303+
assert (templates != null);
304+
// order of templates are highest order first, we have to iterate through them all though
305+
String defaultPipeline = null;
306+
for (IndexTemplateMetaData template : templates) {
307+
final Settings settings = template.settings();
308+
if (requiredPipeline == null && IndexSettings.REQUIRED_PIPELINE.exists(settings)) {
309+
requiredPipeline = IndexSettings.REQUIRED_PIPELINE.get(settings);
310+
requestCanOverridePipeline = false;
311+
// we can not break in case a lower-order template has a default pipeline that we need to reject
312+
} else if (defaultPipeline == null && IndexSettings.DEFAULT_PIPELINE.exists(settings)) {
313+
defaultPipeline = IndexSettings.DEFAULT_PIPELINE.get(settings);
314+
// we can not break in case a lower-order template has a required pipeline that we need to reject
315+
}
316+
}
317+
if (requiredPipeline != null && defaultPipeline != null) {
318+
// we can not have picked up a required and a default pipeline from applying templates
319+
final String message = String.format(
320+
Locale.ROOT,
321+
"required pipeline [%s] and default pipeline [%s] can not both be set",
322+
requiredPipeline,
323+
defaultPipeline);
324+
throw new IllegalArgumentException(message);
325+
}
326+
final String pipeline;
327+
if (requiredPipeline != null) {
328+
pipeline = requiredPipeline;
329+
} else {
330+
pipeline = Objects.requireNonNullElse(defaultPipeline, IngestService.NOOP_PIPELINE_NAME);
331+
}
332+
indexRequest.setPipeline(pipeline);
333+
}
334+
335+
if (requestPipeline != null) {
336+
if (requestCanOverridePipeline == false) {
337+
final String message = String.format(
338+
Locale.ROOT,
339+
"request pipeline [%s] can not override required pipeline [%s]",
340+
requestPipeline,
341+
requiredPipeline);
342+
throw new IllegalArgumentException(message);
343+
} else {
344+
indexRequest.setPipeline(requestPipeline);
345+
}
346+
}
347+
348+
/*
349+
* We have to track whether or not the pipeline for this request has already been resolved. It can happen that the
350+
* pipeline for this request has already been derived yet we execute this loop again. That occurs if the bulk request
351+
* has been forwarded by a non-ingest coordinating node to an ingest node. In this case, the coordinating node will have
352+
* already resolved the pipeline for this request. It is important that we are able to distinguish this situation as we
353+
* can not double-resolve the pipeline because we will not be able to distinguish the case of the pipeline having been
354+
* set from a request pipeline parameter versus having been set by the resolution. We need to be able to distinguish
355+
* these cases as we need to reject the request if the pipeline was set by a required pipeline and there is a request
356+
* pipeline parameter too.
357+
*/
358+
indexRequest.isPipelineResolved(true);
359+
}
360+
361+
// Return whether this index request has a pipeline
362+
return IngestService.NOOP_PIPELINE_NAME.equals(indexRequest.getPipeline()) == false;
363+
}
364+
362365
boolean needToCheck() {
363366
return autoCreateIndex.needToCheck();
364367
}

0 commit comments

Comments
 (0)