-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
[Monitoring] Remove _type usages in _search requests #38819
[Monitoring] Remove _type usages in _search requests #38819
Conversation
Pinging @elastic/es-core-features |
"should": [ | ||
{ | ||
"term": { | ||
"_type": "cluster_state" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filter was being used for legacy documents to ease migration through 6.x. However, we should not see such documents in 6.7 onwards. So it's safe to remove this filter in 7.0 onwards IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To check I understand, all documents will have an explicit type
field starting in 6.7, so we don't need this extra 'or' clause that matches on document type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. In fact, documents in .monitoring-*
should've had an explicit type field much before 6.7
but I'm not sure exactly when that was introduced (@pickypg?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any document created in 5.5+ should have _type
of doc
and, cluster_state
in particular will no longer exist (even as a type
since cluster_state
and cluster_stats
were merged together).
So, of interest to the Watch, the source cluster creates the watch, therefore it would appropriately handles its own data. As a result, this is safe to change, but it means that if someone just upgraded from 5.4 to 5.6 to 6.7, then it would effectively ignore the pre-5.5 data once they got to 6.7 with that type dropped. Given its purpose, that's safe.
This looks good to me, but I'll let the others have the final sign-off as I'm not familiar with the history here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* master: Remove _type term filters from cluster alert watches (elastic#38819) Adjust log and unmute testFailOverOnFollower (elastic#38762) Fix line separators in JSON logging tests (elastic#38771) Fix synchronization in LocalCheckpointTracker#contains (elastic#38755) muted test Remove TLSv1.2 pinning in ssl reload tests (elastic#38651) Don't fail init script if `/proc/.../max_map_count` absent (elastic#35933) Format Watcher.status.lastChecked and lastMetCondition (elastic#38626) SQL: Implement `::` cast operator (elastic#38774) Ignore failing test
This PR removes usages of the
_type
field in_search
requests issued from Monitoring code.Addresses #37442.