Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix metadata migration logic error when update setting call failed #328

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

bowenlan-amzn
Copy link
Member

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 in the end which is unwanted.

Signed-off-by: bowenlan-amzn [email protected]

Issue #, if available:

Description of changes:

CheckList:

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: bowenlan-amzn <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #328 (53b7427) into main (34b480c) will decrease coverage by 0.13%.
The diff coverage is 53.84%.

@@             Coverage Diff              @@
##               main     #328      +/-   ##
============================================
- Coverage     76.71%   76.57%   -0.14%     
- Complexity     2044     2046       +2     
============================================
  Files           253      253              
  Lines         11636    11642       +6     
  Branches       1807     1808       +1     
============================================
- Hits           8926     8915      -11     
- Misses         1742     1762      +20     
+ Partials        968      965       -3     
Impacted Files Coverage Δ
...management/indexstatemanagement/MetadataService.kt 62.68% <53.84%> (+3.31%) ⬆️
...statemanagement/model/destination/CustomWebhook.kt 67.14% <0.00%> (-28.58%) ⬇️
.../rollup/action/start/TransportStartRollupAction.kt 71.76% <0.00%> (-3.53%) ⬇️
...anagement/indexstatemanagement/model/Transition.kt 63.01% <0.00%> (-2.74%) ⬇️
...ent/indexstatemanagement/util/ManagedIndexUtils.kt 75.94% <0.00%> (-1.89%) ⬇️
...nt/rollup/action/stop/TransportStopRollupAction.kt 76.47% <0.00%> (-1.18%) ⬇️
.../opensearch/indexmanagement/rollup/model/Rollup.kt 86.04% <0.00%> (-0.47%) ⬇️
...nt/indexstatemanagement/ManagedIndexCoordinator.kt 75.84% <0.00%> (+0.61%) ⬆️
...earch/indexmanagement/transform/model/Transform.kt 86.32% <0.00%> (+0.85%) ⬆️
...exmanagement/opensearchapi/OpenSearchExtensions.kt 87.50% <0.00%> (+1.13%) ⬆️
... and 4 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 34b480c...53b7427. Read the comment docs.

@thalurur
Copy link
Contributor

thalurur commented Apr 13, 2022

Can we also include the exists check in template migration? -

@bowenlan-amzn
Copy link
Member Author

Can we also include the exists check in template migration? -

Since template migration migrates from index template, config index not existing doesn't really mean there is no related index template, so I didn't add this check this time.

@bowenlan-amzn bowenlan-amzn merged commit f4a3938 into opensearch-project:main Apr 14, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 14, 2022
)

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: bowenlan-amzn <[email protected]>
(cherry picked from commit f4a3938)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 14, 2022
)

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: bowenlan-amzn <[email protected]>
(cherry picked from commit f4a3938)
bowenlan-amzn added a commit that referenced this pull request Apr 16, 2022
)

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: Bowen Lan <[email protected]>
(cherry picked from commit f4a3938)
bowenlan-amzn added a commit that referenced this pull request Apr 16, 2022
)

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: Bowen Lan <[email protected]>
(cherry picked from commit f4a3938)
bowenlan-amzn added a commit that referenced this pull request Apr 17, 2022
) (#329)

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: Bowen Lan <[email protected]>
(cherry picked from commit f4a3938)

Co-authored-by: Bowen Lan <[email protected]>
bowenlan-amzn added a commit that referenced this pull request Apr 17, 2022
) (#330)

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: Bowen Lan <[email protected]>
(cherry picked from commit f4a3938)

Co-authored-by: Bowen Lan <[email protected]>
@bowenlan-amzn bowenlan-amzn deleted the mdmigrationfix branch November 16, 2022 07:55
wuychn pushed a commit to ochprince/index-management that referenced this pull request Mar 16, 2023
…pensearch-project#328)

If update setting to 1 failed, runTimeCounter shouldn't be increased.
Otherwise it will increase to be larger than 10, and start to update
setting to -1 which is unwanted.

Signed-off-by: bowenlan-amzn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants