Skip to content

Randomly disable sequence numbers in SimpleNestedIT#144093

Closed
romseygeek wants to merge 2 commits intoelastic:mainfrom
romseygeek:seqno/nested-tests
Closed

Randomly disable sequence numbers in SimpleNestedIT#144093
romseygeek wants to merge 2 commits intoelastic:mainfrom
romseygeek:seqno/nested-tests

Conversation

@romseygeek
Copy link
Copy Markdown
Contributor

Nested queries use entries in _primary_term docvalues fields to
identify parent documents. This should not be affected by enabling
or disabling sequence numbers, but to double check we now
randomly change the sequence number setting in SimpleNestedIT.

@romseygeek romseygeek requested review from fcofdez, kkrik-es and tlrx March 12, 2026 10:52
@romseygeek romseygeek self-assigned this Mar 12, 2026
@romseygeek romseygeek added >non-issue :StorageEngine/Mapping The storage related side of mappings v9.4.0 labels Mar 12, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@kkrik-es
Copy link
Copy Markdown
Member

Worth checking with test-release.

@romseygeek romseygeek added the test-release Trigger CI checks against release build label Mar 12, 2026
Copy link
Copy Markdown
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

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

LGTM

Nested queries use entries in _primary_term docvalues fields to
identify parent documents.  This should not be affected by enabling
or disabling sequence numbers, but to double check we now randomly
change the sequence number setting in SimpleNestedIT.
We can't assumeTrue in method called during test setup.
@romseygeek
Copy link
Copy Markdown
Contributor Author

Worth checking with test-release.

This actually found a bug in #144046 - have added a commit here which will fix it.

@fcofdez
Copy link
Copy Markdown
Contributor

fcofdez commented Mar 12, 2026

Maybe one thing that we should do in this test suite is to trigger merges too? otherwise we won't be trimming seq nos @romseygeek

@romseygeek
Copy link
Copy Markdown
Contributor Author

Maybe one thing that we should do in this test suite is to trigger merges too? otherwise we won't be trimming seq nos @romseygeek

Oh, that's a very good point. I'll add some force merges to make sure pruning actually happens here.

@romseygeek
Copy link
Copy Markdown
Contributor Author

I'm going to close this one out and open a new PR with explicit tests for disabled sequence numbers.

@romseygeek romseygeek closed this Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :StorageEngine/Mapping The storage related side of mappings Team:StorageEngine test-release Trigger CI checks against release build v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants