-
Notifications
You must be signed in to change notification settings - Fork 25k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rethink template inheritance to allow for better validation #21105
Comments
Definitely a good idea IMO. In such a case, you know exactly what you are doing as a end user. |
I wonder if we really need to make it all that sophisticated. It might be enough for 99% of the usecases to just have a single |
I just discussed it with Simon since I was not clear how this would help. Since the perceived benefit of having multiple matching templates is to save some typing when putting them, the |
What if the A second question is how on Earth are you going to enforce the single association to a brand new index that just happens to be created by a document insertion request? You have to resort to limitations on the declared template patterns like "only non-overlapping prefixes" and such. I personally don't like artificial limits. |
We just ran into this trying to upgrade our cluster to 2.4. We essentially have one "root" template containing all our custom Probably worth mentioning this as a breaking change in the 2.4 migration docs, btw. |
I think explicit template dependencies could work. Requiring templates to be valid on their own would be a little difficult since we have a somewhat complex index/template strategy. We recently upgrade from ES 0.90 to 2.3.x and refactored our massive single template into many much smaller templates where we rely on inheritance to keep them concise and re-use shared parts. With 2.3.x we had to partition our indices by language, so we ended up with templates with specific responsibilities. Settings Analyzers/Filters Mappings Here's a simplified example of the templates: https://gist.github.com/mbarker/40bd82de268cd5d73d3788621c4b80df |
(subscribing) |
@s1monw I'm not sure that a single root dependency would work either. Like @mbarker we have a relatively complex index/template strategy that works pretty well for us and we'd like to keep it. In particular we tend to define global settings like compression and whether or not Inheritance is a certainly a feature we like and use regularly (albeit we've definitely been hit with a few gotchas, eg #20479) but I also agree that the explicit dependencies would be very easy to work with (and arguably a lot easier to write tests against too, which we also do). @acarstoiu brings up a good point about nested inheritance. For what it's worth, it would not be difficult at all for us to work without nested inheritance. |
We had a long discussion about this issue yesterday. We want to be able to validate index templates at |
If you:
what is the problem in validating a (sub)tree of templates when a parent template is changed? You're sparring the machine from doing more computations at the expense of more manual work necessary to multiply and keep in sync the common parts - which is itself error-prone. |
@jpountz would explicit inheritance simplify the validation at all? If not, I think it will be important to write a tool that generates explicit templates outside of ES (for example, in Bash/Python/Go/whatever) by using a hierarchy/import statements/something. I'd be happy to help out with said tool since, without inheritance, it's something a lot of us are going to need, myself included. One clarification if you don't mind: "clusters are expected to have a reasonable total number of templates, which means having some duplicate sections in multiple templates is not that much of an issue." Do you have an upper bound on what you'd consider reasonable? Is this expectation documented? My experience and yours seem quite different. I do expect this to be an issue for quite a few people. I actually do agree that there are good reasons to make the change you have in mind, and ways to work around it (additional tooling, mentioned above). Among the reasons this change makes sense, though, I'm personally not convinced "It shouldn't be much of an issue" is one of them. |
@jpountz is just a messenger here, so there's no reason to shoot him. Yet, at least 😆 |
Another possibility that has been put forward for discussion is to specify the template name in an indexing request. /cc @uboness |
That's a no go - it would add an extra thing to worry about in each and every indexing query. |
Yeah, that's not ideal. However, if that were optional that would be pretty interesting. |
Templates could also be validated/converted at upgrade time if we don't have to worry about merging. A migration plugin could also provide early warning about invalid templates ahead of time before an upgrade. |
I don't see this in the Breaking Changes for 6.0 yet. My company is still working on upgrading from 1.7 to 2.4. It seems like the simplest approach for us is to copy ES's merge logic into a custom tool (already done) and generate fully-qualified templates. If Index Templates really aren't going to be modular go forward, would it be possible to release an official tool that encapsulates the merge logic? Or perhaps a helper endpoint that we can send a list of templates and get back the combined version? I don't have any concrete API suggestions, but I'd be happy to discuss if there's interest. |
Template inheritance allows us to add aliasses to for example the default Beats index templates, like this:
Please don't deprecate template inheritance completely or else we will have to manually update each beats template after each upgrade. |
We've started to migrate away from nested templates, but we're finding it really hard to get away from a default level 0 template that contains our dynamic mapping settings. If you guys do remove template inheritance, would there be any way to add an option to templates to import one other template that contains common dynamic mapping config? It would not be too hard for us to be explicit about the fields we expect in a template.... but we really don't want to have to repeat the dynamic mapping rules in 100 different places if we can avoid it. |
For those who want to migrate to 5.5.1, I have ported the patch I made for 2.4.1 (see this comment and the next one) in order to make the template validation lenient with respect to analyzers. The port is here. |
It seems unreasonable to take away convenience and DRY in the name of dubiously-useful index template validation. The end result is that I will have to code the template merging myself. |
Pinging @elastic/es-core-infra |
Closing this as it's been resolved by composable index templates (#53101) |
Validation of index templates has proven difficult to get right due to how multiple templates that might be invalid on their own can result in a valid template in the end, for instance if two templates match a given index name and one of them is providing analysis index settings while the other one is providing mappings that make use of these analyzers. (#20479)
The fact that we cannot validate index templates is not good, so I think we need to think about how we could validate them all the time without false positives. We were discussing this issue yesterday and the following options have been suggested:
order
, a template could refer to other templates that should be included in order to not have to repeat the same bits over multiple templates. And again, we would probably only allow one template to be applied per index.The text was updated successfully, but these errors were encountered: