Resolve pipeline on lazy rollover write #115987
Conversation
| if (indexRequest.isPipelineResolved() == false) { | ||
| // Resolve the pipeline from setting or templates if not cached | ||
| var pipelines = storedPipelineCache.computeIfAbsent( | ||
| indexRequest.index(), |
There was a problem hiding this comment.
This is now only looking up pipelines per index (rather than per index request) for all requests. So this will apply whether or not lazy rollover is set.
| @@ -228,13 +227,10 @@ private boolean applyPipelines(Task task, BulkRequest bulkRequest, Executor exec | |||
| metadata = clusterService.state().getMetadata(); | |||
| } | |||
|
|
|||
| for (DocWriteRequest<?> actionRequest : bulkRequest.requests) { | |||
There was a problem hiding this comment.
Refactored this, pushing the loop down into IngestService. Did this so that storedPipelineCache didn't need to be passed down into IngestService.resolvePipelinesAndUpdateIndexRequest. But, this required iterating over the request twice, once withing IngestService.resolveAndUpdateAllPipelines and once below.
There was a problem hiding this comment.
Do y'all think this is a worthwhile refactoring? Or better to just pass storedPipelineCache into IngestService.resolvePipelinesAndUpdateIndexRequest and return the resolved pipeline from IngestService.resolvePipelinesAndUpdateIndexRequest so it can be set in the cache within the main loop.
Though I like the refactoring, resolvePipelinesAndUpdateIndexRequest has some nice test coverage that I'm disappointed to no longer be using.
There was a problem hiding this comment.
This is an alternative approach without this refactoring: https://github.com/elastic/elasticsearch/pull/116031/files
There was a problem hiding this comment.
Since this is pretty performance sensitive code, I think it makes sense to avoid the extra looping
There was a problem hiding this comment.
@dakrone So you prefer the approach in https://github.com/elastic/elasticsearch/pull/116031/files ?
|
Replacing this PR with the work in #116031 |
If lazy rollover is set, when write with bulk request, always resolve default (and final) pipeline from index templates.
Related to #112781