Skip to content

RFC: PostgreSQL trace query optimization#23

Closed
aws-khatria wants to merge 2 commits into
mlflow:mainfrom
aws-khatria:postgres-optimization
Closed

RFC: PostgreSQL trace query optimization#23
aws-khatria wants to merge 2 commits into
mlflow:mainfrom
aws-khatria:postgres-optimization

Conversation

@aws-khatria

@aws-khatria aws-khatria commented Jun 26, 2026

Copy link
Copy Markdown

Proposes schema-level optimizations to improve MLflow trace analytics performance on PostgreSQL — based on a load test with 10M traces, 30M spans, and 30M assessments in a single experiment.

Foreign key constraints that can be dropped to improve write throughput. The performance improvement load testing is still pending but this can prove to be beneficial for logging high volume metrics in an experiment.

Happy for suggestions to further improve the MLflow dashboarding experience.

Proposes composite indexes, token metric denormalization, and
experiment_id addition to assessments table to improve Usage
page and Traces page load times at scale.

The root causes are:

1. **Missing indexes** — no composite index on `(experiment_id, timestamp_ms)` on the `trace_info` table forces full table scans for every time-bounded query

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps they meant covering index? The composite index definitely exists so this is incorrect as written.

-- from the index without going back to the table for each row. Since
-- assessment values are short (numbers, booleans, short strings), the index
-- size increase could be modest.
CREATE INDEX CONCURRENTLY idx_assessments_traceid_created_ts

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the assessments part of this section could be strengthened by avoiding INCLUDE (value) on the covering index and instead introducing precomputed aggregate columns on assessments.

Example schema change:

ALTER TABLE assessments
    ADD COLUMN aggregate_value DOUBLE PRECISION,
    ADD COLUMN aggregate_value_kind SMALLINT;

Where aggregate_value_kind records how aggregate_value was derived, e.g.:

  • 1 = numeric
  • 2 = boolean
  • 3 = yes/no string
  • NULL = not aggregateable

Then the numeric/percentile path can operate on aggregate_value directly instead of repeatedly applying CASE / CAST against value. Some of the advantages of doing so:

  • faster AVG / Pxx queries because the numeric representation is materialized at write time
  • no per-row CASE / CAST work in the hot query path
  • avoids carrying value in a covering index since value can contain arbitrary JSON payloads / non-numeric content
  • preserves provenance via aggregate_value_kind, so 1.0 from a boolean-like assessment is distinguishable from a true numeric score

Concretely, for the index in this section, perhaps something like this?

CREATE INDEX CONCURRENTLY idx_assessments_traceid_created_ts
    ON assessments (trace_id, created_timestamp)
    INCLUDE (name, aggregate_value, aggregate_value_kind);

And, if this RFC later adopts denormalized experiment_id on assessments, also a partial index for the numeric aggregate path:

CREATE INDEX CONCURRENTLY idx_assessments_exp_ts_name_agg
    ON assessments (experiment_id, last_updated_timestamp, name)
    INCLUDE (aggregate_value, aggregate_value_kind)
    WHERE aggregate_value IS NOT NULL;

I would also add API-side validation when creating a new assessment so we do not mix value types under the same assessment name within the same experiment. Today the aggregate query effectively assumes that a given assessment name has a stable value type/semantics, but that is not enforced. aggregate_value_kind gives us a clean way to validate that invariant and reject writes that would make AVG / Pxx ambiguous or misleading.

What do you think?


The current query filters on `trace_info.timestamp_ms` (when the trace *ran*).

