-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add stream search feature flag and auto fallback logic #19373
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 stream search feature flag and auto fallback logic #19373
Conversation
|
❌ Gradle check result for 90ccce3: 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? |
90ccce3 to
0bd4c77
Compare
|
❌ Gradle check result for bb3e8e2: 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? |
|
❌ Gradle check result for bb3e8e2: 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? |
bb3e8e2 to
3cb08c3
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19373 +/- ##
============================================
+ Coverage 72.95% 72.98% +0.03%
- Complexity 70115 70123 +8
============================================
Files 5687 5688 +1
Lines 321703 321804 +101
Branches 46534 46550 +16
============================================
+ Hits 234702 234883 +181
+ Misses 68074 67917 -157
- Partials 18927 19004 +77 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java
Outdated
Show resolved
Hide resolved
|
❌ Gradle check result for 1fc5227: null 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? |
server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/rest/action/search/RestSearchAction.java
Outdated
Show resolved
Hide resolved
|
@bowenlan-amzn looks good, couple of minor comments. |
1fc5227 to
2fc4ba3
Compare
|
❌ Gradle check result for 4ece007: 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: bowenlan-amzn <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
4ece007 to
8d53534
Compare
|
❌ Gradle check result for 8d53534: 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? |
|
❕ Gradle check result for 8d53534: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Signed-off-by: Rishabh Maurya <[email protected]>
|
❕ Gradle check result for a74ef16: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
51a091b
into
opensearch-project:main
…oject#19373) * Use feature flag instead of query param Signed-off-by: bowenlan-amzn <[email protected]> * add tests Signed-off-by: bowenlan-amzn <[email protected]> * add changelog Signed-off-by: bowenlan-amzn <[email protected]> * Disable stream search for normal search Signed-off-by: bowenlan-amzn <[email protected]> * Address comment Signed-off-by: bowenlan-amzn <[email protected]> --------- Signed-off-by: bowenlan-amzn <[email protected]> Signed-off-by: Rishabh Maurya <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]>
|
@bowenlan-amzn on thinking more about this feature flag, it makes more sense to me to have it as a dynamic cluster setting with default as off. Let me know what do you think? |
…oject#19373) * Use feature flag instead of query param Signed-off-by: bowenlan-amzn <[email protected]> * add tests Signed-off-by: bowenlan-amzn <[email protected]> * add changelog Signed-off-by: bowenlan-amzn <[email protected]> * Disable stream search for normal search Signed-off-by: bowenlan-amzn <[email protected]> * Address comment Signed-off-by: bowenlan-amzn <[email protected]> --------- Signed-off-by: bowenlan-amzn <[email protected]> Signed-off-by: Rishabh Maurya <[email protected]> Co-authored-by: Rishabh Maurya <[email protected]>
Description
This PR changes the way to enable stream search feature.
Previously it's through query param, now change to a feature flag.
If enable stream transport and search feature flag at the same time, user can try out the stream search feature. Example gradle run command below.
./gradlew run -PinstalledPlugins="['arrow-flight-rpc']" \ -Dtests.opensearch.opensearch.experimental.feature.transport.stream.enabled=true \ -Dtests.opensearch.opensearch.experimental.feature.stream.search.enabled=trueRelated Issues
Resolves #[Issue number to be closed when this PR is merged]
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.