-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add experimental features flag option for tests #454
Conversation
Changes AnalysisCommit SHA: 84f40ca API ChangesSummaryNO CHANGES ReportThe full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/10204919379/artifacts/1766492962 API Coverage
|
bee495a
to
13ef94b
Compare
13ef94b
to
91a2f54
Compare
91a2f54
to
eaf77fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code passes generic OPENSEARCH_JAVA_OPTS
but claims this is OPENSEARCH_EXPERIMENTAL_FEATURES
which it's not, it can be any opts. Get rid of mentions of "experimental features".
An alternate and likely a nicer implementation could be to actually pass a list of experimental features.
In test-spec.yml.
- version: 2.16.0
features:
- tiered_remote_index: true
To convert this into OPENSEARCH_JAVA_OPTS
you could use join.
1b75a98
to
6a70c80
Compare
Spec Test Coverage Analysis
|
6a70c80
to
ec085d3
Compare
I tried the following:
However, its coming as So, I am going to go with opts for now unless u have another suggestion. |
31590aa
to
76464c6
Compare
|
I updated the 2.16 ref to the latest in #459 and tests didn't fail the way they did here, so I looked at the |
76464c6
to
310b20e
Compare
Looks like its coming from here: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/node/Node.java#L385 |
Are you looking into it? Open an issue at least? |
yeah checking with @andrross to understand his thought process behind this change - doesnt look super right to me as If the tiered remote index feature flag is on and there is no search node it the cluster it would set the NODE_SEARCH_CACHE_SIZE_SETTING to 80%. |
For this PR, remove In your other PR where you actually need |
…ests Signed-off-by: Neetika Singhal <[email protected]>
310b20e
to
84f40ca
Compare
The intermittent failure is an OOM in the sql plugin, I changed the threshold in another PR. |
Description
As of now, there is no way in the tests to turn on the experimental feature flags for the tests, this pr aims to address it.
Issues Resolved
Related prs: #368
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.