Fix SimpleNestedIT.testExplain flaky test #11681
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neetikasinghal
requested review from
abbashus,
adnapibar,
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
dbwiddis,
dreamer-89,
gbbafna,
kartg,
kotwanikunal,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
ryanbogan,
sachinpkale,
saratvemulapalli,
setiah,
shwetathareja,
sohami,
tlfeng and
VachaShah
as code owners
github-actions
bot
added
bug
jed326
approved these changes
Jan 2, 2024
server/src/internalClusterTest/java/org/opensearch/search/nested/SimpleNestedIT.java
Outdated
Show resolved
Hide resolved
server/src/internalClusterTest/java/org/opensearch/search/nested/SimpleNestedIT.java
Outdated
Show resolved
Hide resolved
neetikasinghal
force-pushed
the
flaky-nested
branch
from
065c22d to
69378f1
Compare
sohami
reviewed
Jan 3, 2024
server/src/internalClusterTest/java/org/opensearch/search/nested/SimpleNestedIT.java
Outdated
Show resolved
Hide resolved
server/src/internalClusterTest/java/org/opensearch/search/nested/SimpleNestedIT.java
Outdated
Show resolved
Hide resolved
neetikasinghal
force-pushed
the
flaky-nested
branch
from
69378f1 to
9045235
Compare
neetikasinghal
commented
Jan 4, 2024
server/src/internalClusterTest/java/org/opensearch/search/nested/SimpleNestedIT.java
Outdated
Show resolved
Hide resolved
|
❌ Gradle check result for 9045235: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
neetikasinghal
force-pushed
the
flaky-nested
branch
from
9045235 to
7c3df12
Compare
|
❌ Gradle check result for 7c3df12: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
reta
reviewed
Jan 8, 2024
server/src/internalClusterTest/java/org/opensearch/search/nested/SimpleNestedIT.java
Outdated
Show resolved
Hide resolved
neetikasinghal
force-pushed
the
flaky-nested
branch
from
7c3df12 to
7fb9849
Compare
|
❌ Gradle check result for 7fb9849: Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
kkmr
approved these changes
Jan 10, 2024
sohami
reviewed
Jan 10, 2024
server/src/internalClusterTest/java/org/opensearch/search/nested/SimpleNestedIT.java
Show resolved
Hide resolved
|
Thanks @neetikasinghal , LGTM from my side (but @sohami has a good point) |
neetikasinghal
force-pushed
the
flaky-nested
branch
from
7fb9849 to
4172f1f
Compare
|
❌ Gradle check result for 4172f1f: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Neetika Singhal <[email protected]>
neetikasinghal
force-pushed
the
flaky-nested
branch
from
4172f1f to
5cb6b98
Compare
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Jan 11, 2024
Signed-off-by: Neetika Singhal <[email protected]> (cherry picked from commit bbe790b) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kotwanikunal
pushed a commit
that referenced
this pull request
Jan 11, 2024
(cherry picked from commit bbe790b) Signed-off-by: Neetika Singhal <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
rayshrey
pushed a commit
to rayshrey/OpenSearch
that referenced
this pull request
Mar 18, 2024
Signed-off-by: Neetika Singhal <[email protected]>
8 tasks
shiv0408
pushed a commit
to Gaurav614/OpenSearch
that referenced
this pull request
Apr 25, 2024
Signed-off-by: Neetika Singhal <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The explain itself is coming from Lucene: https://github.com/apache/lucene/blob/a6f70ad2bb0b682eb65feb522358ee6d16cad766/lucene/join/src/java/org/apache/lucene/search/join/ToParentBlockJoinQuery.java#L432-L440
Here's some info on the docIds in lucene:
A DocId in lucene is not actually unique to the Index but is unique to a Segment. Lucene does this mainly to optimize writing and compression. Since it is only unique to a Segment, how can a Doc be uniquely identified at the Index level? The solution is simple. The segments are ordered. To take a simple example, an Index has two segments and each segment has 100 docs respectively. The DocId's in the Segment are 0-100 but when they are converted to the Index level, the range of the DocId's in the second Segment is converted to 100-200.
DocId's are unique within a Segment, numbered progressively from zero. But this does not mean that the DocId's are continuous. When a Doc is deleted, there is a gap.
The DocId corresponding to a document can change, usually when Segments are merged.
The test fails on the validation of range of the docIds in the assertion, the range changes as with indexRandomForConcurrentSearch function there are several bogus documents ingested and deleted which could trigger background merges and cause the range of the docIds matched with the search query to change.
One of the solutions is to call refresh after indexing the actual document which would create a new segment for the actual document and prevent it to be merged with the bogus documents, this is done as part of this PR.
Related Issues
Resolves #11413
Check List
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.