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

Always delegate TransportIndexAction and TransportDeleteAction to bulk action #40424

Merged
merged 1 commit into from
Mar 27, 2019

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Mar 25, 2019

The implementation of TransportIndexAction and TransportDeleteAction as TransportReplicationAction existed for interoperability with older 5.x nodes, as these older nodes coordinated single index / deletes as replication requests. This BWC layer is no longer needed in 7.x, where these single actions are now mapped to bulk requests. Completely removing the deprecated transport actions is not possible yet if we want to keep BWC with a 6.x transport client. The best way here is to wait for the transport client to go away and then just remove the actions.

@ywelsch ywelsch added >non-issue :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v8.0.0 v7.2.0 labels Mar 25, 2019
@ywelsch ywelsch requested a review from bleskes March 25, 2019 19:26
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for picking this up.

@@ -166,6 +166,7 @@ protected void registerRequestHandlers(String actionName, TransportService trans

@Override
protected void doExecute(Task task, Request request, ActionListener<Response> listener) {
assert request.shardId() != null : "request shardId must be set";
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting - did you run into a situation when this wasn't the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, but this work got triggered by looking at open Github issues, in particular #20279 . In particular I noticed that the desired property to have a complete ShardId when replication requests get to the reroute phase already holds today.
This PR does not close that issue out (needs further investigation on what exactly needs to be done), but addresses the first point on it.

@ywelsch
Copy link
Contributor Author

ywelsch commented Mar 26, 2019

@elasticmachine run elasticsearch-ci/packaging-sample

@ywelsch ywelsch merged commit 78636e3 into elastic:master Mar 27, 2019
ywelsch added a commit that referenced this pull request Mar 27, 2019
The implementation of TransportIndexAction and TransportDeleteAction as
TransportReplicationAction existed for interoperability with older 5.x nodes, as these older nodes
coordinated single index / deletes as replication requests. This BWC layer is no longer needed in 7.x,
where these single actions are now mapped to bulk requests. Completely removing the deprecated
transport actions is not possible yet if we want to keep BWC with a 6.x transport client. The best
way here is to wait for the transport client to go away and then just remove the actions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >non-issue v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants