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

Strengthen scroll search in Coordinator #356

Merged
merged 2 commits into from
May 12, 2022

Conversation

bowenlan-amzn
Copy link
Member

@bowenlan-amzn bowenlan-amzn commented May 10, 2022

Signed-off-by: bowenlan-amzn bowenlan23@gmail.com

Issue #, if available:

Description of changes:

Normally user won't have >10k ISM jobs, so we don't want to use scroll search every time.

For getting accurate total hits, there's a param track_total_hits, just keep a note here.

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.

@codecov-commenter
Copy link

codecov-commenter commented May 10, 2022

Codecov Report

Merging #356 (0d213a7) into main (a7fd6c4) will decrease coverage by 0.07%.
The diff coverage is 26.31%.

@@             Coverage Diff              @@
##               main     #356      +/-   ##
============================================
- Coverage     75.09%   75.01%   -0.08%     
- Complexity     2135     2141       +6     
============================================
  Files           262      262              
  Lines         12444    12461      +17     
  Branches       1966     1969       +3     
============================================
+ Hits           9345     9348       +3     
- Misses         2028     2040      +12     
- Partials       1071     1073       +2     
Impacted Files Coverage Δ
...nt/indexstatemanagement/ManagedIndexCoordinator.kt 68.71% <15.62%> (-6.52%) ⬇️
...ent/indexstatemanagement/util/ManagedIndexUtils.kt 77.87% <83.33%> (+1.53%) ⬆️
...arch/indexmanagement/rollup/RollupSearchService.kt 57.40% <0.00%> (-3.71%) ⬇️
.../rollup/action/start/TransportStartRollupAction.kt 72.94% <0.00%> (-2.36%) ⬇️
...earch/indexmanagement/transform/model/Transform.kt 85.89% <0.00%> (+0.42%) ⬆️
.../opensearch/indexmanagement/rollup/model/Rollup.kt 86.51% <0.00%> (+0.93%) ⬆️
...ndexstatemanagement/IndexStateManagementHistory.kt 80.00% <0.00%> (+1.37%) ⬆️
...gement/indexstatemanagement/action/ShrinkAction.kt 69.11% <0.00%> (+1.47%) ⬆️
...anagement/indexstatemanagement/model/Transition.kt 65.75% <0.00%> (+4.10%) ⬆️
... and 1 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 a7fd6c4...0d213a7. Read the comment docs.

