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

[Search Pipelines] Add default_search_pipeline index setting #7470

Merged
merged 4 commits into from
May 16, 2023

Conversation

msfroh
Copy link
Collaborator

@msfroh msfroh commented May 8, 2023

Description

Once users have defined and tested a search pipeline, they may want to apply it by default to all queries hitting a given index.

Users should be able to bypass the default pipeline for the index by explicitly specifying search_pipeline=_none in the URL parameter of their search request.

Related Issues

Resolves #6719

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2023

Gradle Check (Jenkins) Run Completed with:

@msfroh msfroh changed the title [Search Pipelines] Add default_search_pipline index setting [Search Pipelines] Add default_search_pipeline index setting May 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2023

Codecov Report

Merging #7470 (0cb9894) into main (27dbcd5) will decrease coverage by 0.10%.
The diff coverage is 91.30%.

@@             Coverage Diff              @@
##               main    #7470      +/-   ##
============================================
- Coverage     70.54%   70.45%   -0.10%     
+ Complexity    59713    59677      -36     
============================================
  Files          4896     4896              
  Lines        286798   286815      +17     
  Branches      41331    41335       +4     
============================================
- Hits         202334   202063     -271     
- Misses        67761    68071     +310     
+ Partials      16703    16681      -22     
Impacted Files Coverage Δ
...pensearch/common/settings/IndexScopedSettings.java 100.00% <ø> (ø)
...nsearch/search/pipeline/SearchPipelineService.java 82.81% <85.71%> (-0.34%) ⬇️
.../main/java/org/opensearch/index/IndexSettings.java 84.98% <100.00%> (+0.35%) ⬆️

... and 501 files with indirect coverage changes

@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@reta
Copy link
Collaborator

reta commented May 12, 2023

@msfroh could you please rebase?

@kotwanikunal
Copy link
Member

Seeing a bunch of

