Sample Snowflake query history to bound extract size (#2532)#2543
Sample Snowflake query history to bound extract size (#2532)#2543jneil17 wants to merge 2 commits into
Conversation
The full 90-day QUERY_HISTORY pull (incl. QUERY_TEXT, avg ~500 chars, up to ~1M) produced multi-GB extracts on high-volume accounts that exceeded export limits. Replace it with a bounded set of extracts: - query_history: random sample (<=10k rows) with QUERY_TEXT for the skill to classify the workload mix. - query_history_top: top 20 queries by credits (with text) so outliers the sample misses are still captured. - query_history_stats: full-window aggregates (no text) by query type and warehouse size, carrying the true totals the sampled mix is applied against. - user_activity: per-user summary of LOGIN_HISTORY instead of a per-event dump. Per-query credits use QUERY_ATTRIBUTION_HISTORY, falling back to an execution-time x warehouse-size-rate estimate (IS_CREDITS_ESTIMATED). Workload classification stays in the downstream skill; the profiler only delivers totals and a representative sample. Co-authored-by: Isaac
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2543 +/- ##
=======================================
Coverage 69.18% 69.18%
=======================================
Files 105 105
Lines 9503 9503
Branches 1052 1052
=======================================
Hits 6575 6575
Misses 2731 2731
Partials 197 197 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
✅ 169/169 passed, 8 flaky, 2 skipped, 45m22s total Flaky tests:
Running from acceptance #4962 |
Drop the user_activity step entirely rather than summarizing it. The login history it captured isn't used in the TCO analysis, so removing it trims the extract further and keeps the profiler focused on cost-relevant data. Co-authored-by: Isaac
dgomez04
left a comment
dgomez04
left a comment
There was a problem hiding this comment.
The core issue is fixed but grows execution time significantly. Let's look into this deeper before approving.
| -- Extract query execution patterns, performance metrics, and user activity | ||
| -- Query History — bounded RANDOM sample for workload classification | ||
| -- | ||
| -- Issue #2532: the previous step pulled the full 90-day QUERY_HISTORY including |
There was a problem hiding this comment.
We don't need to mention the issue within the comment, we should make it flatter and straight to the point.
| frequency: daily | ||
| flag: active | ||
|
|
||
| # Bounded RANDOM sample of query history (incl. QUERY_TEXT) for workload |
| flag: active | ||
|
|
||
| - name: storage_usage | ||
| # Top-N most expensive queries (with QUERY_TEXT) so the random sample never |
| flag: active | ||
|
|
||
| - name: user_activity | ||
| # Full-window aggregate (no query text): true totals + per-group distribution |
There was a problem hiding this comment.
The pull request description mentions that user_activity still exists but here it's completely removed. Can we confirm whether this is the intended behavior?
There was a problem hiding this comment.
Each of the new pipeline steps scans the full 90-day QUERY_HISTORY window independently. Export size is fixed, but the total execution time can grow by ~3x compared to the old, single-step run.
I believe this is something we should look into further.
| FROM SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY qh | ||
| LEFT JOIN attributed a ON qh.QUERY_ID = a.QUERY_ID | ||
| WHERE qh.START_TIME >= DATEADD('day', -90, CURRENT_TIMESTAMP()) | ||
| ORDER BY RANDOM() |
There was a problem hiding this comment.
ORDER BY RANDOM() LIMIT 10000 may still require Snowflake to touch every row before applying the limit.
Has execution time been benchmarked?
Did we consider other alternatives such as SAMPLE (10000 ROWS) or hash-based sampling like (MOD(ABS(HASH(QUERY_ID)), …))? They could be much cheaper while still giving a representative mix.
Additionally, is there any reason for us to get 90 days of history rather than 30?
|
Closing off as I'll be reworking based on some new insights. |
3 participants
Summary
query_historyproduced a multi-GB extract driven byQUERY_TEXT(avg ~500 chars, up to ~1M). This pull request replaces it with a random sample that always leads to a flat, predictable size.Changes
query_history— now a random sample (ORDER BY RANDOM() LIMIT 10000, 90-day window) withQUERY_TEXT, for the downstream skill to classify the workload mix.query_history_top(new) — top 20 queries by credits (withQUERY_TEXT) so outliers the sample misses are still captured.query_history_stats(new) — full-window aggregates (no query text) byQUERY_TYPE × WAREHOUSE_SIZE: counts + total/avg/median/p90 credits & elapsed. Carries the true totals the sampled mix is applied against.user_activity— per-user/client summary ofLOGIN_HISTORYinstead of a per-event dump (secondary bloat noted in the issue).Notes
query_history_statscredits are attributed/estimated and won't perfectly reconcile withwarehouse_usage(WAREHOUSE_METERING_HISTORY), which remains the billing source of truth.Closes #2532
This pull request and its description were written by Isaac.