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

Reindex cancel and status don't work via transport client #19979

Closed
javanna opened this issue Aug 12, 2016 · 6 comments
Closed

Reindex cancel and status don't work via transport client #19979

javanna opened this issue Aug 12, 2016 · 6 comments
Assignees
Labels
>bug :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v5.0.0-beta1

Comments

@javanna
Copy link
Member

javanna commented Aug 12, 2016

We recently moved the registration of named writeables to pull approach. The task status named writeables were left out of this refactoring and should be migrated as well. As a result, transport client doesn't see the BulkByScrollTask.Status as its registration happens in NetworkModule after the transport client pulled all the named writeables during its initialization.

I guess we want to create a TaskPlugin interface like we have done with SearchPlugin, ActionPlugin etc.

@javanna
Copy link
Member Author

javanna commented Aug 12, 2016

This can be tested by removing the @ClusterScope annotation from ReindexTestCase, which currently disables transport client in all of the reindex integration tests. One of the problems that we would have caught using transport client is #19977, another one is this issue. Once we have resolved this we can enable the transport client in those tests.

@nik9000
Copy link
Member

nik9000 commented Aug 12, 2016

@ClusterScope

So I added that annotation because it was on the test case from that I cribbed the module test from. I can't remember which one it was, but I expect we use this annotation all over the place for dubious reasons. Maybe we should remove go through the uses and prune them?

@nik9000
Copy link
Member

nik9000 commented Aug 12, 2016

TaskPlugin

I'd put the method in ActionPlugin because tasks are always paired with some action. There is also a registerTaskStatus which maybe can move in there too....

@javanna
Copy link
Member Author

javanna commented Aug 12, 2016

I can't remember which one it was, but I expect we use this annotation all over the place for dubious reasons. Maybe we should remove go through the uses and prune them?

I had a quick look, and I don't see that many usages, most seem legit, but sure we should probably go over the tests that use it and re-evaluate.

@nik9000
Copy link
Member

nik9000 commented Aug 13, 2016

I can fix this on Monday if no one else claims it first.

@nik9000 nik9000 self-assigned this Aug 15, 2016
@nik9000
Copy link
Member

nik9000 commented Aug 15, 2016

Claimed. I'll work on it this morning.

@lcawl lcawl added :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. and removed :Reindex API labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v5.0.0-beta1
Projects
None yet
Development

No branches or pull requests

4 participants