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

Make reindex wait for cleanup before responding #23677

Merged
merged 2 commits into from
Mar 21, 2017

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Mar 21, 2017

Changes reindex and friends to wait until the entire request has
been "cleaned up" before responding. "Clean up" in this context
is clearing the scroll and (for reindex-from-remote) shutting
down the client. Failures to clean up are still only logged, not
returned to the user.

Closes #23653

Changes reindex and friends to wait until the entire request has
been "cleaned up" before responding. "Clean up" in this context
is clearing the scroll and (for reindex-from-remote) shutting
down the client. Failures to clean up are still only logged, not
returned to the user.

Closes elastic#23653
@@ -113,8 +113,9 @@ public void onFailure(Exception e) {
}

@Override
protected void cleanup() {
protected void cleanup(Runnable onCompletion) {
// Nothing to do
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this comment?

@nik9000 nik9000 merged commit bc65be2 into elastic:master Mar 21, 2017
@nik9000
Copy link
Member Author

nik9000 commented Mar 21, 2017

Thanks @tlrx !

nik9000 added a commit that referenced this pull request Mar 21, 2017
Changes reindex and friends to wait until the entire request has
been "cleaned up" before responding. "Clean up" in this context
is clearing the scroll and (for reindex-from-remote) shutting
down the client. Failures to clean up are still only logged, not
returned to the user.

Closes #23653
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 22, 2017
* master:
  [API] change wait_for_completion defaults according to docs (elastic#23672)
  Share XContent rendering code in terms aggs (elastic#23680)
  Update ingest-node.asciidoc
  [DOCS] Update the docs about the fact that global ordinals for _parent field are loaded eagerly instead of lazily by default.
  Build: remove progress logger hack for gradle 2.13 (elastic#23679)
  Test: Add dump of integ test cluster logs on failure (elastic#23688)
  Plugins: Add plugin cli specific exit codes (elastic#23599)
  Plugins: Output better error message when existing plugin is incompatible (elastic#23562)
  Reindex: wait for cleanup before responding (elastic#23677)
  Packaging: Remove classpath ordering hack (elastic#23596)
  Docs: Add note about updating plugins requiring removal and reinstallation (elastic#23597)
  Build: Make plugin list for smoke tester dynamic (elastic#23601)
  [TEST] Propertly cleans up failing restore test
@abeyad
Copy link

abeyad commented Apr 13, 2017

@nik9000 the CancelTests#testDeleteByQueryCancelWithWorkers failed on master, see https://internal-ci.elastic.co/job/elastic+x-pack-elasticsearch+master/452/console. So perhaps the test failure was something else?

@lcawl lcawl added :Distributed Indexing/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 Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v5.4.0 v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants