- Notifications
You must be signed in to change notification settings - Fork 25.5k
Add RemoveBlock API to allow DELETE /{index}/_block/{block}
#129128
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 RemoveBlock API to allow DELETE /{index}/_block/{block}
#129128
Conversation
- Add RemoveIndexBlockRequest/Response/ClusterStateUpdateRequest classes - Add removeIndexBlock method to MetadataIndexStateService Provides core service infrastructure for removing index blocks.
- Add TransportRemoveIndexBlockAction and RemoveIndexBlockRequestBuilder - Add prepareRemoveBlock() and removeBlock() methods to IndicesAdminClient - Register RemoveBlock actions in ActionModule - Add integration tests and security permissions
- Add RestRemoveIndexBlockAction for DELETE /{index}/_block/{block} - Add indices.remove_block.json API specification - Update YAML REST tests and documentation - Remove shards_acknowledged field references from docs Completes public HTTP interface providing symmetry with addBlock API.
💚 CLA has been signed |
Pinging @elastic/es-data-management (Team:Data Management) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @HyunSangHan, thank you very much for your contribution to Elasticsearch! The PR is already looking good, I just have a few small comments for now - I need to have another look later on. I also pushed some changes myself, as they're hard to explain, but I didn't want to completely hijack your PR, so I'll let you take care of the remaining changes.
.../src/main/java/org/elasticsearch/action/admin/indices/readonly/RemoveIndexBlockResponse.java Outdated Show resolved Hide resolved
...r/src/main/java/org/elasticsearch/action/admin/indices/readonly/RemoveIndexBlockRequest.java Outdated Show resolved Hide resolved
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.blocks/10_basic.yml Show resolved Hide resolved
.../src/main/java/org/elasticsearch/action/admin/indices/readonly/RemoveIndexBlockResponse.java Outdated Show resolved Hide resolved
buildkite test this |
Replace implicit default timeouts with explicit parameters in constructor to improve code clarity and follow established patterns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed I didn't actually push my commit last time, so I just did so. I added a few more comments. Thanks for incorporating my previous comments so soon!
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.blocks/10_basic.yml Outdated Show resolved Hide resolved
server/src/internalClusterTest/java/org/elasticsearch/blocks/SimpleBlocksIT.java Outdated Show resolved Hide resolved
rest-api-spec/src/main/resources/rest-api-spec/api/indices.remove_block.json Outdated Show resolved Hide resolved
...rc/internalClusterTest/java/org/elasticsearch/operateAllIndices/DestructiveOperationsIT.java Outdated Show resolved Hide resolved
@nielsbauman |
🔍 Preview links for changed docs: 🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes. |
buildkite test this |
buildkite test this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @HyunSangHan, sorry for the late reply. I was discussing the changes with some colleagues who are more familiar with the inner workings of blocks. I added two more small comments, after that I think we're good to go.
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexStateService.java Outdated Show resolved Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexStateService.java Show resolved Hide resolved
- Add logic to MetadataIndexStateService.removeIndexBlock() to automatically clear VERIFIED_READ_ONLY_SETTING when no write-blocking constraints remain
server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexStateService.java Outdated Show resolved Hide resolved
buildkite test this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you very much for your contribution and iterations on this PR, @HyunSangHan!
@nielsbauman |
…c#129128) Introduces a new `RemoveBlock` API that complements the existing `AddBlock` API by allowing users to remove index blocks using `DELETE /{index}/_block/{block}`. Resolves elastic#128966 --------- Co-authored-by: Niels Bauman <nielsbauman@gmail.com>
Awesome work @HyunSangHan !! 👏👏👏 |
@nielsbauman |
Hi @HyunSangHan, thank you very much for addressing that and opening a new issue. I've asked someone from our docs team to have a look at what's going on there. |
* Add index block removal API Follow-up of elastic/elasticsearch#129128 * Fix block enum * Add doc-id to table * Fix lint * Move IndicesBlockOptions and IndicesBlockStatus to indices/_types It's breaking but avoids duplicating the type names, which is forbidden. * Separate BlockStatus as it is different * Rename response example file * Fix response fields * Add index privilege --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
* Add index block removal API Follow-up of elastic/elasticsearch#129128 * Fix block enum * Add doc-id to table * Fix lint * Move IndicesBlockOptions and IndicesBlockStatus to indices/_types It's breaking but avoids duplicating the type names, which is forbidden. * Separate BlockStatus as it is different * Rename response example file * Fix response fields * Add index privilege --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co> (cherry picked from commit e668a98)
* Add index block removal API Follow-up of elastic/elasticsearch#129128 * Fix block enum * Add doc-id to table * Fix lint * Move IndicesBlockOptions and IndicesBlockStatus to indices/_types It's breaking but avoids duplicating the type names, which is forbidden. * Separate BlockStatus as it is different * Rename response example file * Fix response fields * Add index privilege --------- (cherry picked from commit e668a98) Co-authored-by: Niels Bauman <33722607+nielsbauman@users.noreply.github.com> Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
We can find RemoveBlock API in the document after recent release: |
Summary
Introduces a new
RemoveBlock
API that complements the existingAddBlock
API by allowing users to remove index blocks usingDELETE /{index}/_block/{block}
.Resolves: #128966
Background
Currently, users add blocks via the
AddBlock
API (PUT /{index}/_block/{block}
) but must remove them through the more generic Settings API (PUT /{index}/_settings
).What’s Changed
Core
RemoveIndexBlockRequest
,RemoveIndexBlockResponse
,TransportRemoveIndexBlockAction
.removeIndexBlock
toMetadataIndexStateService
.REST API
DELETE /{index}/_block/{block}
indices.remove_block.json
AcknowledgedResponse
(no shard-level confirmation required)Features
Compatibility
Testing