[Bugfix] Fix IllegalArgumentException thrown when creating a PIT

[peteralfonsi]

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.

[github-actions]

❌ Gradle check result for 6d35c7e: 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]

Flaky test: #14293

[github-actions]

❌ Gradle check result for ea30484: 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]

Flaky tests: #15829, #16015, #14311

[github-actions]

❌ Gradle check result for a41f9be: 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]

Flaky test: #16576

[github-actions]

❕ Gradle check result for e20a4de: 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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.19%. Comparing base (57a6605) to head (4831adb).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #16781      +/-   ##
============================================
- Coverage     72.24%   72.19%   -0.05%     
- Complexity    65334    65346      +12     
============================================
  Files          5318     5318              
  Lines        304069   304073       +4     
  Branches      43995    43995              
============================================
- Hits         219678   219529     -149     
- Misses        66389    66589     +200     
+ Partials      18002    17955      -47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andrross]

I count 4 method calls in this line. The problem with this try-catch is that any one of them in theory could throw an IllegalArgumentException, and you probably don't want to swallow it in all cases. Also, exception handling in the control flow is generally something to avoid.

I might recommend revisiting that API on SearchPhase to make it explicit that it may not be possible to get a SearchPhaseName. Like if it returned an Optional you could do:

phase.asSearchPhaseName().ifPresent(name -> {
    phaseStatsMap.get(name).current.inc();
});

[peteralfonsi]

Makes sense. I've just pushed a commit changing getSearchPhaseName to return Optional<SearchPhaseName>.

[github-actions]

❌ Gradle check result for 49fec57: 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]

Flaky test: #14296

[github-actions]

✅ Gradle check result for 4831adb: SUCCESS

[andrross]

This is a breaking change to a class annotated PublicApi so I think you'll want to make this change in way that keeps the existing signature.