Closed Karsonito closed 2 years ago
There is a commit in a separate branch (based on current master which is code-wise same as 0.15.5) that makes that feature optional (enabled by default, see carbonapi.example.yaml for how to disable it).
It would be great if you can test if that improves performance.
Anyway that is something that I need to properly improve later on. I'll try to make the test setup you've provided if not per-commit, but at least per release and publish some comparisons.
If it doesn't help, can you try to do git bisect to determine the commit that caused the problem?
I found the root of a problem: https://github.com/go-graphite/carbonapi/commit/9199a582f3b17aa9ba4bf3b53ff3b1893cc33e26 It's a little different sort after fetch. After reverting this commit cpu usage dramatically decreased
I don't really know how important it is to sort the data before aggregation. May be @Felixoid can helps explain his change in v0.14: "[Fix] Sorting metrics should work now in the same way as in graphite-web (thx to @Felixoid)"
Basically, as it described in https://t.me/ru_go_graphite/5076, without sorting some results were complete trash. If functions require data pre-ordered, it should be in the correct order in advance.
If functions require data pre-order, it should be in the correct order in advance.
Is it possible to find all such functions and sort only before applying them?
In our case it was as well a question of determined results for *
targets. Without sorting each request gave us metrics in a random order, and graphs been a bit derpy. For example, CPU metrics (idle, sys, user etc) each time were in different colors in grafana.
So, what basically the changes have done, is implemented the same behavior as in graphite-web.
I think having a switch that disables that behavior might be still useful.
And later on we can try to get a list of such functions and do pre-sorting on a per-function level
Without sorting each request gave us metrics in a random order, and graphs been a bit derpy. For example, CPU metrics (idle, sys, user etc) each time were in different colors in grafana.
I understand the goal to follow graphite-web's behavior But it can be solved with sortByName We shouldn't mix correctness and visualize, IMHO. Especially at the cost of performance
But it can be solved with sortByName
It would be required for literally every target in myriads of dashboards.
Maybe, something can be improved in https://github.com/go-graphite/carbonapi/blob/main/expr/helper/sort.go#L50-L57? Or simply replacing ByNameNatural
with ByName
is a way to quick but dirty win.
It would be required for literally every target in myriads of dashboards.
What about adding switchable option? I don't see any problem to add sortByName manually if it needed for dashboard
I think having a switch that disables that behavior might be still useful.
Yep, this would work too, for sure. What I am just trying to say is, the behavior can be improved. For example, I've found two libraries with nat-sorting and compared them.
The results are in https://github.com/maruel/natural/pull/2
We can use the natsort from https://github.com/maruel/natural/blob/5d883ec/natsort.go#L18
Could you please check that after #673 was merged, performance became better?
Nice improvement! Even with implicit sorting last version became as quick as 0.12.6
Good, than it's done for sure
Hi! Do you plan to build new release with this improvement??
Problem description Implicit sorting of rows for all functions leads to slow requests and high CPU usage For example query duration slows in 1.5 times and cpu usage grows in 4 times
carbonapi's version 0.15.4 (problem started in 0.14.0)
carbonapi's config
backend software and config
carbonapi performance metrics
Query that causes problems
target=aggregate(seriesByTag('hostname=~host_*'),'count')&from=1653350400&until=1653436800&format=json&maxDataPoints=1488
Data was generated in such way:
Additional context We discussed with @Civil in telegram about improvements