[ML] Automatically rollover legacy ml indices #120405
Conversation
davidkyle
added
>upgrade
:ml
|
Pinging @elastic/ml-core (Team:ML) |
|
Hi @davidkyle, I've created a changelog YAML for you. |
valeriy42
added
the
cloud-deploy
| public boolean isMinTransportVersionSupported(TransportVersion minTransportVersion) { | ||
| // Automatic rollover does not require any new features | ||
| // but wait for all nodes to be upgraded anyway | ||
| return minTransportVersion.onOrAfter(TransportVersions.ML_ROLLOVER_LEGACY_INDICES); |
There was a problem hiding this comment.
this will effectively wait until the entire cluster is on 8.x or later, guaranteeing that the index will be created as v8?
There was a problem hiding this comment.
Yes, I've updated the comment to that effect
| PlainActionFuture<Boolean> rolloverIndices = new PlainActionFuture<>(); | ||
| rolloverLegacyIndices(latestState, indexPatternAndAlias.indexPattern(), indexPatternAndAlias.alias(), rolloverIndices); | ||
| try { | ||
| rolloverIndices.actionGet(); |
There was a problem hiding this comment.
Is there any risk of a deadlock here? Or is that avoided because the rolloverLegacyIndices and submethods are answered on a transport/management thread, and this waiting thread is an ML utility thread? Asking because I feel like refactoring this would be tricky, since I think SubscribableListener doesn't have a good way to ignore failures and carry on with the next call
There was a problem hiding this comment.
Is there any risk of a deadlock here? Or is that avoided because the rolloverLegacyIndices and submethods are answered on a transport/management thread,
This got me thinking 🫢
The good news is that PlainActionFuture has assertions that throw if the request could deadlock due to threadpool exhaustion https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/action/support/PlainActionFuture.java#L372
But this code path isn't testing in CI because we don't have testing route for 7 -> 8 -> 9 so I hacked the code to always rollover the index and ran an 8 -> 9 upgrade test. See commit 97995d4
I also added an assertion to check which thread the response comes from and it is the clusterApplierService#updateTask thread not one of the ml_utility threads. This makes sense as rollover and alias are master node actions and this code only runs on the master node. There is very little work performed on the responding thread it basically completes the listener and that is ok to do on a cluster update thread.
The for loop in runUpdate(ClusterState) is executed on a ml_utility thread so the chain of actions for each index is started on that thread and it is the ml_utility thread is the one blocked by the actionGet().
Blocking with actionGet() is not usually recommended but in this case it has the advantage that requests are processed serially instead of firing off a bunch of requests at once. I'm not sure it is worth a complicated refactor to completely unblock the code.
💚 Backport successful
|
Rollover ml indices created in 7.x and create new indices that version 9 can read and write to. This is required for ml to continue to run after during upgrade and reindex of 7.x indices
* [ML] Automatically rollover legacy ml indices (#120405) Rollover ml indices created in 7.x and create new indices that version 9 can read and write to. This is required for ml to continue to run after during upgrade and reindex of 7.x indices * fix for backport * annotations index can be ignored * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]>
Indices created in 7.x cannot be written to in 9. For ml to continue working in 9 any pre-8 indices must be rolled over and a new index created. Non legacy (i.e. created in 8) indices are not rolled over. This PR uses the existing
MlAutoUpdateServiceto trigger the rollover. It will happen soon after the cluster is upgraded.The indices rolled over in this PR are
TODO
The remaining ml associated indices need extra work which is not covered in this PR