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 slice collectors to leaves association with post filter #11134

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

ticheng-aws
Copy link
Contributor

@ticheng-aws ticheng-aws commented Nov 8, 2023

Description

In the post filter query, a profile node is created during the creation of the filtered collector context, which occurred before the concurrent path in ContextIndexSearcher::search(). However, there is no association call for the filtered collector to leaves. Consequently, this led to an unexpected context size when generating the profile results.

This PR adds the association for the collector to leaves before building the scorer on the post-filter profile node. Following this modification, we can identify the missing association call for sliceCollectorsToLeave@739870973, and we can also confirm that the leaf counts in the contexts map match those in sliceCollectorsToLeaves when we invoke toBreakdownMap() to build the result.

------------ post filter query with 2 slices and 1 leaf/slice -------------
-- query rewrite --
-- query rewrite --
[T#21] ConcurrentQueryProfileBreakdown.sliceCollectorsToLeaves@2013178468
[T#21] searcher.search(query, collectorManager)
-- query rewrite --
[T#21] ConcurrentQueryProfileBreakdown.sliceCollectorsToLeaves@739870973
[T#21] ConcurrentQueryProfileBreakdown.sliceCollectorsToLeaves@208094559
[T#26] == search ===
[T#27] == search ===
[T#26] = searchLeaf() =
[T#27] = searchLeaf() =
[T#27] ConcurrentQueryProfileBreakdown.associateCollectorToLeaves() | sliceCollectorsToLeaves@739870973
[T#26] ConcurrentQueryProfileBreakdown.associateCollectorToLeaves() | sliceCollectorsToLeaves@739870973
[T#27] FilteredCollector.getLeafCollector() called
[T#26] FilteredCollector.getLeafCollector() called
[T#27] ConcurrentQueryProfileBreakdown.associateCollectorToLeaves() | sliceCollectorsToLeaves@2013178468
[T#26] ConcurrentQueryProfileBreakdown.associateCollectorToLeaves() | sliceCollectorsToLeaves@2013178468
[T#27] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_1(9.8.0):C76 | sliceCollectorsToLeaves@2013178468
[T#26] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_0(9.8.0):C524 | sliceCollectorsToLeaves@2013178468
[T#27] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_1(9.8.0):C76 | sliceCollectorsToLeaves@2013178468
[T#26] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_0(9.8.0):C524 | sliceCollectorsToLeaves@2013178468
[T#27] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_1(9.8.0):C76 | sliceCollectorsToLeaves@739870973
[T#26] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_0(9.8.0):C524 | sliceCollectorsToLeaves@739870973
[T#27] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_1(9.8.0):C76 | sliceCollectorsToLeaves@208094559
[T#26] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_0(9.8.0):C524 | sliceCollectorsToLeaves@208094559
[T#27] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_1(9.8.0):C76 | sliceCollectorsToLeaves@208094559
[T#26] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_0(9.8.0):C524 | sliceCollectorsToLeaves@208094559
[T#27] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_1(9.8.0):C76 | sliceCollectorsToLeaves@739870973
[T#26] ConcurrentQueryProfileBreakdown.context() context=LeafReaderContext(FilterLeafReader(_0(9.8.0):C524 | sliceCollectorsToLeaves@739870973
[T#21] ConcurrentQueryProfileBreakdown.associateCollectorsToLeaves() | sliceCollectorsToLeaves@208094559
[T#21] toBreakdownMap() contexts=2 | sliceCollectorsToLeaves@2013178468=2
...
[T#21] toBreakdownMap() contexts=2 | sliceCollectorsToLeaves@208094559=2
...
[T#21] toBreakdownMap() contexts=2 | sliceCollectorsToLeaves@739870973=2
...

Related Issues

Resolves #10767

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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 the bug Something isn't working label Nov 8, 2023
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

github-actions bot commented Nov 8, 2023

Compatibility status:

Checks if related components are compatible with change a19ce33

Incompatible components

Incompatible components: [https://github.com/opensearch-project/performance-analyzer.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/k-nn.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/anomaly-detection.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

ticheng-aws added a commit to ticheng-aws/OpenSearch that referenced this pull request Nov 8, 2023
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Gradle Check (Jenkins) Run Completed with:

ticheng-aws added a commit to ticheng-aws/OpenSearch that referenced this pull request Nov 9, 2023
Copy link
Contributor

github-actions bot commented Nov 9, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #11134 (a19ce33) into main (61a598b) will increase coverage by 0.02%.
The diff coverage is 50.00%.

@@             Coverage Diff              @@
##               main   #11134      +/-   ##
============================================
+ Coverage     71.15%   71.18%   +0.02%     
- Complexity    58798    58811      +13     
============================================
  Files          4883     4883              
  Lines        277141   277146       +5     
  Branches      40280    40281       +1     
============================================
+ Hits         197200   197283      +83     
+ Misses        63452    63376      -76     
+ Partials      16489    16487       -2     
Files Coverage Δ
...search/common/lucene/search/FilteredCollector.java 100.00% <100.00%> (ø)
...profile/query/ConcurrentQueryProfileBreakdown.java 93.56% <0.00%> (-1.08%) ⬇️

... and 472 files with indirect coverage changes

ticheng-aws added a commit to ticheng-aws/OpenSearch that referenced this pull request Nov 9, 2023
Copy link
Contributor

github-actions bot commented Nov 9, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

github-actions bot commented Nov 9, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testShardRoutingWithNetworkDisruption_FailOpenEnabled

@ticheng-aws
Copy link
Contributor Author

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testShardRoutingWithNetworkDisruption_FailOpenEnabled

This is a known flaky test #10673

@ticheng-aws
Copy link
Contributor Author

Hi @sohami and @reta, please help to review. Thank you in advance.

@ticheng-aws ticheng-aws changed the title Fix slice collectors to leaves association with profile enabled Fix slice collectors to leaves association with post filter Nov 9, 2023
@github-actions github-actions bot added Search Search query, autocomplete ...etc v2.12.0 Issues and PRs related to version 2.12.0 v3.0.0 Issues and PRs related to version 3.0.0 labels Nov 9, 2023
…rrentQueryProfileBreakdown.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Signed-off-by: Ticheng Lin <ticheng@amazon.com>
Copy link
Contributor

github-actions bot commented Nov 9, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

github-actions bot commented Nov 9, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testStrictWeightedRoutingWithCustomString_FailOpenEnabled
      1 org.opensearch.remotestore.RemoteStoreStatsIT.testNonZeroPrimaryStatsOnNewlyCreatedIndexWithZeroDocs
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

@ticheng-aws
Copy link
Contributor Author

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testStrictWeightedRoutingWithCustomString_FailOpenEnabled
      1 org.opensearch.remotestore.RemoteStoreStatsIT.testNonZeroPrimaryStatsOnNewlyCreatedIndexWithZeroDocs
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

#3603
#8030
#11149

@reta reta merged commit 3eda422 into opensearch-project:main Nov 9, 2023
31 of 32 checks passed
@reta reta added the backport 2.x Backport to 2.x branch label Nov 9, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 9, 2023
* Fix slice collectors to leaves association with profile enabled (#11134)

Signed-off-by: Ticheng Lin <ticheng@amazon.com>

* Update server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Signed-off-by: Ticheng Lin <ticheng@amazon.com>

---------

Signed-off-by: Ticheng Lin <ticheng@amazon.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
(cherry picked from commit 3eda422)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta added a commit that referenced this pull request Nov 10, 2023
…11150)

* Fix slice collectors to leaves association with profile enabled (#11134)



* Update server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java





---------




(cherry picked from commit 3eda422)

Signed-off-by: Ticheng Lin <ticheng@amazon.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.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>
Co-authored-by: Andriy Redko <drreta@gmail.com>
fahadshamiinsta pushed a commit to fahadshamiinsta/OpenSearch270 that referenced this pull request Dec 4, 2023
…ch-project#11134)

* Fix slice collectors to leaves association with profile enabled (opensearch-project#11134)

Signed-off-by: Ticheng Lin <ticheng@amazon.com>

* Update server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Signed-off-by: Ticheng Lin <ticheng@amazon.com>

---------

Signed-off-by: Ticheng Lin <ticheng@amazon.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
…ch-project#11134)

* Fix slice collectors to leaves association with profile enabled (opensearch-project#11134)

Signed-off-by: Ticheng Lin <ticheng@amazon.com>

* Update server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Signed-off-by: Ticheng Lin <ticheng@amazon.com>

---------

Signed-off-by: Ticheng Lin <ticheng@amazon.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…ch-project#11134)

* Fix slice collectors to leaves association with profile enabled (opensearch-project#11134)

Signed-off-by: Ticheng Lin <ticheng@amazon.com>

* Update server/src/main/java/org/opensearch/search/profile/query/ConcurrentQueryProfileBreakdown.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Signed-off-by: Ticheng Lin <ticheng@amazon.com>

---------

Signed-off-by: Ticheng Lin <ticheng@amazon.com>
Signed-off-by: Ticheng Lin <51488860+ticheng-aws@users.noreply.github.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shivansh Arora <hishiv@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 bug Something isn't working Search Search query, autocomplete ...etc skip-changelog v2.12.0 Issues and PRs related to version 2.12.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.

[Concurrent Segment Search][BUG] Incorrect slice collectors to leaves association with profile enabled
2 participants