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 local segments stats update in RemoteStoreRefreshListener #8758

Merged
merged 1 commit into from
Aug 6, 2023

Conversation

ashking94
Copy link
Member

@ashking94 ashking94 commented Jul 18, 2023

Description

Today, if a refresh occurs for an IndexShard, the afterRefresh method of ReferenceManager.RefreshListener (Lucene class) objects are invoked. RemoteStoreRefreshListener, currently, has the responsibility of updating the segment tracker with the upto date information of segment files which are part of the most recent refresh. It also has the responsibility of uploading the segments to the remote store after each refresh and then updating the segment tracker with the relevant information about the upload. This information is exposed via remote store stats api and is also used for rejection incoming write requests when there is a lag between the local store and the remote store (remote segment backpressure). While updating the tracker with local state happens at the beginning of the performAfterRefresh, there can be cases where the retry could have been blocking the performAfterRefresh (& hence the local state update in segment tracker). This would lead to incorrect information being saved in tracker due to the tight coupling.

With this PR, we aim to introduce a new method inside the CloseableRetryableRefreshListener which gets executed exactly once. The idempotent logic can continue to reside in the performAfterRefreshWithPermit method. We are also adding tests for existing classes that covers more flows.

Related Issues

Resolves #8717

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:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testReplicaHasDiffFilesThanPrimary
      1 org.opensearch.search.SearchWeightedRoutingIT.testSearchAggregationWithNetworkDisruption_FailOpenEnabled
      1 org.opensearch.remotestore.multipart.RemoteStoreMultipartIT.testStaleCommitDeletionWithInvokeFlush
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPitCreatedOnReplica
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testNodeDropWithOngoingReplication

@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #8758 (ba0f515) into main (bb78930) will decrease coverage by 0.04%.
Report is 1 commits behind head on main.
The diff coverage is 74.30%.

@@             Coverage Diff              @@
##               main    #8758      +/-   ##
============================================
- Coverage     71.06%   71.02%   -0.04%     
- Complexity    57314    57331      +17     
============================================
  Files          4766     4765       -1     
  Lines        270453   270478      +25     
  Branches      39555    39552       -3     
============================================
- Hits         192197   192115      -82     
- Misses        62045    62199     +154     
+ Partials      16211    16164      -47     
Files Changed Coverage Δ
.../opensearch/client/indices/CreateIndexRequest.java 75.55% <0.00%> (-1.12%) ⬇️
...opensearch/client/slm/SnapshotLifecyclePolicy.java 0.00% <ø> (ø)
...ch/client/slm/SnapshotLifecyclePolicyMetadata.java 0.00% <ø> (ø)
.../opensearch/client/slm/SnapshotLifecycleStats.java 0.00% <ø> (ø)
...rch/client/slm/SnapshotRetentionConfiguration.java 0.00% <ø> (ø)
...h/script/mustache/MultiSearchTemplateResponse.java 59.01% <ø> (ø)
...rg/opensearch/index/rankeval/RankEvalResponse.java 88.57% <ø> (-1.43%) ⬇️
...va/org/opensearch/index/rankeval/RankEvalSpec.java 97.36% <ø> (ø)
...a/org/opensearch/index/rankeval/RatedDocument.java 81.03% <0.00%> (ø)
...va/org/opensearch/index/rankeval/RatedRequest.java 91.79% <ø> (ø)
... and 133 more

... and 450 files with indirect coverage changes

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockIsRemovedWhenAnyNodesNotExceedHighWatermarkWithAutoReleaseEnabled

@ashking94
Copy link
Member Author

Rebased with main and committed as single commit as it was getting difficult to rebase every time.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

Gradle Check (Jenkins) Run Completed with:

@ashking94
Copy link
Member Author

Gradle Check (Jenkins) Run Completed with:

Flaky tests - #9048, #9130, #8887, #9048

@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:


> Task :checkCompatibility
Checking compatibility for: https://github.com/opensearch-project/reporting.git with ref: main
Incompatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/cross-cluster-replication.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]
Compatible components: [https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/opensearch-oci-object-storage.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]

BUILD SUCCESSFUL in 22m 57s

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2023

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Ashish Singh <ssashish@amazon.com>
@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:



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

BUILD SUCCESSFUL in 23m 14s

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2023

Gradle Check (Jenkins) Run Completed with:

@sachinpkale sachinpkale merged commit 2416d45 into opensearch-project:main Aug 6, 2023
10 checks passed
@sachinpkale sachinpkale added the backport 2.x Backport to 2.x branch label Aug 6, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 6, 2023
Signed-off-by: Ashish Singh <ssashish@amazon.com>
(cherry picked from commit 2416d45)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ashking94 added a commit to ashking94/OpenSearch that referenced this pull request Aug 11, 2023
ashking94 added a commit to ashking94/OpenSearch that referenced this pull request Aug 12, 2023
ashking94 added a commit to ashking94/OpenSearch that referenced this pull request Aug 12, 2023
ashking94 added a commit to ashking94/OpenSearch that referenced this pull request Aug 12, 2023
ashking94 added a commit to ashking94/OpenSearch that referenced this pull request Aug 13, 2023
sachinpkale pushed a commit that referenced this pull request Aug 14, 2023
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
…arch-project#8758)

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…arch-project#8758)

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…arch-project#8758)

Signed-off-by: Ashish Singh <ssashish@amazon.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 skip-changelog
Projects
None yet
5 participants