Skip to content

Derive the model download and cancellation timeouts from measurement #114

Description

@ckelseynv

Model download and cancellation are covered by a chain of timeouts in the desktop app, the broker, the engine manager, and the terminal interface. Reviewing #111 turned up several that are not wrong so much as unjustified: they were picked independently, and a few cannot fire or fire far too early. None of them produces a wrong user-visible outcome today — the ones that did were fixed in that pull request — so this is hardening, not a bug report.

The common thread is that each budget was chosen on its own rather than derived from the thing it waits on. The goal is to make them derived, and to measure the real durations first so the derivation rests on evidence.

Budgets that cannot apply, or apply too soon

  • PULL_TIMEOUT_MS is unreachable. The desktop allows six hours for pull_model, but the engine manager's actionTimeout is 30 minutes and compile-time only, so it cuts every pull first. The real download ceiling is 30 minutes, and the desktop comment describing "a multi-GB download on a slow link" describes a budget that can never take effect. Decide deliberately whether pull_model is exempt from the action ceiling or the value becomes configurable.
  • Post-pull list_models gets 10s by omission. It inherits the json-rpc-subprocess.ts default while the dispatch it invokes is budgeted at 30 minutes. For LM Studio that call is a CLI shell-out.
  • PENDING_TIMEOUT_MS is 60s. It expires the optimistic button cover one minute into a download that can legitimately run for half an hour.
  • The terminal interface's callTimeout is 35s, below the worst-case cancel. It is tolerated only because the terminal interface swallows DeadlineExceeded.
  • Cold dial versus the desktop's cancel budget. A first contact with a peer pays dial and TLS handshake before the cancel's own wait begins, and the desktop budget does not account for it.

Supporting work

  • Instrumentation first. Phase-duration debug logging around a cancel — interrupt to acknowledgement, acknowledgement to exit, exit to cleanup complete — so the budgets can be set from measurement rather than assumption. Operational metadata only, never prompts or response bodies.
  • Derive rather than pick. Once measured, express each budget in terms of the layer beneath it so the two cannot drift apart, and add a check that the ordering still holds.
  • A permanent -race gate in CI. The race detector does not run in CI today. It was run manually for Add model download cancellation and progress #111 in a throwaway container and found nothing, but nothing keeps it that way.
  • A TypeScript parity check for the budget relationships that currently live only in comments.

Smaller items deferred from the same review

  • lmspull.go — give LM Studio's partial-file cleanup the budgeted retry loop cleanupOllamaAfterCancel already has. Skipping a busy file already prevents a wrong deletion; the retry only reduces how often a partial is left behind, so it is an improvement rather than a fix.
  • pullcancel.go — compare with errors.Is rather than ==. Defensive; there is no current failure case.
  • Progress scanning — scan only the newly appended chunk and truncate on a rune boundary. Bounded to roughly 8KiB of ASCII today, so the only effect of a non-ASCII rune is cosmetically garbled diagnostic text.
  • Classifying pre-existing partials — sample them for movement before a pull starts, so a file already growing is known to belong to another client. This is a more principled attribution rule than the current pre-pull snapshot, but it is new machinery plus startup latency.

Related

Most of these budgets exist only because a cancel blocks until it finishes. See #115, on making cancellation asynchronous, which would remove the need for several of them outright.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions