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

[Remote Store] Add support to add nested settings for remote store #4060

Merged

Conversation

sachinpkale
Copy link
Member

@sachinpkale sachinpkale commented Aug 1, 2022

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

Description

Current setting index.remote_store removes the flexibility of adding few sub-settings related to remote store for the same index. In the future, we can have requirements like supporting only remote trans-log, separate remote store for given index. With this change, we introduce index.remote_store.enabled, which will allow inclusion of all the remote_store settings under index.remote_store.

Issues Resolved

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

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.

@sachinpkale sachinpkale requested review from a team and reta as code owners August 1, 2022 04:56
@sachinpkale sachinpkale changed the title Add support to add nested settings for remote store [Remote Store] Add support to add nested settings for remote store Aug 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@sachinpkale
Copy link
Member Author

Build failed with: > java.lang.OutOfMemoryError: Java heap space, force pushing the changes again.

@sachinpkale sachinpkale force-pushed the remote_store_settings_change branch from 540e4bb to b95a7ef Compare August 1, 2022 05:45
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@sachinpkale sachinpkale force-pushed the remote_store_settings_change branch from b95a7ef to ec5da2e Compare August 2, 2022 02:06
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

@dreamer-89
Copy link
Member

@sachinpkale : Looks like a legit failure in last gradle check failure.

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.index.IndexSettingsTests.testUpdateRemoteStoreFails" -Dtests.seed=4CDBB95C3D84618E -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=he -Dtests.timezone=America/Antigua -Druntime.java=17

org.opensearch.index.IndexSettingsTests > testUpdateRemoteStoreFails FAILED
    org.junit.ComparisonFailure: expected:<[index setting [index.remote_store], not recognized]> but was:<[final index setting [index.remote_store], not updateable]>
        at __randomizedtesting.SeedInfo.seed([4CDBB95C3D84618E:16581A01075C7ACB]:0)
        at org.junit.Assert.assertEquals(Assert.java:117)
        at org.junit.Assert.assertEquals(Assert.java:146)
        at org.opensearch.index.IndexSettingsTests.testUpdateRemoteStoreFails(IndexSettingsTests.java:793)

@dreamer-89 dreamer-89 self-requested a review August 2, 2022 02:42
@sachinpkale
Copy link
Member Author

@sachinpkale : Looks like a legit failure in last gradle check failure.

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.index.IndexSettingsTests.testUpdateRemoteStoreFails" -Dtests.seed=4CDBB95C3D84618E -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=he -Dtests.timezone=America/Antigua -Druntime.java=17

org.opensearch.index.IndexSettingsTests > testUpdateRemoteStoreFails FAILED
    org.junit.ComparisonFailure: expected:<[index setting [index.remote_store], not recognized]> but was:<[final index setting [index.remote_store], not updateable]>
        at __randomizedtesting.SeedInfo.seed([4CDBB95C3D84618E:16581A01075C7ACB]:0)
        at org.junit.Assert.assertEquals(Assert.java:117)
        at org.junit.Assert.assertEquals(Assert.java:146)
        at org.opensearch.index.IndexSettingsTests.testUpdateRemoteStoreFails(IndexSettingsTests.java:793)

Ohh! Let me fix it. Thanks @dreamer-89 !

Signed-off-by: Sachin Kale <kalsac@amazon.com>
@sachinpkale sachinpkale force-pushed the remote_store_settings_change branch from ec5da2e to b9f413c Compare August 3, 2022 05:08
Signed-off-by: Sachin Kale <kalsac@amazon.com>
@sachinpkale sachinpkale force-pushed the remote_store_settings_change branch from b9f413c to f95de5c Compare August 3, 2022 05:08
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2022

Gradle Check (Jenkins) Run Completed with:

@dreamer-89
Copy link
Member

Gradle Check (Jenkins) Run Completed with:

Jenkins pipeline failure

[Pipeline] End of Pipeline
ERROR: script returned exit code 128
[Checks API] No suitable checks publisher found.
Finished: FAILURE

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2022

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #4060 (f95de5c) into main (66c24ff) will increase coverage by 0.13%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##               main    #4060      +/-   ##
============================================
+ Coverage     70.56%   70.70%   +0.13%     
- Complexity    56938    57036      +98     
============================================
  Files          4588     4588              
  Lines        274132   274132              
  Branches      40178    40178              
