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

Add checks for reindex task failures and number of docs created #1291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

msvechla
Copy link

Currently curator simply checks if the destination index was created after a reindex task.
This is not enough for running this in production for us, as we want to delete the source indices after a successful reindex.

Therefore it can be the case, that the reindex task finishes with failures and curator does not throw an exception. The following actions will still get executed and in our case this means the source indices get deleted, even though the reindex task completed with a lot of errors.

Proposed Changes

  • check the reindex task status for failures
  • check if the number of created docs match the total docs

Would be awesome if you could take a look at this. The implementation here is just a first draft.

@untergeek
Copy link
Member

Sorry for the delays. I will hopefully get around to reviewing this further tonight.

@untergeek
Copy link
Member

I will look this all over in a few days. I'm super busy with work right now.

total_docs = response['task']['status']['total']
created_docs = response['task']['status']['created']

if not created_docs == total_docs:
Copy link
Member

Choose a reason for hiding this comment

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

So, devil's advocate here. What happens if someone does a reindex from query instead of a full index? Will these still be equal? What if someone reindexes from multiple indices to 1 index (e.g. 1 week's worth of daily indices into a single weekly index)? Do these numbers match up? If the answer is not "yes," then I can't use the code—at least not as-is.

The draft code works just fine in the event of a 1:1 reindex. I could merge it if the test is only made made when a verify_doc_count flag is present (or something like it). It would, of course, have to be vetted by conditional tests. It wouldn't be valid, for example, if Curator was able to detect a many-to-one reindex (code which would have to be added).

Copy link

Choose a reason for hiding this comment

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

If you reindex going from a mapping file without nested documents to a mapping file with nested documents, the document count will be different. I assume the doc count verification should be optional...

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for providing a concrete example, @rahst12

@untergeek
Copy link
Member

Cross-reference #1299

@untergeek
Copy link
Member

Sorry for the hideous delay on this. I'm barely catching up. Can you rebase this so I can merge it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants