Skip to content

restrict generic parameter type in ClusterStateTaskExecutor #83024

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 31, 2022
Merged

restrict generic parameter type in ClusterStateTaskExecutor #83024

merged 3 commits into from
Jan 31, 2022

Conversation

idegtiarenko
Copy link
Contributor

Currently, submitStateUpdateTask only accept tasks that implement
ClusterStateTaskListener. This pr adjusts the ClusterStateTaskExecutor
to have similar restriction.

Related to: #82644

@idegtiarenko
restrict generic parameter type in ClusterStateTaskExecutor
b482576 
Currently, submitStateUpdateTask only accept tasks that implement
ClusterStateTaskListener. This pr adjusts the ClusterStateTaskExecutor
to have similar restriction.
@idegtiarenko idegtiarenko added >refactoring :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.1.0 labels Jan 25, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

idegtiarenko
idegtiarenko commented Jan 25, 2022
View reviewed changes
server/src/main/java/org/elasticsearch/cluster/service/MasterService.java
@@ -180,10 +180,15 @@ protected void run(Object batchingKey, List<? extends BatchedTask> tasks, String

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we can not remove the listener as it is wrapped with safe during the task creation at

elasticsearch/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java

Line 925 in b482576

.map(e -> taskBatcher.new UpdateTask(config.priority(), source, e, safe(e, supplier), executor))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think that's right for now at least. This prompted me to realise that we're missing something similar in a few other places: #83019, #83021, #83035. Fortunately I don't think anything relies on the context being correct in those places (... yet).

DaveCTurner
DaveCTurner approved these changes Jan 30, 2022
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DaveCTurner DaveCTurner mentioned this pull request Jan 30, 2022
Closed
9 tasks
@idegtiarenko
Merge branch 'master' into 82644_generic_param
e6c01df 
# Conflicts:
#	server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java
@idegtiarenko idegtiarenko merged commit 0431d85 into elastic:master Jan 31, 2022
@idegtiarenko idegtiarenko deleted the 82644_generic_param branch January 31, 2022 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaveCTurner DaveCTurner DaveCTurner approved these changes

Assignees
No one assigned
Labels
:Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >refactoring Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@idegtiarenko @elasticmachine @DaveCTurner