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

fix: make tagKeys and tagValues work for edge cases involving _field #19856

Merged
merged 6 commits into from
Oct 30, 2020

Conversation

wolffcm
Copy link

@wolffcm wolffcm commented Oct 29, 2020

Fixes #19806
Fixes #19794

This updates the tagKeys and tagValues methods of the Store interface to return the answers that Flux is used to, by properly filtering on _field when such a predicate is present. These methods are frequently used by the UI query builder to describe the shape of the users data.

@wolffcm
Copy link
Author

wolffcm commented Oct 29, 2020

I had to re-skip the Flux test show_tag_keys because the current version of Flux has a bug in the test (type of a field changes for different tag values, which tsm1 apparently does not like).

The tip of Flux master has more tests (all of which pass against this branch), and also fixes this issue with the test. I made a note for myself to unskip this test after Flux has upgraded on Monday.

Copy link
Contributor

@stuartcarnie stuartcarnie left a comment

Choose a reason for hiding this comment

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

Looks great to me, @wolffcm – I had one suggestion to improve a struct iteration, as I've seen these loops show up in profiles in the past. Happy to skip the change in this PR and we can address it in a subsequent update if it shows up in profiles.

Comment on lines +585 to +590
// tagValuesSlow will determine the tag values for the given tagKey.
// It's generally faster to use tagValues, measurementFields or
// MeasurementNames, but those methods will only use the index and metadata
// stored in the shard. Because fields are not themselves indexed, we have no way
// of correlating fields to tag values, so we sometimes need to consult tsm to
// provide an accurate answer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Appreciate the detailed comment 👍

v1/services/storage/store.go Outdated Show resolved Hide resolved
Copy link
Contributor

@nathanielc nathanielc left a comment

Choose a reason for hiding this comment

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

🎉 This is great!

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