org.opensearch.backwards.IndexingIT > testSeqNoCheckpoints FAILED
    org.opensearch.client.ResponseException: method [GET], host [http://[::1/]:37425], URI [test/_count?preference=_only_nodes%3Av2.8.0-3], status line [HTTP/1.1 500 Internal Server Error]
    {"error":{"root_cause":[{"type":"illegal_state_exception","reason":"unexpected byte [0x3f]"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"test","node":"6sjRAsVtRGa9N2kVkQUBlA","reason":{"type":"illegal_state_exception","reason":"unexpected byte [0x3f]"}}],"caused_by":{"type":"illegal_state_exception","reason":"unexpected byte [0x3f]","caused_by":{"type":"illegal_state_exception","reason":"unexpected byte [0x3f]"}}},"status":500}
        at __randomizedtesting.SeedInfo.seed([FDA6AED85DA67D18:CCCA44139739F844]:0)
        at app//org.opensearch.client.RestClient.convertResponse(RestClient.java:384)
        at app//org.opensearch.client.RestClient.performRequest(RestClient.java:354)
        at app//org.opensearch.client.RestClient.performRequest(RestClient.java:329)
        at app//org.opensearch.backwards.IndexingIT.assertCount(IndexingIT.java:358)
        at app//org.opensearch.backwards.IndexingIT.testSeqNoCheckpoints(IndexingIT.java:213)

Ref: https://build.ci.opensearch.org/job/gradle-check/15296/consoleFull

Can it be because of the version mismatch between 2.x and main since #7253 was merged and this isn't?

@reta
Copy link
Collaborator

reta commented May 12, 2023

Can it be because of the version mismatch between 2.x and main since #7253 was merged and this isn't?

Yes, the fix is bundled there #7470

@msfroh
Copy link
Collaborator Author

msfroh commented May 12, 2023

could you please rebase?

@reta Done

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Once users have defined and tested a search pipeline, they may want to
apply it by default to all queries hitting a given index.

Users should be able to bypass the default pipeline for the index by
explicitly specifying `search_pipeline=_none` in the URL parameter of
their search request.

Signed-off-by: Michael Froh <[email protected]>
Also change version check for ad hoc pipeline to 2.8

Signed-off-by: Michael Froh <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.indices.replication.SegmentReplicationIT.testScrollWithOngoingSegmentReplication

@msfroh
Copy link
Collaborator Author

msfroh commented May 16, 2023

Merge conflicts have been fixed. Rebased onto main recently.

Any remaining blockers to merging? @kotwanikunal @andrross @reta

Thanks!

@andrross andrross merged commit 5984735 into opensearch-project:main May 16, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-7470-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 59847356b1c82562498f38896adef66ea1702cd6
# Push it to GitHub
git push --set-upstream origin backport/backport-7470-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7470-to-2.x.

@andrross
Copy link
Member

@msfroh Can you pick up the backport since the automated cherry-pick failed?

msfroh added a commit to msfroh/OpenSearch that referenced this pull request May 16, 2023
…rch-project#7470)

* [Search Pipelines] Add default_search_pipeline index setting

Once users have defined and tested a search pipeline, they may want to
apply it by default to all queries hitting a given index.

Users should be able to bypass the default pipeline for the index by
explicitly specifying `search_pipeline=_none` in the URL parameter of
their search request.

Signed-off-by: Michael Froh <[email protected]>

* Rename default search pipeline setting and add getter/setter

Also change version check for ad hoc pipeline to 2.8

Signed-off-by: Michael Froh <[email protected]>

* Add tests for new IndexSettings methods

Signed-off-by: Michael Froh <[email protected]>

* Update test to reflect change to FeatureFlagSetter

Signed-off-by: Michael Froh <[email protected]>

---------

Signed-off-by: Michael Froh <[email protected]>
(cherry picked from commit 5984735)
@msfroh
Copy link
Collaborator Author

msfroh commented May 17, 2023

Backport PR: #7589

reta pushed a commit that referenced this pull request May 17, 2023
…tting (#7470) (#7589)

* [Search Pipelines] Add default_search_pipeline index setting (#7470)

* [Search Pipelines] Add default_search_pipeline index setting

Once users have defined and tested a search pipeline, they may want to
apply it by default to all queries hitting a given index.

Users should be able to bypass the default pipeline for the index by
explicitly specifying `search_pipeline=_none` in the URL parameter of
their search request.

Signed-off-by: Michael Froh <[email protected]>

* Rename default search pipeline setting and add getter/setter

Also change version check for ad hoc pipeline to 2.8

Signed-off-by: Michael Froh <[email protected]>

* Add tests for new IndexSettings methods

Signed-off-by: Michael Froh <[email protected]>

* Update test to reflect change to FeatureFlagSetter

Signed-off-by: Michael Froh <[email protected]>

---------

Signed-off-by: Michael Froh <[email protected]>
(cherry picked from commit 5984735)

* Only enable search pipelines for BWC tests after 2.7.0

Signed-off-by: Michael Froh <[email protected]>

* Fix version check for search pipelines REST test

I previously misunderstood the syntax for skipping tests by version.
Thanks @reta for catching this!

Signed-off-by: Michael Froh <[email protected]>

---------

Signed-off-by: Michael Froh <[email protected]>
stephen-crawford pushed a commit to stephen-crawford/OpenSearch that referenced this pull request May 31, 2023
…rch-project#7470)

* [Search Pipelines] Add default_search_pipeline index setting

Once users have defined and tested a search pipeline, they may want to
apply it by default to all queries hitting a given index.

Users should be able to bypass the default pipeline for the index by
explicitly specifying `search_pipeline=_none` in the URL parameter of
their search request.

Signed-off-by: Michael Froh <[email protected]>

* Rename default search pipeline setting and add getter/setter

Also change version check for ad hoc pipeline to 2.8

Signed-off-by: Michael Froh <[email protected]>

* Add tests for new IndexSettings methods

Signed-off-by: Michael Froh <[email protected]>

* Update test to reflect change to FeatureFlagSetter

Signed-off-by: Michael Froh <[email protected]>

---------

Signed-off-by: Michael Froh <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…rch-project#7470)

* [Search Pipelines] Add default_search_pipeline index setting

Once users have defined and tested a search pipeline, they may want to
apply it by default to all queries hitting a given index.

Users should be able to bypass the default pipeline for the index by
explicitly specifying `search_pipeline=_none` in the URL parameter of
their search request.

Signed-off-by: Michael Froh <[email protected]>

* Rename default search pipeline setting and add getter/setter

Also change version check for ad hoc pipeline to 2.8

Signed-off-by: Michael Froh <[email protected]>

* Add tests for new IndexSettings methods

Signed-off-by: Michael Froh <[email protected]>

* Update test to reflect change to FeatureFlagSetter

Signed-off-by: Michael Froh <[email protected]>

---------

Signed-off-by: Michael Froh <[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
Labels
backport 2.x Backport to 2.x branch
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Index default pipelines
6 participants