Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

move metadata to config index #368

Closed
wants to merge 5 commits into from
Closed

move metadata to config index #368

wants to merge 5 commits into from

Conversation

bowenlan-amzn
Copy link
Contributor

@bowenlan-amzn bowenlan-amzn commented Jan 4, 2021

Issue #, if available:
#207

Description of changes:
Previously we save job metadata to cluster state, which will be a performance constraint when there are thousands of parallel running job. Now we save job metadata to our configuration index to relieve this performance concern.

  • What if cluster has 2 version of ISM plugin (like during upgrading)
    • The node with new version ISM plugin will skip execution if there is node with old version ISM plugin
    • The node with old version ISM plugin can still run ISM job
    • After all nodes upgraded, our new version ISM plugin will move metadata from cluster state to config index, and delete metadata left in cluster state

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Jan 4, 2021

Codecov Report

Merging #368 (e7fa52d) into opendistro-1.11 (4640f2c) will increase coverage by 0.27%.
The diff coverage is 84.11%.

Impacted file tree graph

@@                  Coverage Diff                  @@
##             opendistro-1.11     #368      +/-   ##
=====================================================
+ Coverage              76.08%   76.36%   +0.27%     
- Complexity               873      891      +18     
=====================================================
  Files                    123      124       +1     
  Lines                   4533     4794     +261     
  Branches                 676      701      +25     
=====================================================
+ Hits                    3449     3661     +212     
- Misses                   713      746      +33     
- Partials                 371      387      +16     
Impacted Files Coverage Δ Complexity Δ
...ndexstatemanagement/IndexStateManagementHistory.kt 77.88% <ø> (+1.92%) 28.00 <0.00> (+1.00)
...exstatemanagement/settings/ManagedIndexSettings.kt 98.71% <ø> (+0.12%) 1.00 <0.00> (ø)
...ement/model/managedindexmetadata/ActionMetaData.kt 84.41% <33.33%> (-6.50%) 12.00 <0.00> (ø)
...agement/model/managedindexmetadata/StepMetaData.kt 66.66% <60.00%> (-11.91%) 7.00 <1.00> (ø)
...agement/indexstatemanagement/ManagedIndexRunner.kt 57.92% <71.08%> (+2.26%) 50.00 <9.00> (+9.00)
...exmanagement/indexstatemanagement/SkipExecution.kt 80.95% <80.95%> (ø) 6.00 <6.00> (?)
...nt/indexstatemanagement/ManagedIndexCoordinator.kt 67.46% <84.21%> (+5.98%) 35.00 <7.00> (ø)
...dexstatemanagement/elasticapi/ElasticExtensions.kt 76.78% <85.48%> (+15.24%) 0.00 <0.00> (ø)
...gedindex/TransportRetryFailedManagedIndexAction.kt 75.26% <86.20%> (+4.00%) 2.00 <0.00> (ø)
...action/changepolicy/TransportChangePolicyAction.kt 77.96% <87.75%> (+1.44%) 4.00 <0.00> (ø)
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4640f2c...e7fa52d. Read the comment docs.

@bowenlan-amzn bowenlan-amzn added the enhancement An improvement on the existing feature’s functionalities label Jan 5, 2021
@dbbaughe
Copy link
Contributor

dbbaughe commented Jan 5, 2021

High level comment - worried about perf issues by moving it into config index. Job scheduler does have a postIndex subscriber to the config index which is executed every time a doc is added/updated. Imagining someone with 10k indices every 1 minute which means 20k updates per minute (each ISM execution ends up doing 2 updates to metadata for starting/closing a transaction) which are all triggering that callback which then has to check if it's of a job type to schedule (which it isn't). Alternatively we can consider another ism index which is purely for metadata which won't have this listener attached... we could have both rollup and ism metadata in there instead of the config index. Some testing could be good to see if this is a real concern.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement An improvement on the existing feature’s functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants