Repository navigation
fix(mlflow): verify or reclaim the tracking port instead of adopting it - #41
Merged
Merged
Conversation
The bootstrap decided MLflow was already running from a bare TCP connect, which only proves something answers. Two ways that broke runs: A listener that is not MLflow, such as AirPlay Receiver on macOS :5000, was adopted as the tracking server, so every create_run failed with a 403 and the operator saw a run that died at birth. Another Caldera tree's server was adopted too, so this tree's runs landed in that tree's store, and the boot reconcile then found that tree's live runs with an empty live cache and marked them KILLED. Fingerprint the listener instead: the tracking API says whether it is MLflow at all, and the artifact root says whether it serves this tree's store. Adopt only our own, reclaim a foreign MLflow by cmdline since macOS denies a port to pid lookup without root, and leave an unidentified process alone with a log naming the port and mlflow.port. Restarting is a normal path now, so spawn via the interpreter running Caldera rather than a bare "mlflow" that PATH may not carry.
Every MLflow write in a run is best effort except the create_run that mints it, so a tracking server that is down surfaced only there, as a raw provider exception. /execute returned "API request to endpoint /api/2.0/mlflow/experiments/get-by-name failed with error code 403" to the chat UI, naming neither MLflow nor the port it came from. Wrap it so the operator is told the tracking server is unavailable, at which URI, and where to look for the cause.
Both refusal tests only asserted that nothing was spawned and nothing was killed, which the original bare port check also satisfied, so neither could fail against the bug they describe. Reinstating the old adopt anything behaviour left them green. Assert the log instead: silent adoption was the defect, so not spawning is not the property that matters, being told why runs are about to fail is.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CALDERA decided MLflow was already running from a bare TCP connect, which only
proves something answers on the port. Whatever held it became the tracking
server for every run. Two ways that broke runs:
:5000, wasadopted, so every
create_runfailed with a 403 and the run died at birth.tree's store, and the boot reconcile then marked that tree's live runs KILLED.
The bootstrap now fingerprints the listener first. The tracking API says whether
it is MLflow, and the artifact root says whether it serves this tree's store.
plugins/mcp/mlruns.dbmlflow.portoverrideAn unidentified process is never killed, only reported. The reclaim finds its
target by cmdline, since macOS denies a port to pid lookup without root.
Two smaller changes: MLflow is spawned via the interpreter running CALDERA
rather than a bare
mlflow, because restarting is now a normal path and PATHmay not carry the right venv. And the
create_runinmcp_svc.executeiswrapped, since it was the only MLflow write in a run that was not best effort,
so a broken tracking server reached the chat UI as a raw 403 naming neither
MLflow nor the port.
Type of change
How Has This Been Tested?
Both failures were reproduced first, then fixed.
red. Two refusal tests originally could not fail against the bug they
described, and now assert that the operator is told.
clean start and stop, foreign squatter reclaimed, second boot adopts,
non-MLflow squatter refused and left alive,
kill -9leak then adopted.server.pyon:5000with a foreign MLflow squatting on it: loggedthe mismatch, killed the squatter, came up on its own store, minted a run,
and stopped MLflow with SIGTERM. Clean restart and stop after that.
Checklist:
the MLflow bootstrap; README and PLUGIN_MCP.md mention MLflow only as the
observability layer)