@bowenlan-amzn bowenlan-amzn marked this pull request as ready for review May 10, 2022 18:38
@bowenlan-amzn bowenlan-amzn requested a review from a team May 10, 2022 18:38
Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
.indices(INDEX_MANAGEMENT_INDEX)
.scroll(TimeValue.timeValueMinutes(1))
val req = SearchRequest().indices(INDEX_MANAGEMENT_INDEX)
.allowPartialSearchResults(false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My feeling is that this line alone would be enough for the fix. Do you not trust it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's supposed to throw exception. But I just don't know every thing for scroll search, so still going to check the response failure here

@bowenlan-amzn
Copy link
Member Author

The failed test looks to be a race condition

REPRODUCE WITH: ./gradlew ':integTest' --tests "org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionIT.test basic" -Dtests.seed=2B4F0CB850D3EE5B -Dtests.security.manager=false -Dtests.locale=cs -Dtests.timezone=US/Aleutian -Druntime.java=11
  1> [2022-05-10T14:06:07,842][INFO ][o.o.i.i.a.DeleteActionIT ] [test basic] before test

Suite: Test class org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionIT
  2> REPRODUCE WITH: ./gradlew ':integTest' --tests "org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionIT.test basic" -Dtests.seed=2B4F0CB850D3EE5B -Dtests.security.manager=false -Dtests.locale=cs -Dtests.timezone=US/Aleutian -Druntime.java=11
  2> java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([2B4F0CB850D3EE5B:3864805276420A8F]:0)
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionIT$test basic$4.invoke(DeleteActionIT.kt:58)
        at org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionIT$test basic$4.invoke(DeleteActionIT.kt:56)
        at org.opensearch.indexmanagement.TestHelpersKt.waitFor(TestHelpers.kt:119)
        at org.opensearch.indexmanagement.TestHelpersKt.waitFor$default(TestHelpers.kt:112)
        at org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionIT.test basic(DeleteActionIT.kt:56)
  2> NOTE: leaving temporary files on disk at: /home/runner/work/index-management/index-management/build/testrun/integTest/temp/org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionIT_2B4F0CB850D3EE5B-001
  2> NOTE: test params are: codec=Asserting(Lucene91): {}, docValues:{}, maxPointsInLeafNode=1268, maxMBSortInHeap=5.426779553062035, sim=Asserting(RandomSimilarity(queryNorm=true): {}), locale=cs, timezone=US/Aleutian
  2> NOTE: Linux 5.13.0-1022-azure amd64/Azul Systems, Inc. 11.0.15 (64-bit)/cpus=2,threads=1,free=427742008,total=536870912
  2> NOTE: All tests run in this JVM: [IndexManagementIndicesIT, MetadataRegressionIT, ActionRetryIT, ActionTimeoutIT, AllocationActionIT, CloseActionIT, DeleteActionIT]
  1> [2022-05-10T14:06:07,850][INFO ][o.o.i.i.a.DeleteActionIT ] [test basic] initializing REST clients against [http://[::1]:36167, http://127.0.0.1:37191]
  1> [2022-05-10T14:06:34,174][INFO ][o.o.i.i.a.DeleteActionIT ] [test basic] after test

We delete index, but coordinator listen to index delete event and clean metadata, I see this error log:

[2022-05-10T23:06:14,035][ERROR][o.o.i.i.ManagedIndexRunner] [integTest-0] There was VersionConflictEngineException trying to update the metadata for deleteactionit_index_1. Message: [WT3xpmYwTX6fyqWN8TOYnA#metadata]: version conflict, required seqNo [5], primary term [1]. but no document was found
org.opensearch.index.engine.VersionConflictEngineException: [WT3xpmYwTX6fyqWN8TOYnA#metadata]: version conflict, required seqNo [5], primary term [1]. but no document was found
[2022-05-10T23:06:14,036][ERROR][o.o.i.i.ManagedIndexRunner] [integTest-0] Failed to update ManagedIndexMetaData after executing the Step : attempt_delete

Basically for delete action, this race condition always exists.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
@bowenlan-amzn bowenlan-amzn merged commit d7469bd into opensearch-project:main May 12, 2022
@bowenlan-amzn bowenlan-amzn deleted the scroll branch May 12, 2022 19:04
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 12, 2022
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
(cherry picked from commit d7469bd)
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 12, 2022
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
(cherry picked from commit d7469bd)
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 12, 2022
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
(cherry picked from commit d7469bd)
bowenlan-amzn added a commit that referenced this pull request May 13, 2022
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
(cherry picked from commit d7469bd)

Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>
bowenlan-amzn added a commit that referenced this pull request May 13, 2022
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
(cherry picked from commit d7469bd)

Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>
bowenlan-amzn added a commit that referenced this pull request May 13, 2022
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
(cherry picked from commit d7469bd)

Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>
bowenlan-amzn added a commit to bowenlan-amzn/index-management that referenced this pull request May 16, 2022
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
bowenlan-amzn added a commit that referenced this pull request May 17, 2022
* Strengthen scroll search in Coordinator (#356)

* Removes rc1 qualifier (#353)

* Allows error in shrink test

* Adds workflow to create documentation issues (#358)

* Adds documentation issue workflow

* Makes the issue template more relevant

* Data model and Basic CRUD APIs

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
wuychn pushed a commit to ochprince/index-management that referenced this pull request Mar 16, 2023
* Strengthen scroll search in Coordinator

Normally user won't have >10k ISM jobs, so we don't want to use scroll
search every time.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
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