Add default snapshot repo cluster setting#139155
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a cluster setting to specify a default snapshot repository for snapshot and restore operations. When configured, this eliminates the need to explicitly specify a repository name in these operations.
Key changes:
- Added
repositories.default_repositorydynamic cluster setting - Implemented validation to ensure only registered repositories can be set as default
- Added protection to prevent deletion of the default repository
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
RepositoriesService.java |
Adds the default repository cluster setting, validation logic, getter/setter methods, and prevention of default repository deletion |
ClusterSettings.java |
Registers the new DEFAULT_REPOSITORY_SETTING in the cluster settings registry |
10_default_repository.yml |
Adds comprehensive YAML REST tests covering setting, validation, and edge cases for the default repository feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Show resolved
Hide resolved
…esService.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…esService.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Outdated
Show resolved
Hide resolved
|
Pinging @elastic/es-data-management (Team:Data Management) |
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Outdated
Show resolved
Hide resolved
…m being read-only
|
Based on our discussions, i added some additional logic to ensure default repos are not read-only, and only validating on the master node/master update thread. Thank you everybody for the comments, learning a lot on this one! |
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java
Show resolved
Hide resolved
masseyke
left a comment
There was a problem hiding this comment.
LGTM, assuming the conversations about whether to validate on start and whether to handle multi-project have been resolved.
I left a few minor comments.
This PR adds a cluster setting for the default repository. The setting defaults to "".
The default repo cannot be deleted, you must change the setting before being able to delete a repo. We check that the repo is registered upon updating the setting. The setting can be cleared by setting it to ""
closes #66040
closes https://github.com/elastic/elasticsearch-team/issues/2103