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

[Monitoring] Remove _type usages in _search requests #38819

Merged
merged 1 commit into from
Feb 13, 2019

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Feb 12, 2019

This PR removes usages of the _type field in _search requests issued from Monitoring code.

Addresses #37442.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

"should": [
{
"term": {
"_type": "cluster_state"
Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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?).

Copy link
Member

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.

@jtibshirani
Copy link
Contributor

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.

Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM

@ycombinator ycombinator merged commit d09ed17 into elastic:master Feb 13, 2019
@ycombinator ycombinator deleted the monitoring-remove-type branch February 13, 2019 00:21
ycombinator added a commit to ycombinator/elasticsearch that referenced this pull request Feb 13, 2019
ycombinator added a commit to ycombinator/elasticsearch that referenced this pull request Feb 13, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 13, 2019
* 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
ycombinator added a commit that referenced this pull request Feb 13, 2019
Backport of #38819. Original message:

This PR removes usages of the `_type` field in `_search` requests issued from Monitoring code.
ycombinator added a commit that referenced this pull request Feb 13, 2019
Backport of #38819. Original message:

This PR removes usages of the `_type` field in `_search` requests issued from Monitoring code.
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.

7 participants