Open jtibshirani opened 3 years ago
Pinging @elastic/es-core-infra (Team:Core/Infra)
In our discussion, we noted that the index version is technically already available in Validator#validate
, since we pass in the complete settings (which includes the version). We decided to work through examples in order to determine exactly when/ how the version is needed. Here are some recent index settings changes I'm aware of (note that by accident we didn't always handle bwc precisely):
index.optimize_auto_generated_id
index.unassigned.node_left.delayed_timeout
fix
option from index.shard.check_on_startup
In the first two cases, settings were deprecated in n-1 then removed entirely in n. So here we would keep the setting definition, but check the version during validation. In the last two cases, we start to disallow certain values: in n indices the values will be outright rejected, but for n-1 indices we'll accept them and use a fallback. For example, when we encounter fix
for index.shard.check_on_startup
we would log a warning and use the behavior for false
instead. The latter case currently wouldn't be covered by the Validator#validate
.
For example, when we encounter fix for index.shard.check_on_startup we would log a warning and use the behavior for false instead. The latter case currently wouldn't be covered by the Validator#validate.
In this case, I think the calling code would need to do the translation. So the code that looks at index.shard.check_on_startup
would see the fix
value, and in 8.0 it would change the behavior to act like false
. Wdyt @jtibshirani?
I think it'd be valuable to consolidate + standardize the places where we access the index version (as suggested in #65399), and encapsulating this in the settings framework would be in line with that goal. Some downsides I see with the calling code being responsible:
While I understand the concern over duplication of the compatibility logic, in practice I think we usually have a single place reading settings. If there are multiple places needing to read the same setting, we can share utility code to read that setting.
However, I'm not sure what I suggested here would require compatibility logic on the read side in this case. With the case of the fix
value, the code reading the setting would continue to have logic for the fix
value. The limitation of which indexes can contain the fix value would be limited to the validator. Then once there are no longer any indices that could legitimately use the fix
value, the reading code can be adjusted to no longer need this case.
I don't think we should complete the settings infrastructure with Version, since not all settings are index settings. We could in theory create wrappers for all index setting types, but I think that would be quite complex for the relatively few number of cases we have for modifying the behavior for compatibility.
Our index compatibility policy requires that indices created in major version n-1 continue to work in version n without manual intervention, which means we must be able to read index metadata that was valid in n-1. We haven't always followed this policy closely for index settings. It hasn't been a big problem in the past, perhaps because we archive all unknown and invalid settings when a node starts up. We plan to remove this leniency on start-up and would like to uphold clearer compatibility guarantees.
To support this, the behavior of index settings must be able to depend on the index version. That way if a setting value is deprecated in 7.x, we can reject it for new 8.x indices but continue to allow it for 7.x indices (maybe falling back to another value). Perhaps there should be a mechanism for validating settings that has access to the index version. Note this set-up would only be relevant to 'index scoped' settings, not
Setting
objects in general.