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
idegtiarenko merged 3 commits into from
Jan 31, 2022
Merged

restrict generic parameter type in ClusterStateTaskExecutor #83024

idegtiarenko merged 3 commits into from
Jan 31, 2022

Conversation

@idegtiarenko
Copy link
Contributor

@idegtiarenko idegtiarenko commented Jan 25, 2022

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 Meta label for distributed team. v8.1.0 labels Jan 25, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jan 25, 2022

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
super(priority, source, executor, task);
this.listener = listener;
}

Copy link
Contributor Author

@idegtiarenko idegtiarenko Jan 25, 2022

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

@DaveCTurner DaveCTurner Jan 25, 2022

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 Meta label for distributed team. v8.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@idegtiarenko @elasticmachine @DaveCTurner