[Deprecation] Add transform_ids to outdated index #120821
Conversation
When a transform is writing to an outdated destination index, the transform's id will show up in the index's deprecation warning under `index_settings._meta.transform_ids`.
|
Hi @prwhelan, I've created a changelog YAML for you. |
|
Pinging @elastic/ml-core (Team:ML) |
| @@ -42,7 +47,7 @@ | |||
| public class TransportDeprecationInfoAction extends TransportMasterNodeReadAction< | |||
| DeprecationInfoAction.Request, | |||
| DeprecationInfoAction.Response> { | |||
| private static final List<DeprecationChecker> PLUGIN_CHECKERS = List.of(new MlDeprecationChecker(), new TransformDeprecationChecker()); | |||
| private static final DeprecationChecker ML_CHECKER = new MlDeprecationChecker(); | |||
There was a problem hiding this comment.
I don't know if we want to get rid of PLUGIN_CHECKERS, do we? I would not be surprised if some new ones get added soon (although I don't know of any). There were 3 in the 7->8 upgrade.
There was a problem hiding this comment.
We could add it back if you'd like? That would make line 132 look something like:
Stream.concat(PLUGIN_CHECKERS.stream(), Stream.of(new TransformDeprecationChecker(transformConfigs))).toList()
To account for the static and non-static objects
| @@ -172,4 +180,46 @@ static void pluginSettingIssues( | |||
| } | |||
| } | |||
|
|
|||
| private void transformConfigs(ActionListener<List<TransformConfig>> transformConfigsListener) { | |||
There was a problem hiding this comment.
Given that we've got a Client in the components that TransformDeprecationChecker receives, would it make sense to fetch the transform configs at the beginning of TransformDeprecationChecker::check instead, to avoid putting all of this ML-specific code into TransportDeprecationInfoAction (and the IndexDeprecationChecks API)? I haven't fully thought that through, so maybe there's a reason it has to be here?
There was a problem hiding this comment.
I agree that it's not great, this isn't my favorite PR.
I moved it from TransformDeprecationChecker because the index checks run after and now will now need to know about Transform Configs, so it felt like we should move the call up the stack so that the results can be shared between the two.
The alternative (that I thought of) was passing the TransformConfigs back from TransformDeprecationChecker and doing some sort instance of check to get the data out and pass it to the index check. I also tried moving all the index checks to their own DeprecationChecker but that was significantly messier.
Pretty much anything that writes to an index needs some way handle the reindexing (transforms, data streams, maybe ESQL one day), we just don't really have a pattern for that yet.
| assertEquals(singletonList(expected), issues); | ||
| } | ||
|
|
||
| public void testOldTransformIndicesCheck() { |
There was a problem hiding this comment.
Should we have tests for the case where multiple transform destination indices are outdated either for the same index or for different ones?
💔 Backport failed
You can use sqren/backport to manually backport by running |
When a transform is writing to an outdated destination index, the transform's id will show up in the index's deprecation warning under `index_settings._meta.transform_ids`.
When a transform is writing to an outdated destination index, the transform's id will show up in the index's deprecation warning under
index_settings._meta.transform_ids.Notes:
_metafield if the destination index is out of dateIndexDeprecationChecks now has its own declaration@FunctionalInterfacelike Node checksIndexDeprecationChecks now has its own Components blob object like the Plugin checks (hopefully making it easier to add more parameters the next time we have to do this)Testing done: