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

Add node id to segment and translog metadata #10229

Merged
merged 12 commits into from
Oct 4, 2023

Conversation

gbbafna
Copy link
Collaborator

@gbbafna gbbafna commented Sep 26, 2023

Adding validation to identify multiple writers to same primary term and generation in remote store

Description

Adding node id to remote segment and translog metadata files . This is to identify if we have concurrent writers to same file , causing consistency issues.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

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

github-actions bot commented Sep 26, 2023

Compatibility status:

Checks if related components are compatible with change 43fbb5d

Incompatible components

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/cross-cluster-replication.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git]

@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:

@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:
      1 org.opensearch.search.SearchWeightedRoutingIT.testMultiGetWithNetworkDisruption_FailOpenEnabled

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #10229 (43fbb5d) into main (d3bf230) will decrease coverage by 0.07%.
Report is 5 commits behind head on main.
The diff coverage is 89.39%.

@@             Coverage Diff              @@
##               main   #10229      +/-   ##
============================================
- Coverage     71.15%   71.09%   -0.07%     
+ Complexity    58247    58239       -8     
============================================
  Files          4831     4831              
  Lines        274684   274736      +52     
  Branches      40026    40034       +8     
============================================
- Hits         195443   195312     -131     
- Misses        62884    63085     +201     
+ Partials      16357    16339      -18     
Files Coverage Δ
...rch/extensions/rest/RestActionsRequestHandler.java 100.00% <100.00%> (ø)
...extensions/rest/RestInitializeExtensionAction.java 65.00% <100.00%> (-0.44%) ⬇️
...rch/extensions/rest/RestSendToExtensionAction.java 42.14% <100.00%> (ø)
...c/main/java/org/opensearch/index/IndexService.java 75.65% <100.00%> (+0.05%) ⬆️
.../org/opensearch/index/remote/RemoteStoreUtils.java 96.66% <100.00%> (+2.22%) ⬆️
...in/java/org/opensearch/index/shard/IndexShard.java 69.58% <100.00%> (-0.46%) ⬇️
...search/index/shard/RemoteStoreRefreshListener.java 86.09% <100.00%> (+0.07%) ⬆️
...earch/index/store/RemoteSegmentStoreDirectory.java 89.93% <100.00%> (+0.25%) ⬆️
...rg/opensearch/index/translog/RemoteFsTranslog.java 72.59% <100.00%> (+1.09%) ⬆️
.../org/opensearch/index/translog/TranslogConfig.java 100.00% <100.00%> (ø)
... and 6 more

... and 447 files with indirect coverage changes

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Gradle Check (Jenkins) Run Completed with:

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

github-actions bot commented Oct 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.basic.SearchWithRandomIOExceptionsIT.testRandomDirectoryIOExceptions {p0={"search.concurrent_segment_search.enabled":"false"}}
      1 org.opensearch.search.basic.SearchWithRandomIOExceptionsIT.classMethod
      1 org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards

@sachinpkale sachinpkale changed the title Adding node id to segment and translog metadata Add node id to segment and translog metadata Oct 4, 2023
@sachinpkale sachinpkale merged commit 7159e2e into opensearch-project:main Oct 4, 2023
21 checks passed
@sachinpkale sachinpkale added the backport 2.x Backport to 2.x branch label Oct 4, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 4, 2023
* Adding node id to segment and translog metadata

Adding validation to identify multiple writers to same primary term and
generation in remote store

Signed-off-by: Gaurav Bafna <[email protected]>

* Fix bug to find node id

Signed-off-by: Gaurav Bafna <[email protected]>

* Spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* Moving node id before uuid as it was interfering with _ character split

Signed-off-by: Gaurav Bafna <[email protected]>

* Removing uuid from segment metadata file

Signed-off-by: Gaurav Bafna <[email protected]>

* simplifying the detection logic

Signed-off-by: Gaurav Bafna <[email protected]>

* PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Addressing PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Adding translog gen to remote segment metadata fn as well

Signed-off-by: Gaurav Bafna <[email protected]>

* spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* reducing METADATA_FILES_TO_FETCH  to 10

Signed-off-by: Gaurav Bafna <[email protected]>

* adding missing import

Signed-off-by: Gaurav Bafna <[email protected]>

---------

Signed-off-by: Gaurav Bafna <[email protected]>
(cherry picked from commit 7159e2e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Comment on lines +92 to +98
throw new IllegalStateException(
"Multiple metadata files from different nodes"
+ nodeIdByPrimaryTermAndGen.v1()
+ " and "
+ nodeIdByPrimaryTermAndGen.v2()
+ "having same primary term and generations detected"
);
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding more details

@rramachand21 rramachand21 added the v2.11.0 Issues and PRs related to version 2.11.0 label Oct 4, 2023
sachinpkale pushed a commit that referenced this pull request Oct 4, 2023
---------


(cherry picked from commit 7159e2e)

Signed-off-by: Gaurav Bafna <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
deshsidd pushed a commit to deshsidd/OpenSearch that referenced this pull request Oct 9, 2023
* Adding node id to segment and translog metadata

Adding validation to identify multiple writers to same primary term and
generation in remote store

Signed-off-by: Gaurav Bafna <[email protected]>

* Fix bug to find node id

Signed-off-by: Gaurav Bafna <[email protected]>

* Spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* Moving node id before uuid as it was interfering with _ character split

Signed-off-by: Gaurav Bafna <[email protected]>

* Removing uuid from segment metadata file

Signed-off-by: Gaurav Bafna <[email protected]>

* simplifying the detection logic

Signed-off-by: Gaurav Bafna <[email protected]>

* PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Addressing PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Adding translog gen to remote segment metadata fn as well

Signed-off-by: Gaurav Bafna <[email protected]>

* spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* reducing METADATA_FILES_TO_FETCH  to 10

Signed-off-by: Gaurav Bafna <[email protected]>

* adding missing import

Signed-off-by: Gaurav Bafna <[email protected]>

---------

Signed-off-by: Gaurav Bafna <[email protected]>
vikasvb90 pushed a commit to vikasvb90/OpenSearch that referenced this pull request Oct 10, 2023
* Adding node id to segment and translog metadata

Adding validation to identify multiple writers to same primary term and
generation in remote store

Signed-off-by: Gaurav Bafna <[email protected]>

* Fix bug to find node id

Signed-off-by: Gaurav Bafna <[email protected]>

* Spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* Moving node id before uuid as it was interfering with _ character split

Signed-off-by: Gaurav Bafna <[email protected]>

* Removing uuid from segment metadata file

Signed-off-by: Gaurav Bafna <[email protected]>

* simplifying the detection logic

Signed-off-by: Gaurav Bafna <[email protected]>

* PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Addressing PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Adding translog gen to remote segment metadata fn as well

Signed-off-by: Gaurav Bafna <[email protected]>

* spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* reducing METADATA_FILES_TO_FETCH  to 10

Signed-off-by: Gaurav Bafna <[email protected]>

* adding missing import

Signed-off-by: Gaurav Bafna <[email protected]>

---------

Signed-off-by: Gaurav Bafna <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
* Adding node id to segment and translog metadata

Adding validation to identify multiple writers to same primary term and
generation in remote store

Signed-off-by: Gaurav Bafna <[email protected]>

* Fix bug to find node id

Signed-off-by: Gaurav Bafna <[email protected]>

* Spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* Moving node id before uuid as it was interfering with _ character split

Signed-off-by: Gaurav Bafna <[email protected]>

* Removing uuid from segment metadata file

Signed-off-by: Gaurav Bafna <[email protected]>

* simplifying the detection logic

Signed-off-by: Gaurav Bafna <[email protected]>

* PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Addressing PR comments

Signed-off-by: Gaurav Bafna <[email protected]>

* Adding translog gen to remote segment metadata fn as well

Signed-off-by: Gaurav Bafna <[email protected]>

* spotless fix

Signed-off-by: Gaurav Bafna <[email protected]>

* reducing METADATA_FILES_TO_FETCH  to 10

Signed-off-by: Gaurav Bafna <[email protected]>

* adding missing import

Signed-off-by: Gaurav Bafna <[email protected]>

---------

Signed-off-by: Gaurav Bafna <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
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 v2.11.0 Issues and PRs related to version 2.11.0
Projects
Status: 2.11.0 - (Launched)
Development

Successfully merging this pull request may close these issues.

5 participants