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

[Bugfix] Fix IllegalArgumentException thrown when creating a PIT #16781

Merged
merged 18 commits into from
Feb 15, 2025

Conversation

peteralfonsi
Copy link
Contributor

@peteralfonsi peteralfonsi commented Dec 4, 2024

Description

Fixes java.lang.IllegalArgumentException: No enum constant org.opensearch.action.search.SearchPhaseName.CREATE_PIT which is thrown when creating a PIT.

The exception comes from SearchRequestStats. It doesn't track PIT stats as this is handled elsewhere. So, when it got search phases with the name "create_pit", it couldn't find a matching enum value and threw the error. Adds a try-catch block on search phase start/end/failure so that if no matching enum value is found, nothing happens.

Adds UTs. Also, manually tested that after making the change, the following commands no longer caused the exception:

curl -XPOST "http://localhost:9200/test-index/_doc/1?op_type=index" -H "Content-Type: application/json" -d '{ "field1": "value1", "field2": "value2", "field3": 123 }'

curl -XPOST "http://localhost:9200/test-index/_search/point_in_time?keep_alive=1h"

And confirmed the new catch-all doesn't show up in the output of curl -XGET "localhost:9200/_nodes/stats/indices/search".

Related Issues

Resolves #16750

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
@github-actions github-actions bot added bug Something isn't working Search Search query, autocomplete ...etc v2.19.0 Issues and PRs related to version 2.19.0 v3.0.0 Issues and PRs related to version 3.0.0 labels Dec 4, 2024
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
@peteralfonsi peteralfonsi changed the title fix create_pit enum bug [Bugfix] Fix IllegalArgumentException thrown when creating a PIT Dec 4, 2024
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>
Copy link
Contributor

❌ Gradle check result for 5824a3d: 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?

@peterzhuamazon
Copy link
Member

Hi @peteralfonsi @andrross are we able to move forward with this PR?

opensearch-project/sql#3319 seens to be blocked by this.

Thanks.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Copy link
Contributor

❌ Gradle check result for 2dd7b77: 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?

Copy link
Contributor

❌ Gradle check result for 2dd7b77: 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?

@peteralfonsi
Copy link
Contributor Author

Flaky tests: #14297. Seems like it's failing to build the 2.19 linux tar, rerunning to see if it resolves

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Copy link
Contributor

❌ Gradle check result for 4a3906b: 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?

@andrross
Copy link
Member

@peteralfonsi Can you rebase this PR with the latest from main?

peteralfonsi and others added 2 commits February 14, 2025 14:55
…java

Co-authored-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>
Copy link
Contributor

✅ Gradle check result for 521405a: SUCCESS

@andrross andrross merged commit 56825f6 into opensearch-project:main Feb 15, 2025
30 checks passed
@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:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-16781-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 56825f63ec19cd1b96651d0c5a3663108ee26c8e
# Push it to GitHub
git push --set-upstream origin backport/backport-16781-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

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

peteralfonsi added a commit to peteralfonsi/OpenSearch that referenced this pull request Feb 17, 2025
…nsearch-project#16781)

* fix create_pit enum bug

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* changelog

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Fix SearchResponse XContent

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed David's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed andrross's comment

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Addressed andrross's comment"

This reverts commit 49fec57.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Addressed David's comments"

This reverts commit a41f9be.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Fix SearchResponse XContent"

This reverts commit ea30484.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "fix create_pit enum bug"

This reverts commit 0c9110d.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Fix create_pit bug with catch-all search phase name

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* switch to new optional method

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Update server/src/main/java/org/opensearch/action/search/SearchPhase.java

Co-authored-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>

---------

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 56825f6)
peteralfonsi added a commit to peteralfonsi/OpenSearch that referenced this pull request Feb 17, 2025
…nsearch-project#16781)

* fix create_pit enum bug

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* changelog

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Fix SearchResponse XContent

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed David's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed andrross's comment

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Addressed andrross's comment"

This reverts commit 49fec57.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Addressed David's comments"

This reverts commit a41f9be.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Fix SearchResponse XContent"

This reverts commit ea30484.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "fix create_pit enum bug"

This reverts commit 0c9110d.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Fix create_pit bug with catch-all search phase name

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* switch to new optional method

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Update server/src/main/java/org/opensearch/action/search/SearchPhase.java

Co-authored-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>

---------

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 56825f6)
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
andrross added a commit that referenced this pull request Feb 18, 2025
…ting a PIT (#17380)

* [Bugfix] Fix IllegalArgumentException thrown when creating a PIT (#16781)

* fix create_pit enum bug

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* changelog

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Fix SearchResponse XContent

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed David's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Addressed andrross's comment

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Addressed andrross's comment"

This reverts commit 49fec57.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Addressed David's comments"

This reverts commit a41f9be.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "Fix SearchResponse XContent"

This reverts commit ea30484.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Revert "fix create_pit enum bug"

This reverts commit 0c9110d.

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Fix create_pit bug with catch-all search phase name

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* switch to new optional method

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* Update server/src/main/java/org/opensearch/action/search/SearchPhase.java

Co-authored-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>

---------

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 56825f6)
Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

---------

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>
Signed-off-by: Andrew Ross <andrross@amazon.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
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 backport-failed bug Something isn't working Search Search query, autocomplete ...etc v2.19.0 Issues and PRs related to version 2.19.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] No enum constant org.opensearch.action.search.SearchPhaseName.CREATE_PIT
4 participants