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

[Backport 2.x] [Search Pipelines] Add stats for search pipelines #8376

Merged

Conversation

mingshl
Copy link
Contributor

@mingshl mingshl commented Jun 30, 2023

Description

manual back-porting #8053 and resolve conflict

Related Issues

Resolves #6723

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
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@mingshl mingshl force-pushed the backport/backport-8053-to-2.x branch from a74d58d to 209a00a Compare June 30, 2023 22:11
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@mingshl
Copy link
Contributor Author

mingshl commented Jun 30, 2023

Need help in rerunning gradle check

@msfroh
Copy link
Collaborator

msfroh commented Jul 2, 2023

It looks like the Gradle check is failing because of the 1.3.11 release -- the buildBwcLinuxTar target is now building 1.3.12?

@msfroh
Copy link
Collaborator

msfroh commented Jul 2, 2023

Note that to avoid breaking backward compatibility tests on main, we will need to update the version checks on main to also check for versions on or after 2.9.0. Please create the PR for that and link it from this PR. They basically need to go in together (until we come up with a better solution).

See #8070 for discussion.

@mingshl mingshl added the v2.9.0 'Issues and PRs related to version v2.9.0' label Jul 3, 2023
@mingshl
Copy link
Contributor Author

mingshl commented Jul 3, 2023

Note that to avoid breaking backward compatibility tests on main, we will need to update the version checks on main to also check for versions on or after 2.9.0. Please create the PR for that and link it from this PR. They basically need to go in together (until we come up with a better solution).

See #8070 for discussion.

created #8416 to update the version checks on main

msfroh and others added 2 commits July 5, 2023 09:22
…#8053)

* [Search Pipelines] Add stats for search pipelines

This adds statistics on executions and time spent on search pipeline
operations, similar to the stats that are available for ingest
pipelines.

Signed-off-by: Michael Froh <[email protected]>

* Compare parsed JSON structure, not exact JSON string

As @lukas-vlcek pointed out, asserting equality with an exact JSON
string is sensitive to formatting, which makes the test brittle.

Instead, we can parse the expected JSON and compare as Maps.

Signed-off-by: Michael Froh <[email protected]>

* Refactor to common stats/metrics classes

Search pipelines and ingest pipelines had identical functionality for
tracking metrics around operations and converting those to immutable
"stats" objects.

That approach isn't even really specific to pipelines, but can be used
to track metrics on any repeated operation, so I moved that common
logic to the common.metrics package.

Signed-off-by: Michael Froh <[email protected]>

* Split pipeline metrics tracking into its own class

Thanks @saratvemulapalli for the suggestion! This lets the Pipeline
class focus on transforming requests / responses, while the subclass
focuses on tracking and managing metrics.

Signed-off-by: Michael Froh <[email protected]>

---------

Signed-off-by: Michael Froh <[email protected]>
(cherry picked from commit 46c9a21)
Signed-off-by: Mingshi Liu <[email protected]>
@mingshl mingshl force-pushed the backport/backport-8053-to-2.x branch from 209a00a to 0d648c8 Compare July 5, 2023 16:23
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.index.shard.RemoteStoreRefreshListenerTests.classMethod
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testDropPrimaryDuringReplication
      1 org.opensearch.index.shard.RemoteStoreRefreshListenerTests.testRefreshSuccessAfterFailureInFirstAttemptAfterSnapshotAndMetadataUpload

@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Merging #8376 (7a40bdc) into 2.x (9b9718f) will increase coverage by 0.04%.
The diff coverage is 87.93%.

@@             Coverage Diff              @@
##                2.x    #8376      +/-   ##
============================================
+ Coverage     70.47%   70.52%   +0.04%     
- Complexity    56953    57039      +86     
============================================
  Files          4737     4740       +3     
  Lines        270103   270416     +313     
  Branches      39905    39940      +35     
============================================
+ Hits         190345   190699     +354     
+ Misses        63404    63348      -56     
- Partials      16354    16369      +15     
Impacted Files Coverage Δ
...min/cluster/stats/TransportClusterStatsAction.java 70.83% <ø> (ø)
...src/main/java/org/opensearch/node/NodeService.java 73.49% <0.00%> (-0.90%) ⬇️
...rch/action/admin/cluster/node/stats/NodeStats.java 47.66% <33.33%> (-0.71%) ⬇️
.../action/admin/cluster/stats/ClusterStatsNodes.java 55.00% <55.55%> (ø)
.../org/opensearch/common/metrics/OperationStats.java 78.78% <78.78%> (ø)
.../java/org/opensearch/search/pipeline/Pipeline.java 87.83% <85.96%> (-1.82%) ⬇️
...pensearch/search/pipeline/SearchPipelineStats.java 89.44% <89.44%> (ø)
...pensearch/search/pipeline/PipelineWithMetrics.java 90.90% <90.90%> (ø)
...main/java/org/opensearch/ingest/IngestService.java 83.37% <93.75%> (ø)
...on/admin/cluster/node/stats/NodesStatsRequest.java 69.30% <100.00%> (+0.30%) ⬆️
... and 8 more

... and 468 files with indirect coverage changes

Signed-off-by: Mingshi Liu <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testWriteRead
      1 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testSnapshotWithLargeSegmentFiles
      1 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testMultipleSnapshotAndRollback
      1 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testList
      1 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testIndicesDeletedFromRepository
      1 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.testDeleteBlobs
      1 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.classMethod
      1 org.opensearch.cluster.ClusterHealthIT.testHealthOnClusterManagerFailover

@tlfeng tlfeng merged commit d150117 into opensearch-project:2.x Jul 5, 2023
@mingshl
Copy link
Contributor Author

mingshl commented Jul 5, 2023

Thank you @tlfeng !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority-High v2.9.0 'Issues and PRs related to version v2.9.0'
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants