Skip to content
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

Remove nodeId from BaseNodeRequest #43658

Merged
merged 3 commits into from
Jun 27, 2019
Merged

Remove nodeId from BaseNodeRequest #43658

merged 3 commits into from
Jun 27, 2019

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Jun 26, 2019

TransportNodesAction provides a mechanism to easily broadcast a request
to many nodes, and collect the respones into a high level response. Each
node has its own request type, with a base class of BaseNodeRequest.
This base request requires passing the nodeId to which the request will
be sent. However, that nodeId is not used anywhere. It is private to the
base class, yet serialized to each node, where the node could just as
easily find the nodeId of the node it is on locally.

This commit removes passing the nodeId through to the node request
creation, and guards its serialization so that we can remove the base
request class altogether in the future.

TransportNodesAction provides a mechanism to easily broadcast a request
to many nodes, and collect the respones into a high level response. Each
node has its own request type, with a base class of BaseNodeRequest.
This base request requires passing the nodeId to which the request will
be sent. However, that nodeId is not used anywhere. It is private to the
base class, yet serialized to each node, where the node could just as
easily find the nodeId of the node it is on locally.

This commit removes passing the nodeId through to the node request
creation, and guards its serialization so that we can remove the base
request class altogether in the future.
@rjernst rjernst added :Core/Infra/Core Core issues without another label v8.0.0 v7.3.0 labels Jun 26, 2019
@rjernst rjernst requested a review from imotov June 26, 2019 18:56
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@rjernst
Copy link
Member Author

rjernst commented Jun 26, 2019

@elasticmachine run elasticsearch-ci/1

Copy link
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

Nice cleanup! Cannot believe we were sending this around for such a long time.

@rjernst rjernst merged commit 25792d3 into elastic:master Jun 27, 2019
@rjernst rjernst deleted the deguice51 branch June 27, 2019 17:31
rjernst added a commit that referenced this pull request Jun 28, 2019
TransportNodesAction provides a mechanism to easily broadcast a request
to many nodes, and collect the respones into a high level response. Each
node has its own request type, with a base class of BaseNodeRequest.
This base request requires passing the nodeId to which the request will
be sent. However, that nodeId is not used anywhere. It is private to the
base class, yet serialized to each node, where the node could just as
easily find the nodeId of the node it is on locally.

This commit removes passing the nodeId through to the node request
creation, and guards its serialization so that we can remove the base
request class altogether in the future.
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Jun 28, 2019
@rjernst rjernst mentioned this pull request Jun 28, 2019
rjernst added a commit that referenced this pull request Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants