Skip to content

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

Merged
Bukhtawar merged 3 commits intoopensearch-project:mainfrom
sachinpkale:main
Aug 12, 2022
Merged

[Remote Store] Change remote_store setting validation message to make it more clear#4199
Bukhtawar merged 3 commits intoopensearch-project:mainfrom
sachinpkale:main

Conversation

@sachinpkale
Copy link
Copy Markdown
Member

@sachinpkale sachinpkale commented Aug 12, 2022 •
edited
Loading

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

Description

  • Currently, if we try to enable index.remote_store.enabled without setting replication type to SEGMENT, we get following error:
{
  "error" : {
    "root_cause" : [
      {
        "type" : "illegal_argument_exception",
        "reason" : "Settings index.remote_store.enabled cannot be enabled when index.replication.type is set to DOCUMENT"
      }
    ],
    "type" : "illegal_argument_exception",
    "reason" : "Settings index.remote_store.enabled cannot be enabled when index.replication.type is set to DOCUMENT"
  },
  "status" : 400
}
  • But this does not make the dependency of segment replication clear. With this change, we are changing the error message to:
{
  "error" : {
    "root_cause" : [
      {
        "type" : "illegal_argument_exception",
        "reason" : "To enable index.remote_store.enabled, index.replication.type should be set to SEGMENT"
      }
    ],
    "type" : "illegal_argument_exception",
    "reason" : "To enable index.remote_store.enabled, index.replication.type should be set to SEGMENT"
  },
  "status" : 400
}

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.

Change remote_store setting validation message to make it more clear
d6dea08 
Signed-off-by: Sachin Kale <kalsac@amazon.com>
@sachinpkale sachinpkale requested review from a team and reta as code owners August 12, 2022 07:00
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 12, 2022

Gradle Check (Jenkins) Run Completed with:

@BhumikaSaini-Amazon
Copy link
Copy Markdown
Contributor

BhumikaSaini-Amazon commented Aug 12, 2022

Thanks for adding this!
Please could you help update the unit tests as well for this change?

@sachinpkale
Copy link
Copy Markdown
Member Author

sachinpkale commented Aug 12, 2022

Please could you help update the unit tests as well for this change?

Forgot to update UTs. Thanks @BhumikaSaini-Amazon

Fix UTs
13999e0 
Signed-off-by: Sachin Kale <kalsac@amazon.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 12, 2022

Gradle Check (Jenkins) Run Completed with:

Apply Spotless check
7cd1001 
Signed-off-by: Sachin Kale <kalsac@amazon.com>
@BhumikaSaini-Amazon
Copy link
Copy Markdown
Contributor

BhumikaSaini-Amazon commented Aug 12, 2022

Looks good to me 👍
Please link the respective Issue to this PR and strike off the PR checklist items
Thanks!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 12, 2022

Gradle Check (Jenkins) Run Completed with:

@sachinpkale
Copy link
Copy Markdown
Member Author

sachinpkale commented Aug 12, 2022

Looks good to me 👍 Please link the respective Issue to this PR and strike off the PR checklist items Thanks!

@BhumikaSaini-Amazon Added issue link and updated PR checklist.

@Bukhtawar Bukhtawar merged commit 8ccedd8 into opensearch-project:main Aug 12, 2022
dreamer-89 pushed a commit to dreamer-89/OpenSearch that referenced this pull request Aug 12, 2022
@sachinpkale @dreamer-89
[Remote Store] Change remote_store setting validation message to make…
d344209 
… it more clear (opensearch-project#4199)

* Change remote_store setting validation message to make it more clear

Signed-off-by: Sachin Kale <kalsac@amazon.com>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 1, 2022
@sachinpkale
[Remote Store] Change remote_store setting validation message to make…
1f973a2 
… it more clear (opensearch-project#4199)

* Change remote_store setting validation message to make it more clear

Signed-off-by: Sachin Kale <kalsac@amazon.com>
@sachinpkale sachinpkale mentioned this pull request Sep 1, 2022
Merged
6 tasks
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 2, 2022
@sachinpkale
[Remote Store] Change remote_store setting validation message to make…
9a4ede8 
… it more clear (opensearch-project#4199)

* Change remote_store setting validation message to make it more clear

Signed-off-by: Sachin Kale <kalsac@amazon.com>
sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 2, 2022
@sachinpkale
[Remote Store] Change remote_store setting validation message to make…
85bc986 
… it more clear (opensearch-project#4199)

* Change remote_store setting validation message to make it more clear

Signed-off-by: Sachin Kale <kalsac@amazon.com>
Bukhtawar pushed a commit that referenced this pull request Sep 2, 2022
@sachinpkale
[Backport 2.x] [Remote Store] Backport remote segment store changes (#…
03273b5 
…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

Reviewers

@Bukhtawar Bukhtawar Bukhtawar approved these changes

@reta reta Awaiting requested review from reta reta is a code owner

+1 more reviewer

@BhumikaSaini-Amazon BhumikaSaini-Amazon BhumikaSaini-Amazon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sachinpkale @BhumikaSaini-Amazon @Bukhtawar