Change InternalSignificantTerms to only sum shard level counts in final reduce #8735

jed326 · 2023-07-17T22:15:38Z

Description

Implements solution 2 from #8703 (comment)

In short: supersetSize and supersetDf are shard level counts that are gathered via count queries. In concurrent segment search these will still be shard level counts rather than segment slice level counts, so we do not want to sum up these counts across segment slices, only at the coordinator level (across shards)

Related Issues

Resolves #8703

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 · 2023-07-17T22:24:17Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/20335/
CommitID: eceb1dc
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 · 2023-07-17T22:49:57Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/20334/
CommitID: a3a09e3

github-actions · 2023-07-17T22:51:26Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/20336/
CommitID: 4625f98
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?

codecov · 2023-07-17T22:53:25Z

Codecov Report

Merging #8735 (11e49db) into main (57d5e90) will increase coverage by 0.08%.
The diff coverage is 33.33%.

@@             Coverage Diff              @@
##               main    #8735      +/-   ##
============================================
+ Coverage     70.76%   70.84%   +0.08%     
- Complexity    57111    57144      +33     
============================================
  Files          4771     4771              
  Lines        270241   270251      +10     
  Branches      39500    39502       +2     
============================================
+ Hits         191237   191465     +228     
+ Misses        62846    62633     -213     
+ Partials      16158    16153       -5

Impacted Files	Coverage Δ
...va/org/opensearch/search/DefaultSearchContext.java	`78.29% <0.00%> (-0.62%)`	⬇️
...nsearch/search/internal/FilteredSearchContext.java	`8.92% <0.00%> (ø)`
.../org/opensearch/search/internal/SearchContext.java	`38.00% <ø> (ø)`
...gations/bucket/terms/InternalSignificantTerms.java	`86.84% <33.33%> (-3.16%)`	⬇️
...earch/search/aggregations/InternalAggregation.java	`77.02% <50.00%> (-1.55%)`	⬇️
...arch/aggregations/AggregationCollectorManager.java	`82.75% <100.00%> (ø)`

... and 463 files with indirect coverage changes

github-actions · 2023-07-18T01:30:50Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.search.SearchTimeoutIT.testSimpleTimeout
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

URL: https://build.ci.opensearch.org/job/gradle-check/20352/
CommitID: 7566b29
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions · 2023-07-18T03:59:34Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      3 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPressureServiceStats

URL: https://build.ci.opensearch.org/job/gradle-check/20364/
CommitID: f6c78b9
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions · 2023-07-18T04:38:01Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testCancellation
      1 org.opensearch.indices.replication.SegmentReplicationIT.testPitCreatedOnReplica
      1 org.opensearch.indices.replication.SegmentReplicationIT.classMethod

URL: https://build.ci.opensearch.org/job/gradle-check/20367/
CommitID: d3e172e
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions · 2023-07-18T18:49:36Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/20436/
CommitID: bb7730e
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 · 2023-07-18T19:44:31Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      2 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testScrollCreatedOnReplica
      2 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.classMethod
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testReplicaHasDiffFilesThanPrimary
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPressureServiceStats
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPressureServiceStats

URL: https://build.ci.opensearch.org/job/gradle-check/20443/
CommitID: 3e66a1e
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions · 2023-07-18T21:39:03Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.snapshots.SnapshotStatusApisIT.testStatusAPIStatsForBackToBackShallowSnapshot
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPitCreatedOnReplica

URL: https://build.ci.opensearch.org/job/gradle-check/20450/
CommitID: a4ab229
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

...Test/java/org/opensearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java

.../src/main/java/org/opensearch/search/aggregations/bucket/terms/InternalSignificantTerms.java

sohami

lgtm. Thanks!

jed326 · 2023-07-19T17:16:43Z

@reta @andrross could one of you take a look at this PR and merge it if it looks good? Thanks!

github-actions · 2023-07-19T17:17:08Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/20528/
CommitID: 0f4bbd5
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 · 2023-07-19T17:56:38Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testScrollCreatedOnReplica
      1 org.opensearch.indices.replication.SegmentReplicationIT.testScrollCreatedOnReplica

URL: https://build.ci.opensearch.org/job/gradle-check/20530/
CommitID: 1f0851c
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

CHANGELOG.md

…al reduce Signed-off-by: Jay Deng <[email protected]>

github-actions · 2023-07-19T19:19:22Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/20538/
CommitID: 11e49db

opensearch-trigger-bot · 2023-07-20T18:47:30Z

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:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-8735-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6cc8da561512890c437ef5c605ad99b575ff858b
# Push it to GitHub
git push --set-upstream origin backport/backport-8735-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

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

…al reduce (opensearch-project#8735) Signed-off-by: Jay Deng <[email protected]>

…al reduce (#8735) (#8806) Signed-off-by: Jay Deng <[email protected]>

…al reduce (opensearch-project#8735) Signed-off-by: Jay Deng <[email protected]>

…al reduce (opensearch-project#8735) Signed-off-by: Jay Deng <[email protected]> Signed-off-by: Kaushal Kumar <[email protected]>

…al reduce (opensearch-project#8735) Signed-off-by: Jay Deng <[email protected]> Signed-off-by: Ivan Brusic <[email protected]>

…al reduce (opensearch-project#8735) Signed-off-by: Jay Deng <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>

jed326 force-pushed the sigterms-superset-fix branch 2 times, most recently from eceb1dc to 4625f98 Compare July 17, 2023 22:19

jed326 force-pushed the sigterms-superset-fix branch from 4625f98 to 7566b29 Compare July 18, 2023 00:52

jed326 force-pushed the sigterms-superset-fix branch 2 times, most recently from f6c78b9 to d3e172e Compare July 18, 2023 03:53

jed326 marked this pull request as ready for review July 18, 2023 04:40

jed326 requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli and shwetathareja as code owners July 18, 2023 04:40

jed326 force-pushed the sigterms-superset-fix branch from d3e172e to bb7730e Compare July 18, 2023 18:26

jed326 force-pushed the sigterms-superset-fix branch from bb7730e to 3e66a1e Compare July 18, 2023 18:52

jed326 force-pushed the sigterms-superset-fix branch from 3e66a1e to a4ab229 Compare July 18, 2023 21:01

sohami reviewed Jul 19, 2023

View reviewed changes

jed326 force-pushed the sigterms-superset-fix branch from a4ab229 to 0f4bbd5 Compare July 19, 2023 17:09

sohami approved these changes Jul 19, 2023

View reviewed changes

jed326 force-pushed the sigterms-superset-fix branch from 0f4bbd5 to 1f0851c Compare July 19, 2023 17:15

andrross approved these changes Jul 19, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Change InternalSignificantTerms to only sum shard level counts in fin…

11e49db

…al reduce Signed-off-by: Jay Deng <[email protected]>

jed326 force-pushed the sigterms-superset-fix branch from 1f0851c to 11e49db Compare July 19, 2023 18:48

andrross added backport 2.x Backport to 2.x branch v2.10.0 labels Jul 19, 2023

jed326 mentioned this pull request Jul 20, 2023

[Concurrent Segment Search] Optimize Significant Terms Agg to not perform count query for each segment slice #8789

Open

andrross merged commit 6cc8da5 into opensearch-project:main Jul 20, 2023

jed326 added a commit to jed326/OpenSearch that referenced this pull request Jul 20, 2023

Change InternalSignificantTerms to only sum shard level counts in fin…

86d577e

…al reduce (opensearch-project#8735) Signed-off-by: Jay Deng <[email protected]>

jed326 mentioned this pull request Jul 20, 2023

[Backport 2.x] Change InternalSignificantTerms to only sum shard level counts in final reduce (#8735) #8806

Merged

6 tasks

andrross pushed a commit that referenced this pull request Jul 21, 2023

Change InternalSignificantTerms to only sum shard level counts in fin…

9524683

…al reduce (#8735) (#8806) Signed-off-by: Jay Deng <[email protected]>

baba-devv pushed a commit to baba-devv/OpenSearch that referenced this pull request Jul 29, 2023

Change InternalSignificantTerms to only sum shard level counts in fin…

b23dcca

…al reduce (opensearch-project#8735) Signed-off-by: Jay Deng <[email protected]>

jed326 deleted the sigterms-superset-fix branch September 13, 2023 19:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change InternalSignificantTerms to only sum shard level counts in final reduce #8735

Change InternalSignificantTerms to only sum shard level counts in final reduce #8735

jed326 commented Jul 17, 2023

github-actions bot commented Jul 17, 2023

github-actions bot commented Jul 17, 2023

github-actions bot commented Jul 17, 2023

codecov bot commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 18, 2023

github-actions bot commented Jul 18, 2023

github-actions bot commented Jul 18, 2023

github-actions bot commented Jul 18, 2023

github-actions bot commented Jul 18, 2023

github-actions bot commented Jul 18, 2023

sohami left a comment

jed326 commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

opensearch-trigger-bot bot commented Jul 20, 2023

Change InternalSignificantTerms to only sum shard level counts in final reduce #8735

Change InternalSignificantTerms to only sum shard level counts in final reduce #8735

Conversation

jed326 commented Jul 17, 2023

Description

Related Issues

Check List

github-actions bot commented Jul 17, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 17, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 17, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Jul 17, 2023 • edited Loading

Codecov Report

github-actions bot commented Jul 18, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 18, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 18, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 18, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 18, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 18, 2023

Gradle Check (Jenkins) Run Completed with:

sohami left a comment

Choose a reason for hiding this comment

jed326 commented Jul 19, 2023

github-actions bot commented Jul 19, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 19, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 19, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Jul 20, 2023

codecov bot commented Jul 17, 2023 •

edited

Loading