-
Notifications
You must be signed in to change notification settings - Fork 130
Add support for include_aliases, ignore_index_settings, and number_of_replicas in convert_index_to_remote action #1527
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
base: main
Are you sure you want to change the base?
Conversation
…ings to convert_index_to_remote action in ISM Signed-off-by: Seung Yeon Joo <[email protected]>
Signed-off-by: Seung Yeon Joo <[email protected]>
Signed-off-by: Seung Yeon Joo <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1527 +/- ##
==========================================
+ Coverage 76.11% 76.25% +0.13%
==========================================
Files 375 375
Lines 17565 17622 +57
Branches 2409 2412 +3
==========================================
+ Hits 13370 13437 +67
+ Misses 2958 2939 -19
- Partials 1237 1246 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Seung Yeon Joo <[email protected]>
| out.writeBoolean(includeAliases) | ||
| out.writeString(ignoreIndexSettings) | ||
| out.writeInt(numberOfReplicas) |
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.
We need a version check here to maintain BWC during rolling upgrades.
| val includeAliases = sin.readBoolean() | ||
| val ignoreIndexSettings = sin.readString() | ||
| val numberOfReplicas = sin.readInt() |
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.
Same as above, we need a version check here to maintain BWC during rolling upgrades.
|
|
||
| when (response.status()) { | ||
| RestStatus.ACCEPTED, RestStatus.OK -> { | ||
| deleteOriginalIndex(context, indexName, mutableInfo) |
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.
Since the snapshot restore is not waiting for completion, I worry delete index here would disrupt the search.
I see you mentioned this:
the action automatically deletes the original index once the restore operation is accepted (status: ACCEPTED or OK), ensuring a clean conversion process where only the remote index remains.
But I think we should keep this an option for user, and disabled by default.
Description
This PR adds three new configuration options to the
convert_index_to_remoteaction, providing more flexibility and control during remote snapshot restore operations:include_aliases(boolean, default:false): Controls whether index aliases should be included during the restore operation.ignore_index_settings(string, default: empty): Comma-separated list of index settings to ignore during restore (e.g.,"index.refresh_interval,index.number_of_replicas").number_of_replicas(integer, default:0): Sets the number of replicas for the restored remote index.Problem Statement:
For
number_of_replicas:When converting an index to a remote snapshot (Searchable Snapshot), users currently need to set the number of replicas either before the snapshot action or after the
convert_index_to_remoteaction completes. Both approaches have drawbacks:For
include_aliasesandignore_index_settings:Previously, the
convert_index_to_remoteaction lacked the ability to control alias inclusion and index settings during restore, limiting users' ability to fine-tune the restore behavior to match their specific requirements. These options are available in the manual restore API but were not exposed through the ISM action.For original index deletion:
The action now automatically deletes the original index after the remote snapshot restore is successfully accepted. This ensures that only the remote index remains, completing the conversion process and avoiding duplicate indices.
Solution:
This change adds three new fields to the
convert_index_to_remoteaction, aligning it with the manual restore API capabilities:include_aliases: Maps toRestoreSnapshotRequest.includeAliases()ignore_index_settings: Maps toRestoreSnapshotRequest.ignoreIndexSettings()number_of_replicas: Applied viaRestoreSnapshotRequest.indexSettings()withSETTING_NUMBER_OF_REPLICASAdditionally, the action automatically deletes the original index once the restore operation is accepted (status:
ACCEPTEDorOK), ensuring a clean conversion process where only the remote index remains.Implementation Details:
includeAliases: Boolean = falseparameter toConvertIndexToRemoteActionclassignoreIndexSettings: String = ""parameter toConvertIndexToRemoteActionclassnumberOfReplicas: Int = 0parameter toConvertIndexToRemoteActionclassConvertIndexToRemoteActionParserto parse all three fields from XContent and StreamInputAttemptRestoreStepto apply all three settings in theRestoreSnapshotRequest:includeAliases()method for alias inclusionignoreIndexSettings()method for ignored settingsindexSettings()method withSETTING_NUMBER_OF_REPLICASfor replica countinclude_aliases(type: boolean)ignore_index_settings(type: keyword)number_of_replicas(type: integer)Example Usage:
{ "convert_index_to_remote": { "repository": "backup-repo", "snapshot": "daily-snapshot", "include_aliases": true, "ignore_index_settings": "index.refresh_interval,index.number_of_replicas", "number_of_replicas": 0 } }Related Issues
Resolves #1487, #808 (comment)
Check List
--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.