Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

Add support for sortByName #827

Merged
merged 7 commits into from
Jan 23, 2018
Merged

Conversation

shanson7
Copy link
Collaborator

Also removes the automatic sorting of output by series target in plan.go.

@Dieterbe
Copy link
Contributor

Dieterbe commented Jan 22, 2018

Also removes the automatic sorting of output by series target in plan.go.

hmm that was specifically added because that's how graphite did it. was that a mistake or has graphite changed since then? ( 0d67756 )

@shanson7
Copy link
Collaborator Author

Hmm, maybe sorting results gets disabled if a sortBy* is used?

@shanson7
Copy link
Collaborator Author

Seems like an unrelated test failed. I have seen this test fail on me a couple of times, transiently.

@shanson7
Copy link
Collaborator Author

@DanCech - Can you comment on the sorting of graphite-web? I'm trying to look through the code, but I'm not sure where that's handled

@DanCech
Copy link
Contributor

DanCech commented Jan 22, 2018

https://github.com/graphite-project/graphite-web/blob/master/webapp/graphite/render/datalib.py#L237

The series that come back from any data query are sorted by name here, running them through sortBy* alters the order from there.

@shanson7
Copy link
Collaborator Author

Ah, so they are ordered before functions are run on them?

@DanCech
Copy link
Contributor

DanCech commented Jan 22, 2018

Yessir

@shanson7
Copy link
Collaborator Author

And now metrictank matches graphite in it's presort.

)

func shuffleSeries(slice []string) {
for i := len(slice) - 1; i > 0; i-- {
Copy link
Contributor

Choose a reason for hiding this comment

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

why count down instead of the more commonly used count up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the most common version of the Fisher-Yates algorithm. The alternative is from 0 to len-2. I don't really care which one to use.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok then, sounds good

@Dieterbe
Copy link
Contributor

thanks for another nice contribution @shanson7 ! 💯

@Dieterbe Dieterbe merged commit 4d64ca0 into grafana:master Jan 23, 2018
@shanson7 shanson7 deleted the feature_sortByName branch January 23, 2018 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants