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

Graphite relies on data to be sorted alphabetical by name when calling fetch_multi #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vladimir-smirnov-sociomantic
Copy link
Contributor

Especialy needed if graphite will return list series unsorted.

unordered_data = [{'name': node.path, 'points': []} for node in nodes]
logger.debug(caller='fetch_multi', FIXING_DATA_TO=unordered_data)
data = sorted(unordered_data, key=itemgetter('name'))
logger.debug(caller='fetch_multi', len_datapoints_before_fixing=len(ata))
Copy link
Contributor

Choose a reason for hiding this comment

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

typo. ata should be data

@Dieterbe
Copy link
Contributor

could you explain what the bug is again? like what kind of graphite query should i execute and what difference should i see before and after this fix?

also, an issue that i've seen, is that when i draw a graph of let's say 5 targets, and i refresh it, sometimes the order of the targets changes, so their colors change, and when they are stacked on the graph, they are stacked in a different order etc. this is with influx returning sorted series.
will this also be fixed as part of this fix?

@vladimir-smirnov-sociomantic
Copy link
Contributor Author

With my PR to graphite-api, implementing "multi fetch" for multiple targets, result get stored in dict - and you can't guarantee that it'll be same order for all fetches (especialy if you fetch more data and apply some function to it, like sum), but functions in graphite-api assumes that all functions are in same order.
I've got an issue with custom functions we've used (they are just multi-line analoge of asPercent and divideSeries), querie was like:
target=asPercentMulti(host.df__.used, sumSeriesWithWildcards(host.df__.*,3))
and it got incorrect results, like "/ used" was applied to "boot free" and got more then 100% free space. It's very rare case, but ordered dict should solve this problem.

@Dieterbe
Copy link
Contributor

so this is only a bug if brutasse/graphite-api#55 is applied?
in that case the order changing bug i've seen is something else, because I don't have your "multi fetch for multiple targets" patch applied.
Or is this also a problem without your pr?

@vladimir-smirnov-sociomantic
Copy link
Contributor Author

No, it happens without brutasse/graphite-api#55 applied. graphite-api#55 is only about expanding multi_fetch to multiple targets, and this bug is about ordering series in one target (in cases where original multi_fetch works)

This can happen in certain cases without any PRs applied to graphite-influx and graphite-api. Though cases are quite rare - when you have function, that accepts 2 list of series, and one of the arguments is result of another function. In that case first argument may have one order of series, and second argument - another and whole result will be incorrect.

It's quite rare in fact, because inner function must select more data than outer, and reduce it somehow (e.x. sum, avg, etc)

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

Successfully merging this pull request may close these issues.

2 participants