Add an RFC for advanced trace archival with Iceberg - #28
Conversation
Signed-off-by: mprahl <mprahl@users.noreply.github.com>
etirelli
left a comment
There was a problem hiding this comment.
Overall it looks good. I just have a few questions.
| # Each remaining method explicitly delegates or implements hybrid behavior. | ||
| ``` | ||
|
|
||
| #### Runtime Composition |
There was a problem hiding this comment.
@mprahl this is what I tried to allude to in our call the other day. The proposed design here is a clean step forward that preserves a lot of the existing code, but creates/retains a dependency in the TraceBackend to the TrackingStore.
Did you consider instead breaking that coupling by making the TraceBackend self contained and introducing a higher level coordinator (instead of a decorator)? Something like:
Coordinator
├── trace ops → TraceBackend (owns its own storage for both hot and cold tiers)
└── everything else → TrackingStore
For an "in-process" implementation like the DuckDB/Iceberg proposed here, it wouldn't make a difference, but for independent implementations like ClickHouse or similar backends, that would allow them to cleanly separate and scale independently from the tracking store (something we will hit on large multi-tenant deployments).
There was a problem hiding this comment.
I think we're on the same page. A trace backend may be fully indepedent and own both its hot and cold storage. The tracking store is passed to the trace backend builder because the Iceberg implementation needs it for its SQL hot tier and shared MLflow metadata, but AbstractTraceBackend does not require implementations to use it.
A ClickHouse backend could ignore it for trace persistence and serve all trace operations from ClickHouse. TraceBackendTrackingStore is effectively the coordinator you are suggesting. It routes trace operations to the trace backend and all other operations to the tracking store. I’ll clarify this in the RFC because the current builder signature makes the dependency look mandatory.
| Payloads or files written by an interrupted operation but never referenced by a committed snapshot | ||
| or locator may remain unreachable in storage. This is an accepted phase-one trade-off; a separate | ||
| CLI tool can be introduced in the future for admins to run on a schedule to detect orphaned | ||
| payloads. |
There was a problem hiding this comment.
I believe such a CLI is quite important on a first release. Failures typically are clustered around a root cause, and as such, have the potential to generate a lot of orphaned payloads in a short period of time until fixed. Admins will need a way to clean such orphaned data.
There was a problem hiding this comment.
Fair point. I'll make it required as part of the initial implementation.
| CLI tool can be introduced in the future for admins to run on a schedule to detect orphaned | ||
| payloads. | ||
|
|
||
| **Single writer:** One designated MLflow instance runs the Iceberg maintenance job like the archival |
There was a problem hiding this comment.
just for my own understanding, can you clarify if the single writer referenced here is an in-process writer or is it referring to a write replica in a cluster (like a k8s deployment)?
will this design prevent the deployment of multiple replicas in a k8s cluster?
There was a problem hiding this comment.
The single writer is referring to the current jobs architecture not having any concept of cross-replica leases or locks until RFC-0002 is complete.
In Kubernetes, the workaround we do is disable jobs on MLflow but have a CronJob for each job that calls the Python code directly. This lets Kubernetes ensure the jobs are not running concurrently.
There was a problem hiding this comment.
thank you for the clarification
Signed-off-by: mprahl <mprahl@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
rfcs/0007-iceberg-trace-archival/benchmark-s3/README.md:24
- This uses a developer-specific absolute path (
/home/mprahl/git/mlflow), which isn’t portable and makes the benchmark harder to reproduce from the RFC. Consider using a repo URL or a placeholder path instead.
- Repository: `/home/mprahl/git/mlflow`
rfcs/0007-iceberg-trace-archival/benchmark-s3/README.md:29
- The benchmark hard-codes a specific S3 bucket/prefix (
rhods-dsp-dev/...). In an RFC, it’s usually better to avoid environment-specific (and potentially sensitive) bucket names and instead use a placeholder bucket/prefix.
- Iceberg warehouse: `s3://rhods-dsp-dev/mlflow-hybrid-10m-30d-iceberg/iceberg/warehouse`
rfcs/0007-iceberg-trace-archival/benchmark-filesystem/README.md:25
- These benchmark paths are specific to one developer machine and also embed a
10m-30ddataset name while the benchmark is labeled as 32 days. Using placeholders here would keep the RFC portable and avoid the naming mismatch.
- Iceberg warehouse: `/home/mprahl/mlflow-hybrid-10m-30d-filesystem/iceberg`
- Archive payload root: `/home/mprahl/mlflow-hybrid-10m-30d-filesystem/trace-archive`
Summary
This RFC makes two related decisions:
The Iceberg backend keeps fresh traces in the tracking database, archives older trace payloads to deterministic object-storage paths as serialized OpenTelemetry protobuf messages, and projects archived trace metadata into Apache Iceberg tables for search and analytics. It preserves MLflow's simple deployment model by using PyIceberg and DuckDB inside the MLflow server process instead of introducing a separate always-on query service. The interface also leaves room for a future ClickHouse backend for deployments that prefer a separately operated analytical database.
Unlike RFC-0001, which only moved span payloads out of the database while leaving trace metadata in SQL forever, this design also moves archived trace metadata and analytics state into Iceberg. The result is a true hot/cold split: recent traces remain in the selected SQLAlchemy tracking store for writes and low-latency reads, while archived traces are served from Iceberg plus archive payloads.