Support scripting for composite aggs in concurrent segment search

[jed326]

Description

Fix race condition related to SourceLookup when using _source scripts in concurrent segment search.

SearchLookup is created per-QueryShardContext so it is shared across threads. SearchLookup::getLeafSearchLookup is called as a part of getLeafCollector so it creates a new LeafSearchLookup for each thread. However, the SourceLookup passed into each LeafSearchLookup is the single instance created in SearchLookup which will be shared across threads but is not thread safe. To resolve this we initialize a new SourceLookup and pass it into each new LeafSearchLookup instance.

Ideally we would create a new SourceLookup only for each slice instead of for each leaf, however I don't see a great way to do that.

Related Issues

Resolves #12947
Relates #15007

Check List

• Functionality includes testing.
  - [ ] API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable. (TODO: Create docs issues)

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 fd19066: 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?

[github-actions]

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

[github-actions]

❌ Gradle check result for 27ee2e8: 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?

[rishabh6788]

Not sure if there is a better way to compare the perf numbers, but I checked against the runs on some other PRs (#14464 (comment), #14993 (comment)) and the numbers look very similar.

Ah found the baseline dashboard: https://github.com/opensearch-project/OpenSearch/blob/main/PERFORMANCE_BENCHMARKS.md#how-to-compare-my-results-against-baseline

#14826 is in progress and would add the baseline comparison results on the PR as well once done.

[rishabh6788]

Not sure if there is a better way to compare the perf numbers, but I checked against the runs on some other PRs (#14464 (comment), #14993 (comment)) and the numbers look very similar.
Ah found the baseline dashboard: https://github.com/opensearch-project/OpenSearch/blob/main/PERFORMANCE_BENCHMARKS.md#how-to-compare-my-results-against-baseline

#14826 is in progress and would add the baseline comparison results on the PR as well once done. Till then you can use the dashboard to compare the numbers.

[github-actions]

✅ Gradle check result for d32496a: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.78%. Comparing base (48634bd) to head (c99a6b1).
Report is 2 commits behind head on main.

Files	Patch %	Lines
.../bucket/composite/CompositeAggregationFactory.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #15072      +/-   ##
============================================
- Coverage     71.82%   71.78%   -0.05%     
+ Complexity    62778    62765      -13     
============================================
  Files          5169     5169              
  Lines        294646   294665      +19     
  Branches      42610    42616       +6     
============================================
- Hits         211644   211529     -115     
- Misses        65573    65721     +148     
+ Partials      17429    17415      -14     

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

[sohami]

@reta @sohami @mch2 mind taking another look now that the perf numbers are in and look fine?

Changes looks fine to me, I think we should create an issue in opensearch benchmark workload to add support for painless script as well.

[github-actions]

✅ Gradle check result for c99a6b1: SUCCESS

[opensearch-trigger-bot]

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-15072-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a785073e5e7925ef8e5605427cae943822100f4a
# Push it to GitHub
git push --set-upstream origin backport/backport-15072-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-15072-to-2.x.

[reta]

@jed326 mind please backport manually? thank you