Run TransportGetAliasesAction on local node #101815
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This action is a pure function of the cluster state, it can run on any node. Moreover it can be fairly expensive if there are a lot of aliases so running it on the master can be quite harmful to the cluster. Finally, it needs to be cancellable to avoid doing unnecessary work after a client failure or timeout.
DaveCTurner
added
>enhancement
:Data Management/Indices APIs
elasticsearchmachine
added
the
Team:Data Management
|
Hi @DaveCTurner, I've created a changelog YAML for you. |
|
Pinging @elastic/es-data-management (Team:Data Management) |
38 tasks
idegtiarenko
reviewed
Nov 6, 2023
| @Override | ||
| public Task createTask(long id, String type, String action, TaskId parentTaskId, Map<String, String> headers) { | ||
| return new CancellableTask(id, type, action, "", parentTaskId, headers); | ||
| } |
There was a problem hiding this comment.
This (overriding createTask to return CancellableTask) seems to be repeating a lot around the code base.
Is it possible to do it in a common base class? Possibly something like:
default Task createTask(long id, String type, String action, TaskId parentTaskId, Map<String, String> headers) {
return this instance of CancellableRequest ?
new CancellableTask(id, type, action, getDescription(), parentTaskId, headers);
new Task(id, type, action, getDescription(), parentTaskId, headers);
}
where CancellableRequest would be a new marker interface?
There was a problem hiding this comment.
Eh maybe, it's kinda tricky when everything already derives from ActionRequest tho. I expect #100111 will make this simpler. Not in scope here tho.
idegtiarenko
reviewed
Nov 6, 2023
| * NB prior to 8.12 this was a TransportMasterNodeReadAction so for BwC it must be registered with the TransportService (i.e. a | ||
| * HandledTransportAction) until we no longer need to support {@link org.elasticsearch.TransportVersions#CLUSTER_FEATURES_ADDED} and | ||
| * earlier. | ||
| */ |
There was a problem hiding this comment.
Seems this is going to be true for all actions from the list in the task.
Is it worth to mention it once in TransportLocalClusterStateAction and ideally have a constant that would let us know when to remove HandledTransportAction base from there?
There was a problem hiding this comment.
I doubt we'll get all of them done by the time v9 is released, so I'd rather keep it a case-by-case thing.
idegtiarenko
approved these changes
Nov 6, 2023
DaveCTurner
added
the
auto-merge
ywangd
reviewed
Nov 6, 2023
server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java
Show resolved
Hide resolved
DaveCTurner
deleted the
2023/11/06/TransportGetAliasesAction-always-local
branch
elasticmachine
pushed a commit
to gmarouli/elasticsearch
that referenced
this pull request
Nov 20, 2023
In elastic#101815 we made some changes to the get-aliases API that need followup actions when preparing to release v9. At the time they were marked with a transport version that will become obsolete in v9, but with elastic#101767 we now have a better mechanism for leaving a reminder for this kind of action. This commit updates things to use the new @UpdateForV9 annotation instead.
andreidan
pushed a commit
to andreidan/elasticsearch
that referenced
this pull request
Nov 22, 2023
In elastic#101815 we made some changes to the get-aliases API that need followup actions when preparing to release v9. At the time they were marked with a transport version that will become obsolete in v9, but with elastic#101767 we now have a better mechanism for leaving a reminder for this kind of action. This commit updates things to use the new @UpdateForV9 annotation instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This action is a pure function of the cluster state, it can run on any
node. Moreover it can be fairly expensive if there are a lot of aliases
so running it on the master can be quite harmful to the cluster.
Finally, it needs to be cancellable to avoid doing unnecessary work
after a client failure or timeout.
Relates #101805