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

Move watcher history to data stream #64252

Merged
merged 54 commits into from
Nov 12, 2020

Conversation

probakowski
Copy link
Contributor

This change moves watcher history to data stream.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Watcher)

@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Nov 6, 2020
@probakowski
Copy link
Contributor Author

I mark this as breaking since we change naming convention of watcher history - it no longer contains date in it, there's single data stream (per version) for all records

@sebelga
Copy link
Contributor

sebelga commented Nov 6, 2020

@probakowski do you mind elaborating with an example of the difference in the response (before, after) so we can better assess if this impacts our UI? cheers!

Copy link
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

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

Thanks, @probakowski. Looks pretty good to me. I left one minor comment to address and one minor question.

Comment on lines +84 to +86
public void flush() {
bulkProcessor.flush();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, is this addressing a scenario that's specific to data streams or just a general case where history entries weren't being written?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Main reason for this is to have clear state after tests. This makes sure that we don't have any pending docs to index so we can do proper cleanup after the test and leave no hanging indices behind

@probakowski
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-sample-windows

@probakowski
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

@probakowski probakowski merged commit 3e426c1 into elastic:master Nov 12, 2020
@probakowski probakowski deleted the watcher-history-data-stream2 branch November 12, 2020 21:41
@probakowski
Copy link
Contributor Author

@sebelga we've changed naming convention here, previously index name contained date it was created (so record created today would go to index .watcher-history-13-2020-11-12) but now everything goes to data stream called .watcher-history-14 (template version bump). So, as long as you use .watcher-history-* for search there's no difference but if your workflow queries specific index (like 3rd party software or watcher with input pointing to name with date math instead of a wildcard) it will break

@sebelga
Copy link
Contributor

sebelga commented Nov 16, 2020

@probakowski was this message for me or @sebgl ?

@cjcenizal
Copy link
Contributor

Tested Watcher UI with these changes and it seems to work as expected. I looked at the code and we're retrieving Watcher history with a search API request aimed at .watcher-history-*, using scroll, sort, size, and query` parameters, all of which I believe are still supported. Thanks for the notes, @probakowski!

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

Successfully merging this pull request may close these issues.

6 participants