============================================
+ Hits         193454   193813     +359     
+ Misses        64452    64011     -441     
- Partials      16226    16308      +82     
Impacted Files Coverage Δ
...pensearch/common/settings/IndexScopedSettings.java 100.00% <ø> (ø)
.../java/org/opensearch/snapshots/RestoreService.java 55.37% <0.00%> (ø)
...org/opensearch/cluster/metadata/IndexMetadata.java 84.36% <100.00%> (+0.50%) ⬆️
.../main/java/org/opensearch/index/IndexSettings.java 86.35% <100.00%> (ø)
.../java/org/opensearch/node/NodeClosedException.java 50.00% <0.00%> (-50.00%) ⬇️
.../index/shard/IndexShardNotRecoveringException.java 0.00% <0.00%> (-50.00%) ⬇️
...search/search/aggregations/pipeline/EwmaModel.java 24.44% <0.00%> (-28.89%) ⬇️
...rc/main/java/org/opensearch/ingest/IngestInfo.java 51.72% <0.00%> (-27.59%) ⬇️
...a/org/opensearch/index/mapper/MapperException.java 75.00% <0.00%> (-25.00%) ⬇️
...ion/admin/cluster/node/info/PluginsAndModules.java 53.12% <0.00%> (-25.00%) ⬇️
... and 488 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@Bukhtawar Bukhtawar merged commit 6f23300 into opensearch-project:main Aug 3, 2022
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 1, 2022
…pensearch-project#4060)

* Add support to add nested settings for remote store

Signed-off-by: Sachin Kale <kalsac@amazon.com>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 2, 2022
…pensearch-project#4060)

* Add support to add nested settings for remote store

Signed-off-by: Sachin Kale <kalsac@amazon.com>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 2, 2022
…pensearch-project#4060)

* Add support to add nested settings for remote store

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Bukhtawar pushed a commit that referenced this pull request Sep 2, 2022
…4380)

* [Remote Store] Upload segments to remote store post refresh (#3460)

* Add RemoteDirectory interface to copy segment files to/from remote store

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

Co-authored-by: Sachin Kale <kalsac@amazon.com>

* Add index level setting for remote store

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

Co-authored-by: Sachin Kale <kalsac@amazon.com>

* Add RemoteDirectoryFactory and use RemoteDirectory instance in RefreshListener

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

* Upload segment to remote store post refresh

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

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

* [Remote Store] Inject remote store in IndexShard instead of RemoteStoreRefreshListener (#3703)

* Inject remote store in IndexShard instead of RemoteStoreRefreshListener

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

* Pass supplier of RepositoriesService to RemoteDirectoryFactory

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

* Create isRemoteStoreEnabled function for IndexShard

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

* Explicitly close remoteStore on indexShard close

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

* Change RemoteDirectory.close to a no-op

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

Co-authored-by: Sachin Kale <kalsac@amazon.com>

* [Remote Store] Add remote store restore API implementation (#3642)

* Add remote restore API implementation

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

* [Remote Store] Add support to add nested settings for remote store (#4060)

* Add support to add nested settings for remote store

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

* [Remote Store] Add rest endpoint for remote store restore (#3576)

* Add rest endpoint for remote store restore

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

* [Remote Store] Add validator that forces segment replication type before enabling remote store (#4175)

* Add validator that forces segment replication type before enabling remote store

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

* [Remote Store] Change remote_store setting validation message to make it more clear (#4199)

* Change remote_store setting validation message to make it more clear

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

* [Remote Store] Add RemoteSegmentStoreDirectory to interact with remote segment store (#4020)

* Add RemoteSegmentStoreDirectory to interact with remote segment store

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

* Use RemoteSegmentStoreDirectory instead of RemoteDirectory (#4240)

* Use RemoteSegmentStoreDirectory instead of RemoteDirectory

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

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Co-authored-by: Sachin Kale <kalsac@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants