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

Time check in query priority now considers overall data select time window (including range selectors, modifiers and lookback delta) #5758

Merged
merged 14 commits into from
Feb 6, 2024

Conversation

justinjung04
Copy link
Contributor

@justinjung04 justinjung04 commented Jan 30, 2024

What this PR does:

Improves time bound check for query priority, such that the range of subquery is also evaluated.
Also, added data_fetch_min_time and data_fetch_max_time in query stats to help with query priority configuration.

For example,

ts=2024-02-05T22:08:44.07399Z caller=handler.go:372 level=info org_id=fake msg="query stats" component=query-frontend method=POST path=/prometheus/api/v1/query_range response_time=3.655097ms query_wall_time_seconds=0.002303539 fetched_series_count=1 fetched_chunks_count=1 fetched_samples_count=6 fetched_chunks_bytes=38 fetched_data_bytes=212 split_queries=1 status_code=200 response_size=357 data_select_max_time=1707170910 data_select_min_time=1707163710 query_length=2 user_agent=Grafana/9.4.3 priority=2 param_start=1707167310 param_step=15 param_end=1707170910 param_query=up

Which issue(s) this PR fixes:
n/a

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@justinjung04 justinjung04 changed the title Improve time bound check for query priority Time range of subqueries are now considered when determining query priority Jan 30, 2024
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jan 31, 2024
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
@justinjung04 justinjung04 marked this pull request as ready for review January 31, 2024 20:45
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/frontend/transport/handler.go Show resolved Hide resolved
@justinjung04 justinjung04 changed the title Time range of subqueries are now considered when determining query priority Time check in query priority now considers offsets, @ modifiers and range selectors. Feb 1, 2024
@justinjung04 justinjung04 changed the title Time check in query priority now considers offsets, @ modifiers and range selectors. Time check in query priority now considers range selectors, modifiers and lookback delta. Feb 1, 2024
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/querier/tripperware/priority.go Outdated Show resolved Hide resolved
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
@justinjung04 justinjung04 changed the title Time check in query priority now considers range selectors, modifiers and lookback delta. Time check in query priority now considers overall data fetched time window (including range selectors, modifiers and lookback delta) Feb 2, 2024
Signed-off-by: Justin Jung <jungjust@amazon.com>
@justinjung04 justinjung04 changed the title Time check in query priority now considers overall data fetched time window (including range selectors, modifiers and lookback delta) Time check in query priority now considers overall data select time window (including range selectors, modifiers and lookback delta) Feb 5, 2024
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
Signed-off-by: Justin Jung <jungjust@amazon.com>
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Great work @justinjung04! Thanks

@yeya24 yeya24 merged commit e8427a5 into cortexproject:master Feb 6, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants