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

Fix concurrent search NPE with track_total_hits, size=0, and terminate_after #10082

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

jed326
Copy link
Collaborator

@jed326 jed326 commented Sep 16, 2023

Description

Change EmptyTopDocsCollectorContext::createManager to not return a null manager.

Same query no longer returns NPE:

curl -X GET "localhost:9200/_search?size=0&track_total_hits=true&pretty&terminate_after=1000000" -H 'Content-Type: application/json' -d'
{
    "query": {                        
        "match_all": {}                         
    }      
}                  
'  
{
  "took" : 889,
  "timed_out" : false,
  "terminated_early" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 4,
      "relation" : "eq"
    },
    "max_score" : null,
    "hits" : [ ]
  }
}

Related Issues

Resolves #10054

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 github-actions bot added enhancement Enhancement or improvement to existing feature or request Search Search query, autocomplete ...etc labels Sep 16, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 16, 2023

Compatibility status:

Checks if related components are compatible with change 0e6d29e

Incompatible components

Incompatible components: [https://github.com/opensearch-project/k-nn.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.client.PitIT.testDeleteAllAndListAllPits

@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

Merging #10082 (0e6d29e) into main (9b5bf5f) will increase coverage by 0.02%.
Report is 2 commits behind head on main.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main   #10082      +/-   ##
============================================
+ Coverage     71.04%   71.06%   +0.02%     
- Complexity    58090    58094       +4     
============================================
  Files          4825     4825              
  Lines        274101   274102       +1     
  Branches      39945    39945              
============================================
+ Hits         194741   194797      +56     
+ Misses        63026    62974      -52     
+ Partials      16334    16331       -3     
Files Changed Coverage Δ
...ensearch/search/query/TopDocsCollectorContext.java 88.28% <0.00%> (+0.33%) ⬆️

... and 448 files with indirect coverage changes

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@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.remotestore.SegmentReplicationUsingRemoteStoreIT.testMultipleShards

Signed-off-by: Jay Deng <jayd0104@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@jed326
Copy link
Collaborator Author

jed326 commented Sep 21, 2023

@reta @sohami could you help review this PR? Thanks!

@sohami sohami added the backport 2.x Backport to 2.x branch label Sep 21, 2023
@sohami sohami merged commit c178d8e into opensearch-project:main Sep 21, 2023
67 of 68 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 21, 2023
Signed-off-by: Jay Deng <jayd0104@gmail.com>
(cherry picked from commit c178d8e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request Sep 22, 2023
Signed-off-by: Jay Deng <jayd0104@gmail.com>
(cherry picked from commit c178d8e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request Sep 22, 2023
(cherry picked from commit c178d8e)

Signed-off-by: Jay Deng <jayd0104@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/OpenSearch that referenced this pull request Sep 24, 2023
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…oject#10082)

Signed-off-by: Jay Deng <jayd0104@gmail.com>
Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
vikasvb90 pushed a commit to vikasvb90/OpenSearch that referenced this pull request Oct 10, 2023
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…oject#10082)

Signed-off-by: Jay Deng <jayd0104@gmail.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
@jed326 jed326 deleted the 10054 branch August 7, 2024 17:29
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 enhancement Enhancement or improvement to existing feature or request Search:Performance Search Search query, autocomplete ...etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Concurrent Segment Search] NPE with size and terminate_after parameters when using concurrent search
3 participants