Skip to content

Conversation

ashwanthgoli
Copy link
Contributor

What this PR does / why we need it:

This PR adds a new engine router middleware that breaks down the request into v1 and v2 query splits.
v1(chunks) requests are additionally split, sharded and cached same as now. v2 request is handed off to the new engine handler (yet to be implemented). This change only applies to range metric queries and log queries.

This change is intended to be a no-op if querier.engine-v2.enable flag is set to false on the query-frontend.

Two now configs are introduced to define the time ranges for which v2 engine handling is applicable.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@ashwanthgoli ashwanthgoli requested a review from a team as a code owner October 9, 2025 14:25
Copy link
Contributor

github-actions bot commented Oct 10, 2025

💻 Deploy preview deleted.

}

func (cfg *Config) ValidQueryRange() (time.Time, time.Time) {
return time.Time(cfg.DataobjAvailableFrom), time.Now().Add(-cfg.DataobjGenerationLagDuration)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these return UTC times?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated. i am guessing this would only help with debugging if we print this somewhere? functionally for time comparisons, this should be a no-op

}

// Merge responses
return e.merger.MergeResponse(responses...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the merger require responses to be sorted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it internally sorts them

sort.Sort(byFirstTime(promResponses))

Comment on lines 181 to 182
// TODO: Add handler for v2 engine.
panic("V2 engine handler not implemented")
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the handler created?

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the handler e.next?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have moved this to newEngineRouterMiddleware arguments, to be linked once we have new scheduler and its handler.

enabling v2 engine on frontend before adding this handler will reuslt in a panic.

Comment on lines 73 to 74
DataobjAvailableFrom dskit_flagext.Time `yaml:"dataobj_available_from" category:"experimental"`
DataobjGenerationLagDuration time.Duration `yaml:"dataobj_generation_lag_duration" category:"experimental"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This conflicts with #19478

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the dataobj configs into V2EngineCfg c988114

@ashwanthgoli ashwanthgoli enabled auto-merge (squash) October 16, 2025 04:28
@ashwanthgoli ashwanthgoli merged commit 72cb649 into main Oct 16, 2025
65 checks passed
@ashwanthgoli ashwanthgoli deleted the qf-v2-engine-splits branch October 16, 2025 04:35
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