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

Decrease store refcount on any failure to create NRTReplicationEngine #9796

Conversation

sachinpkale
Copy link
Member

@sachinpkale sachinpkale commented Sep 6, 2023

Description

  • Currently, NRTReplicationEngine calls store.incRef() in the constructor and corresponding store.decRef() is only called on IOException.
  • But in case of exceptions like TranslogCorruptedException, store is not de-referenced causing ShardLock issues.
  • In this PR, we make sure to always call store.decRef() if there is a failure in creating NRTReplicationEngine.

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

github-actions bot commented Sep 6, 2023

Compatibility status:

Checks if related components are compatible with change 8d37065

Incompatible components

Incompatible components: [https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/neural-search.git]

Skipped components

Compatible components

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testIndexDeletionDuringSnapshotCreationInQueue
      1 org.opensearch.search.SearchWeightedRoutingIT.testMultiGetWithNetworkDisruption_FailOpenEnabled

@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #9796 (f9ad560) into main (951f051) will increase coverage by 0.01%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##               main    #9796      +/-   ##
============================================
+ Coverage     71.02%   71.04%   +0.01%     
- Complexity    58040    58085      +45     
============================================
  Files          4831     4831              
  Lines        273927   273931       +4     
  Branches      39913    39915       +2     
============================================
+ Hits         194567   194604      +37     
+ Misses        63061    62985      -76     
- Partials      16299    16342      +43     
Files Changed Coverage Δ
.../opensearch/index/engine/NRTReplicationEngine.java 78.77% <85.71%> (+1.62%) ⬆️

... and 443 files with indirect coverage changes

Copy link
Contributor

@BhumikaSaini-Amazon BhumikaSaini-Amazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could you help add tests for this change?

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Compatibility status:

Checks if related components are compatible with change 6055d8c

Incompatible components

Incompatible components: [https://github.com/opensearch-project/cross-cluster-replication.git]

Skipped components

Compatible components

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Compatibility status:

Checks if related components are compatible with change 7be0ba7

Incompatible components

Incompatible components: [https://github.com/opensearch-project/cross-cluster-replication.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/alerting.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, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/reporting.git]

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Gradle Check (Jenkins) Run Completed with:

Sachin Kale added 3 commits September 7, 2023 15:20
Signed-off-by: Sachin Kale <kalsac@amazon.com>
Signed-off-by: Sachin Kale <kalsac@amazon.com>
Signed-off-by: Sachin Kale <kalsac@amazon.com>
@sachinpkale sachinpkale force-pushed the nrt-engine-translog-corruption branch from 7be0ba7 to f9ad560 Compare September 7, 2023 09:50
Copy link
Member

@ashking94 ashking94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Compatibility status:

Checks if related components are compatible with change f9ad560

Incompatible components

Incompatible components: [https://github.com/opensearch-project/cross-cluster-replication.git]

Skipped components

Compatible components

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.smoketest.SmokeTestMultiNodeClientYamlTestSuiteIT.test {yaml=pit/10_basic/Delete all}

@sachinpkale sachinpkale merged commit 199123d into opensearch-project:main Sep 7, 2023
@sachinpkale sachinpkale added backport 2.x Backport to 2.x branch backport 2.10 Backport to 2.10 branch labels Sep 7, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 7, 2023
…#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
(cherry picked from commit 199123d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 7, 2023
…#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
(cherry picked from commit 199123d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 7, 2023
…opensearch-project#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 7, 2023
…opensearch-project#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
gbbafna pushed a commit that referenced this pull request Sep 7, 2023
sachinpkale added a commit that referenced this pull request Sep 7, 2023
…#9796) (#9902)

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
…opensearch-project#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/OpenSearch that referenced this pull request Sep 20, 2023
…opensearch-project#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…opensearch-project#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@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
…opensearch-project#9796)

* Decrease store refcount on any failure to create NRTReplicationEngine

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Add unit tests

Signed-off-by: Sachin Kale <kalsac@amazon.com>

* Fix spotless

Signed-off-by: Sachin Kale <kalsac@amazon.com>

---------

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@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 backport 2.10 Backport to 2.10 branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants