Skip to content

Skip reading engagements when not necessary#6137

Closed
ukutaht wants to merge 2 commits intomasterfrom
optimize-engagements-reading
Closed

Skip reading engagements when not necessary#6137
ukutaht wants to merge 2 commits intomasterfrom
optimize-engagements-reading

Conversation

@ukutaht
Copy link
Contributor

@ukutaht ukutaht commented Mar 7, 2026

Adds WHERE name != 'engagement' all events_v2 queries that don't need engagement data. That is queries without time_on_page or scroll_depth metrics, or scroll goals included in dimensions/filters.

Since the name field is included in the events_v2 sort key, engagement events cluster together in storage, allowing ClickHouse to efficiently skip granules that contain only engagement rows when they are not needed.

I don't like that the most natural place for this was in the where_builder.ex module. But I didn't find a much better place for this either. Naturally I'd like it to be in the QueryOptimizer given the name of that module. But it only operates on the Query struct and I didn't want to add another field to that struct.

IMO the query pipeline is overdue for a refactoring where the Query struct is stripped down to represent user intent and site context. And all hints about SQL generation, which tables to query, JOIN types, and optimizations like this would be added to an intermediate ExecutionPlan struct. But this is a separate topic from the optimization itself.

@ukutaht ukutaht added the preview label Mar 7, 2026
@github-actions
Copy link

github-actions bot commented Mar 7, 2026

Preview environment👷🏼‍♀️🏗️
PR-6137

@ukutaht
Copy link
Contributor Author

ukutaht commented Mar 7, 2026

Trying to check if it has some effect on queries on preview env but getting "DB::Exception: Received from localhost:9000. DB::Exception: Too many simultaneous queries. Maximum: 150. (TOO_MANY_SIMULTANEOUS_QUERIES) (version 25.11.5.8 (official build))"

@cnkk do we have too many preview environments at the same time?

@ukutaht ukutaht removed the preview label Mar 9, 2026
@ukutaht ukutaht mentioned this pull request Mar 9, 2026
@ukutaht ukutaht closed this Mar 9, 2026
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.

1 participant