Add a RFC to optimize common trace DB queries#24
Conversation
Co-authored-by: Ayush Khatri <khatria@amazon.com> Signed-off-by: mprahl <mprahl@users.noreply.github.com>
| ON assessments (experiment_id, trace_timestamp_ms); | ||
|
|
||
| CREATE INDEX idx_assessments_exp_trace_ts_name | ||
| ON assessments (experiment_id, trace_timestamp_ms, name); |
There was a problem hiding this comment.
aren't the two indexes above redundant? I believe in postgres and SQLite, the second index can be used for any query the first would.
There was a problem hiding this comment.
I think that it can be reused but it's not as efficient. I added a comment explaining it.
|
|
||
| These numbers are benchmark-based targets, not guarantees. Exact percentile queries may still | ||
| retain significant sort cost even after the denormalization work, and are expected to remain the | ||
| main residual cost on the slowest paths. |
There was a problem hiding this comment.
I think it would be good to also benchmark writes
etirelli
left a comment
etirelli
left a comment
There was a problem hiding this comment.
LGTM, just a couple of minor comments.
Signed-off-by: mprahl <mprahl@users.noreply.github.com>
|
@etirelli could you please take another look? I added a commit with a proposal for rollup tables. |
| - adding targeted indexes for assessment and span-cost workloads | ||
| - cleaning up duplicated EAV rows once the denormalized columns become authoritative | ||
|
|
||
| On a load test with 10M traces, 30M spans, and 30M assessments in one experiment, the current Usage |
There was a problem hiding this comment.
hey, is this load generator available somewhere?
There was a problem hiding this comment.
There are some load generators available in the MLflow repo:
.github/workflows/tracing-benchmark.ymldev/benchmarks/tracing/README.mddev/benchmarks/tracing/test_trace_perf.pydev/benchmarks/tracing/_data.py
That said, the exact 10M-trace Postgres UI replay harness used for these RFC numbers was a local/ad hoc load harness derived from the tracing benchmark utilities to speed up trace ingestion.
Signed-off-by: mprahl <mprahl@users.noreply.github.com>
| similar non-analytic annotations can remain mutable. Adding assessments to older traces should also | ||
| remain allowed, since human review and evaluator backfills often happen after the trace is closed. | ||
|
|
||
| Assessment writes or trace deletes against already-rolled-up traces should delete stale rollup rows |
There was a problem hiding this comment.
should we mark the row as stale instead of deleting? considering queries to the metrics issued after an update to a non-fresh trace, if the row was deleted, the data point for that day would be missing completely, while if we simply mark it as stale, the previous result would be returned/computed (eventual consistency).
There was a problem hiding this comment.
| rollup-supported shape, especially arbitrary filtered requests and non-daily exact percentile | ||
| queries, still fall back to the raw path and remain the main residual cost. | ||
|
|
||
| ## Drawbacks |
There was a problem hiding this comment.
NP: "trade-offs" instead of "drawbacks"? :)
|
@mprahl LGTM The query numbers look great! Did you benchmark writing impact on a loaded DB (i.e., writing performance after pre-loading the DB with the test data)? I don't believe the impact would be significant, but still a good datapoint to have. |
Thanks for the review! I haven't tested it but I'll try to give that a shot today. |
3 participants
Supersedes #23
This was coauthored by @aws-khatria.