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

Use query insead of query_range when we do not need full data. #2736

Conversation

yaacov
Copy link
Member

@yaacov yaacov commented Nov 15, 2017

Compute > Containers > Providers - click a provider - toolbar Monitoring > Ad Hoc Metrics

Description

The ad hoc page use a the same query from Prometheus to get data and to get only labels.
We query labels for all the metrics in the system, this query may take a lot of time (e.g. more then 10s) on large system scales.

This PR changes the query we use when we only want labels. Now we use the less stressful query instead of query_range, when we do not need to get data.

More info

On my small system this reduced query time from 900ms to 600ms, on bigger systems this can change from several s' to the ms' range.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1511402

Screenshots
Before, query takes ~900ms on my very small system:
screenshot from 2017-11-15 09-38-38

After, query takes ~600ms on my very small system:
screenshot from 2017-11-15 09-36-28_01

@yaacov
Copy link
Member Author

yaacov commented Nov 15, 2017

@miq-bot add_label bug, compute/containers, gaprindashvili/yes

@simon3z @moolitayer @zeari @himdel @martinpovolny please review

https://bugzilla.redhat.com/show_bug.cgi?id=1511402 target: 5.9.0

@yaacov
Copy link
Member Author

yaacov commented Nov 15, 2017

@joelddiaz @ilackarms , this may have impact on the OPS effort, PTAL

@yaacov yaacov changed the title Use query insead of query_range when we do not need the data in prome… Use query insead of query_range when we do not need full data. Nov 15, 2017
@yaacov yaacov force-pushed the use-query-instead-of-query_range-to-get-metrics-tags branch from fcd6234 to 510fc86 Compare November 15, 2017 10:21
@miq-bot
Copy link
Member

miq-bot commented Nov 15, 2017

Checked commit yaacov@510fc86 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@joelddiaz
Copy link

👍 this should definitely help with the timeouts we were seeing last week

@yaacov
Copy link
Member Author

yaacov commented Nov 16, 2017

@ilackarms can you make an image with this, so @joelddiaz can test it in the wild and compare with an image without this fix ?

@himdel himdel self-assigned this Nov 16, 2017
@himdel
Copy link
Contributor

himdel commented Nov 16, 2017

Code looks good, has anyone tested this?

(EngLab-prometheus gives me execution expired now)

@yaacov
Copy link
Member Author

yaacov commented Nov 16, 2017

Checking EngLab-prometheus ... a moment :-)

@yaacov
Copy link
Member Author

yaacov commented Nov 16, 2017

@himdel sorry about that, machines are up now.

@himdel
Copy link
Contributor

himdel commented Nov 16, 2017

Perfect, thanks! :)

LGTM, I'm getting the same set of tags :).

@himdel himdel merged commit 6b6a87e into ManageIQ:master Nov 16, 2017
@himdel himdel added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 16, 2017
@himdel
Copy link
Contributor

himdel commented Nov 16, 2017

@yaacov Just a note here though... on my dev setup, this makes the time go from about 5.89 seconds on master to about 6.12s with this PR.

(But not sure if this is statistically significant or anything, just not seeing that huge performance gain ;).)

@yaacov
Copy link
Member Author

yaacov commented Nov 16, 2017

@himdel I have a small setup, on @joelddiaz machine this made a big improvement, but we only tested this query vs query_range, once on a "real" machine.

simaishi pushed a commit that referenced this pull request Nov 17, 2017
…-to-get-metrics-tags

Use query insead of query_range when we do not need full data.
(cherry picked from commit 6b6a87e)

https://bugzilla.redhat.com/show_bug.cgi?id=1514523
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 9f48355bb427410b4a1110c61f8685bd87f4e34b
Author: Martin Hradil <himdel@seznam.cz>
Date:   Thu Nov 16 13:24:15 2017 +0000

    Merge pull request #2736 from yaacov/use-query-instead-of-query_range-to-get-metrics-tags
    
    Use query insead of query_range when we do not need full data.
    (cherry picked from commit 6b6a87e522f4c2bb348e2f8fbd46e6e9984252e3)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1514523

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.

5 participants