The new query filters on `assessments.last_updated_timestamp` (when the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we consider storing the trace timestamp (already normalized for the day perhaps) along with the experiment ID so the filter is accurate?

frequently-queried metrics:

```sql
ALTER TABLE trace_info

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is also missing cache_read_input_tokens and cache_creation_input_tokens. Is the intention to drop these from the dashboard query or should we include them on the table?

Every dashboard query groups traces by day — "how many traces ran on Monday vs
Tuesday vs Wednesday." To do that, the query takes each trace's millisecond
timestamp and rounds it down to midnight of that day using
`floor(timestamp_ms / 86400000) * 86400000` (86,400,000 ms = 1 day).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we also add server-side query-shape caps for the trace metrics endpoint?

I ask because max_results only limits the number of grouped rows returned; it does not limit how much data Postgres may scan, join, sort, cast, or aggregate before the LIMIT is applied. Today the UI has a client-side guard that only offers bucket intervals producing at most 1000 chart points, but direct API callers can bypass that and send arbitrary time_interval_seconds values, long time windows, exact percentile aggregations, or query shapes that are much more expensive than the dashboard path. This seems like it could allow unbounded expensive queries against the MLflow / Postgres server even if the RFC makes the common dashboard queries much faster.

Would it make sense to define some server-side request-shape limits as part of this work, for example:

  • require time_interval_seconds > 0 and start_time_ms < end_time_ms
  • require a bounded time range whenever bucketing is requested
  • allow only a fixed set of supported bucket intervals instead of arbitrary values so query cost is predictable
  • cap computed bucket count, e.g. ceil((end_time_ms - start_time_ms) / (time_interval_seconds * 1000)) <= 1000
  • apply tighter time-window caps for more expensive query shapes
  • apply tighter limits or separate handling for exact percentile queries, since percentile_cont can require sorting large groups
  • consider constraining very expensive filter/dimension combinations for large windows unless they are explicitly optimized

| `logged_model_metrics` | `SqlLoggedModelMetric.experiment_id` |
| `logged_model_params` | `SqlLoggedModelParam.experiment_id` |
| `logged_model_tags` | `SqlLoggedModelTag.experiment_id` |
| `spans` | `SqlSpan.experiment_id` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm supportive of removing this since span insertion is very high volume. The others, I'm not as convinced about. Are you seeing performance issues from those other tables?


## Open questions

1. **Which additional metrics should be denormalized?** The RFC proposes three (`input_tokens`, `output_tokens`, `total_tokens`). Should `total_cost` or `latency`-related metrics also be promoted?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think just add the ones mentioned here: https://github.com/mlflow/rfcs/pull/23/changes#r3482899422.


2. **Should the expression index for time buckets use a generated column instead?** PostgreSQL 12+ supports `GENERATED ALWAYS AS` columns, which might be cleaner than an expression index for the floor() computation.

3. **Approximate percentiles:** Should this RFC include switching to `percentile_disc` (or a t-digest extension) for dashboard queries, or defer to a follow-up RFC?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's rerun the benchmarks after the optimizations are in place and reconsider it after.


3. **Approximate percentiles:** Should this RFC include switching to `percentile_disc` (or a t-digest extension) for dashboard queries, or defer to a follow-up RFC?

4. **Add `experiment_id` to `trace_metrics` too?** This RFC proposes adding `experiment_id` to `assessments` (Section 3) and denormalizing the three hottest metrics off `trace_metrics` entirely (Section 2). For the remaining

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I say leave out for now.

index on `trace_metrics` is proposed.


### 2. Denormalize frequently-accessed token metrics

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we also consider denormalizing mlflow.trace.session from the trace_request_metadata table to optimize the dashboard overview's session count?


-- assessments: replaces the existing index on (trace_id, created_timestamp)
-- with a covering version that adds name and value to the leaf pages. This
-- lets the aggregation query (GROUP BY name, AVG(value)) be served entirely

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think this will allow for the index to be used for average because the backend queries it with a CASE. My other comment had an alternative suggestion for denormalzing the values we need to average and get percentages on.

-- INCLUDE (name, value) carries the aggregation payload in the index so
-- the entire query (filter + group + aggregate) is served from a single
-- index scan.
CREATE INDEX CONCURRENTLY idx_assessments_exp_ts

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From AI:
The proposed index keys on the wrong timestamp column. Section 3 proposes idx_assessments_exp_ts ON assessments (experiment_id, last_updated_timestamp) and rewrites the dashboard filter to last_updated_timestamp. But the assessment time-bucket GROUP BY buckets on created_timestamp, not last_updated_timestamp. See get_time_bucket_expression:

case MetricViewType.ASSESSMENTS:
    timestamp_column = SqlAssessments.created_timestamp

Meanwhile the time-range filter in query_trace_metrics keys off trace_info.timestamp_ms:

if start_time_ms is not None:
    query = query.filter(SqlTraceInfo.timestamp_ms >= start_time_ms)
if end_time_ms is not None:
    query = query.filter(SqlTraceInfo.timestamp_ms <= end_time_ms)

So an index on (experiment_id, last_updated_timestamp) orders neither the created_timestamp bucket/sort nor the timestamp_ms filter — the GROUP BY will still sort. To actually serve the query, key the index on (experiment_id, created_timestamp) to match the real bucketing/ordering. Also worth calling out explicitly in the RFC: the current code already mixes time semantics — it filters on trace_info.timestamp_ms but buckets on assessments.created_timestamp — so the RFC should clarify which timestamp the denormalized index and rewritten filter are meant to align with.

Comment thread rfcs/0006-postgres-optimizations/0006-traces-optimizations.md
is needed to figure out how this edge case would be handled

So the separation is:
- **Dashboard (analytical):** `(experiment_id, last_updated_timestamp)` — fast,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to store two explicit time axes on assessments: one for when the trace ran (e.g. trace_timestamp_ms, optionally trace_day_bucket_ms) and one for when the assessment was created/updated?

Right now this section is forced to pick between trace-time semantics and assessment-activity semantics. Carrying both would let the API support both questions cleanly: "how did traces that ran in this window score?" and "what scoring activity happened in this window?" It also gives the dashboard a no-join path without baking in ambiguous time semantics.

🤖 Generated with Claude

| Query | Before (JOIN) | After (direct) | Improvement |
|-------|--------------|----------------|-------------|
| `assessment_value` AVG/P90/P99 | 105.8s | ~8-10s | **~11x** |
| `assessment_count` by name+value | 60.5s | ~5-7s | **~9x** |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think there may be another worthwhile schema addition specifically for the assessment_count by name+value path.

Even if we materialize numeric aggregate_value for AVG / percentile, this query still groups on raw JSON text in assessments.value, which can be wide and can split logically equivalent payloads by serialization details. Would it help to add a canonicalized grouping representation, e.g. value_text_norm for scalar/enum-like values plus value_hash for larger structured payloads?

Then the count path can group on (name, value_hash) (and optionally display value_text_norm) instead of grouping directly on TEXT JSON.

🤖 Generated with Claude


This is the one of the slow query pattern: **115 seconds** for a daily token SUM.

**Solution:** Add nullable columns directly on `trace_info` for the most

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we widen this section from denormalizing only hot metrics to denormalizing the hot analytics dimensions too?

Even if token columns move onto trace_info, the current trace metrics path still joins trace_tags for trace_name and trace_request_metadata for mlflow.trace.session / session count. A narrow 1:1 analytics projection (either extra columns on trace_info or a sidecar trace_analytics table keyed by request_id) for trace_name, session_id, maybe status, plus the hot token fields, would remove three current joins at once and make future dashboard metrics cheaper to add.

This feels like a better long-term shape than promoting one metric at a time.

🤖 Generated with Claude

@kramaranya kramaranya left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you, @aws-khatria!
I left a few comments

columns on `trace_info`:

```python
# In SqlAlchemyTrackingStore._set_trace_metrics()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This function doesn't seem to exist, token metrics are written in start_trace() via TOKEN_USAGE metadata. Could you update this please?


## Adoption strategy

- **Non-breaking change:** All optimizations are additive schema changes (indexes, columns). No existing APIs or behaviors change.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: RFC also deletes rows from trace_metrics and changes where tokens are written, which isn't purely additive

- **Non-breaking change:** All optimizations are additive schema changes (indexes, columns). No existing APIs or behaviors change.
- **Alembic migration:** Ships as a standard MLflow database migration. User run `mlflow db upgrade` as they do for any release.
- **Requires maintenance window:** The migration assumes no traffic is served during execution. Estimated duration: ~30 min for 10M traces (need to be vetted).
- **New installs:** Get the optimized schema from the start (denormalized columns and all indexes). Token metrics are written directly to `trace_info`, assessments include `experiment_id` and `timestamp_ms` from creation — no JOINs needed for dashboard queries from day one.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To clarify, do you mean the existing assessment timestamps?

Comment on lines +220 to +223
-- 3. Remove migrated rows from trace_metrics
DELETE FROM trace_metrics
WHERE key IN ('input_tokens', 'output_tokens', 'total_tokens');
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What happens if the migration fails after this step?

@dkuc

dkuc commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

One pattern I'm seeing with all the slow time_bucket based queries is that the bucket is being derived dynamically by the group by. If instead we generate the buckets ahead of time with GENERATE_SERIES() and join to it, it should be considerablly faster.

(for these examples I'm using the denormalized table for simplicity)

Before:

SELECT FLOOR(trace_info.timestamp_ms / CAST(2592000000 AS NUMERIC)) * 2592000000 AS time_bucket,
       SUM(trace_info.input_tokens) AS "SUM"
FROM trace_info
WHERE trace_info.experiment_id = 0
  AND trace_info.timestamp_ms >= 1751916029984
  AND trace_info.timestamp_ms <= 1783452029984
GROUP BY FLOOR(trace_info.timestamp_ms / CAST(2592000000 AS NUMERIC)) * 2592000000
ORDER BY FLOOR(trace_info.timestamp_ms / CAST(2592000000 AS NUMERIC)) * 2592000000
LIMIT 1000;

After

SELECT b.bucket_start AS time_bucket,
       sub.sum_val    AS "SUM"
FROM GENERATE_SERIES(1751916029984::bigint, 1783452029984::bigint, 2592000000::bigint) AS b(bucket_start)
         CROSS JOIN LATERAL (
    SELECT SUM(ti.input_tokens) AS sum_val
    FROM trace_info ti
    WHERE ti.experiment_id = 0
      AND ti.timestamp_ms >= b.bucket_start
      AND ti.timestamp_ms < b.bucket_start + 2592000000::bigint
    ) sub
ORDER BY b.bucket_start
LIMIT 1000;

And if we did go with this pattern, the index to go with would probably cover the denormalized columns.

CREATE INDEX idx_trace_info_exp_ts_tokens                                                                                                                                                                                         
      ON trace_info (experiment_id, timestamp_ms)                                                                                                                                                                                   
      INCLUDE (input_tokens, output_tokens, total_tokens);

ADD COLUMN total_tokens BIGINT;

-- 2. Backfill from trace_metrics
UPDATE trace_info ti

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most likely faster to do this without subqueries.

UPDATE trace_info ti                                                                                                                                                                                                              
  SET input_tokens  = tm.input_tokens,                                                                                                                                                                                              
      output_tokens = tm.output_tokens,                                                                                                                                                                                             
      total_tokens  = tm.total_tokens                                                                                                                                                                                               
  FROM (                                                                                                                                                                                                                            
      SELECT request_id,                                                                                                                                                                                                            
             MAX(value) FILTER (WHERE key = 'input_tokens')  AS input_tokens,                                                                                                                                                       
             MAX(value) FILTER (WHERE key = 'output_tokens') AS output_tokens,                                                                                                                                                      
             MAX(value) FILTER (WHERE key = 'total_tokens')  AS total_tokens                                                                                                                                                        
      FROM trace_metrics                                                                                                                                                                                                            
      WHERE key IN ('input_tokens', 'output_tokens', 'total_tokens')                                                                                                                                                                
      GROUP BY request_id                                                                                                                                                                                                           
  ) tm                                                                                                                                                                                                                              
  WHERE ti.request_id = tm.request_id

@mprahl

mprahl commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

One pattern I'm seeing with all the slow time_bucket based queries is that the bucket is being derived dynamically by the group by. If instead we generate the buckets ahead of time with GENERATE_SERIES() and join to it, it should be considerablly faster.

(for these examples I'm using the denormalized table for simplicity)

Before:

SELECT FLOOR(trace_info.timestamp_ms / CAST(2592000000 AS NUMERIC)) * 2592000000 AS time_bucket,
       SUM(trace_info.input_tokens) AS "SUM"
FROM trace_info
WHERE trace_info.experiment_id = 0
  AND trace_info.timestamp_ms >= 1751916029984
  AND trace_info.timestamp_ms <= 1783452029984
GROUP BY FLOOR(trace_info.timestamp_ms / CAST(2592000000 AS NUMERIC)) * 2592000000
ORDER BY FLOOR(trace_info.timestamp_ms / CAST(2592000000 AS NUMERIC)) * 2592000000
LIMIT 1000;

After

SELECT b.bucket_start AS time_bucket,
       sub.sum_val    AS "SUM"
FROM GENERATE_SERIES(1751916029984::bigint, 1783452029984::bigint, 2592000000::bigint) AS b(bucket_start)
         CROSS JOIN LATERAL (
    SELECT SUM(ti.input_tokens) AS sum_val
    FROM trace_info ti
    WHERE ti.experiment_id = 0
      AND ti.timestamp_ms >= b.bucket_start
      AND ti.timestamp_ms < b.bucket_start + 2592000000::bigint
    ) sub
ORDER BY b.bucket_start
LIMIT 1000;

And if we did go with this pattern, the index to go with would probably cover the denormalized columns.

CREATE INDEX idx_trace_info_exp_ts_tokens                                                                                                                                                                                         
      ON trace_info (experiment_id, timestamp_ms)                                                                                                                                                                                   
      INCLUDE (input_tokens, output_tokens, total_tokens);

Interesting suggestion!

I tried the GENERATE_SERIES + LATERAL shape on the local Postgres benchmark with ~822k denormalized traces. With the existing (experiment_id, timestamp_ms) index, it was slower than the current grouped query across daily, hourly, and sparse-prefix ranges. The repeated per-bucket index scans and heap lookups were more expensive than Postgres’s parallel scan/group aggregate plan.

I didn’t test the additional covering-index variant proposed here. That may reduce heap lookups, but it would add a large new index and write amplification, so I think I'll skip this for now.

@mprahl

mprahl commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

After a discussion with @aws-khatria offline, we decided that I continue with the RFC due to capacity constraints. The new PR is #24. Thanks so much for your help on this @aws-khatria and thanks for the reviews!

@mprahl mprahl closed this Jul 8, 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.

4 participants