[ML] Update ML results mappings on process start #37706
Conversation
This change moves the update to the results index mappings from the open job action to the code that starts the autodetect process. When a rolling upgrade is performed we need to update the mappings for already-open jobs that are reassigned from an old version node to a new version node, but the open job action is not called in this case. Closes elastic#37607
|
Pinging @elastic/ml-core |
|
Although this PR appears quite large, almost all of it consists of shuffling existing code between files. The rolling upgrade test is really the only new code. |
| @@ -964,4 +985,95 @@ public static XContentBuilder auditMessageMapping() throws IOException { | |||
| .endObject() | |||
| .endObject(); | |||
| } | |||
|
|
|||
| static String[] mappingRequiresUpdate(ClusterState state, String[] concreteIndices, Version minVersion, | |||
| Logger logger) throws IOException { | |||
There was a problem hiding this comment.
I wonder if we need to pass down an external logger or just use the logger of ElasticsearchMappings.class instead.
There was a problem hiding this comment.
Sure, I changed that in the second commit.
| response -> { | ||
| addDocMappingIfMissing(AnomalyDetectorsIndex.jobStateIndexWriteAlias(), ElasticsearchMappings::stateMapping, | ||
| state, jobUpdateListener); | ||
| ElasticsearchMappings.addDocMappingIfMissing(AnomalyDetectorsIndex.jobStateIndexWriteAlias(), |
There was a problem hiding this comment.
Do we still need to call this from here? Isn't it enough to call it only in AutodetectProcessManager.openJob?
There was a problem hiding this comment.
That one is updating the state index mappings. The one I moved updates the results index mappings.
Since the state index mapping is simply enabled=false at the top level, in 7.x we don't need to update it at all - we know any state index that exists in 7.x is either a single type index from 6.x or else a typeless 7.x index. But in 6.x we might, if a user has just upgraded from 5.4 where we had a multi-type state index. Since this PR is being backported I didn't want to make a 7.x-only change in it. But I can do that in a followup.
…ead-de-duplication * elastic/master: (24 commits) [TEST] Mute MlMappingsUpgradeIT testMappingsUpgrade Streamline skip_unavailable handling (elastic#37672) Only bootstrap and elect node in current voting configuration (elastic#37712) Ensure either success or failure path for SearchOperationListener is called (elastic#37467) Target only specific index in update settings test Add a note how to benchmark Elasticsearch Don't use Groovy's `withDefault` (elastic#37726) Adapt SyncedFlushService (elastic#37691) Mute FilterAggregatorTests#testRandom Switch mapping/aggregations over to java time (elastic#36363) [ML] Update ML results mappings on process start (elastic#37706) Modify removal_of_types.asciidoc (elastic#37648) Fix edge case in PutMappingRequestTests (elastic#37665) Use new bulk API endpoint in the docs (elastic#37698) Expose sequence number and primary terms in search responses (elastic#37639) Remove LicenseServiceClusterNotRecoveredTests (elastic#37528) Migrate SpecificMasterNodesIT to Zen2 (elastic#37532) Fix MetaStateFormat tests Use plain text instead of latexmath Fix a typo in a warning message in TestFixturesPlugin (elastic#37631) ...
This change moves the update to the results index mappings
from the open job action to the code that starts the
autodetect process.
When a rolling upgrade is performed we need to update the
mappings for already-open jobs that are reassigned from an
old version node to a new version node, but the open job
action is not called in this case.
Closes #37607