Pass the settings from non-overruling providers to overruling ones. #115397
Conversation
kkrik-es
added
>non-issue
Team:StorageEngine
:StorageEngine/Logs
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
kkrik-es
added
auto-backport
kkrik-es
changed the title
|
|
||
| // Feed the settings generated by non-overruling to overruling setting providers, | ||
| // along with template and request settings. | ||
| final Settings templateAndRequestAndProviderSettings = settingsBuilder.put(additionalIndexSettings.build()).build(); |
There was a problem hiding this comment.
Does the order matter here? I mean the fact that we apply additionalIndexSettings after request.settings().
There was a problem hiding this comment.
It does, we'd need to filter conflicting ones. I'll add something if we decide to proceed with this.
| templateAndRequestAndProviderSettings, | ||
| combinedTemplateMappings | ||
| ); | ||
| additionalIndexSettings.put(newAdditionalSettings); |
There was a problem hiding this comment.
I think so, additionalIndexSettings contains all provided settings and it's used below.
|
I like the simplicity of #115437 but it is based on our assumptions of how the code is working. IMO this approach is more robust and given that it does not add much complexity i am in favour of this. |
|
Hey @dakrone, we wanted to get your take on this.. We have cases where the output of one index settings provider is used as input for another one (chaining). The particular blocker for logsdb was addressed in #115437, so we were wondering if we should take the time to pursue a more robust solution. In this PR, I'm introducing two phases, with providers marked as non-overruling providing inputs to overruling ones. This covers our case and any pair of chained providers, but can get us so far. Another option is to introduce a priority per provider and start from lower-priority ones, passing their outputs to the following ones. We'd need to see how to efficiently create I was wondering if you could take some time to look at this and provide guidance on how you'd rather proceed. As mentioned, this is not high priority as we have a workaround. |
Can you elaborate a little bit on why these can't be put into a single index provider? If at all possible, it feels cleaner to me to have each provider be independent from the other, so that we don't have to have ordering or priorities at all. |
That can certainly work, though it would have somewhat confusing semantics. Each provider can apply on its own, and they produce different settings (logsdb index mode vs stored source mode). There's a cross-section where they both apply (based on the index name |
|
Martijn and I talked a little bit about this offline. I think my preference would go in order of:
Of course, this is just my own preference, not something we necessarily have to do. |
This is needed for chaining providers, e.g. logsdb provider setting the index mode, then the synthetic source provider deciding to downgrade to stored source.
The plan is to initially cover just index create, to unblock testing, then separately refactor the index setting merging logic to a dedicated class and use it in other places too.