From 435ffe1171677dbc4c72ae9f639fe7f0f559c2b5 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Thu, 29 Oct 2026 19:25:00 +0100 Subject: [PATCH] A file removed outside the running mirror is no longer reported, and its removal reaches the peers Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 +++ internal/cli/mirror.go | 4 ++++ internal/mirror/manager.go | 16 +++++++++++-- internal/mirror/peer_test.go | 45 ++++++++++++++++++++++++++++++++++++ internal/mirror/tombstone.go | 19 +++++++++++++++ 5 files changed, 85 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dec971..f0e679a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,9 @@ APIs change between minors without a deprecation period. to run on a control plane. - **Models fetched before checksums were kept reach the second mirror.** A mirror kept in step copies only what it can check, and those files had no sha256; a mirror with peers now works theirs out and records them, once. +- **A file removed with `llmgw mirror remove` is gone from the mirror's answers too.** The running mirror kept + reporting a file it had fetched as present for a week after it was removed, so a control plane could place a + model on a file that was not there; and the removal was not recorded, so a mirror kept in step copied it back. ## v0.15.2 — 2026-10-29 diff --git a/internal/cli/mirror.go b/internal/cli/mirror.go index 40392e6..35f4a22 100644 --- a/internal/cli/mirror.go +++ b/internal/cli/mirror.go @@ -238,6 +238,10 @@ func newMirrorRemoveCmd(dir *string) *cobra.Command { if err := mirror.Remove(*dir, e); err != nil { return err } + // Kept for a mirror kept in step, which otherwise copies it back. + if err := mirror.RecordRemoval(*dir, model, version); err != nil { + return err + } fmt.Fprintf(cmd.OutOrStdout(), "removed %s/%s/%s\n", model, version, file) return nil }, diff --git a/internal/mirror/manager.go b/internal/mirror/manager.go index 0f9df49..c48357c 100644 --- a/internal/mirror/manager.go +++ b/internal/mirror/manager.go @@ -210,7 +210,7 @@ func (m *Manager) Status(model, version string) (Status, bool) { m.mu.Lock() for _, st := range m.running { - if st.Model == model && st.Version == version { + if st.Model == model && st.Version == version && !m.goneLocked(*st) { out, found = merge(out, *st, found) } } @@ -327,7 +327,9 @@ func (m *Manager) All() ([]Status, error) { } for key, st := range known { - if _, onDisk := seen[key]; onDisk { + if _, onDisk := seen[key]; onDisk || st.State == Present { + // Present and not on the disk: removed since, by hand or by + // llmgw mirror remove. Not reported as here, which it is not. continue } out = append(out, st) @@ -335,6 +337,16 @@ func (m *Manager) All() ([]Status, error) { return out, nil } +// goneLocked says whether a finished download's file has been removed since. +// Called with m.mu held. +func (m *Manager) goneLocked(st Status) bool { + if st.State != Present { + return false + } + _, err := os.Stat(filepath.Join(m.root, st.Model, st.Version, st.File)) + return errors.Is(err, os.ErrNotExist) +} + // keptFinished is how long a finished download's status is remembered. const keptFinished = 7 * 24 * time.Hour diff --git a/internal/mirror/peer_test.go b/internal/mirror/peer_test.go index cff3550..4e25970 100644 --- a/internal/mirror/peer_test.go +++ b/internal/mirror/peer_test.go @@ -198,3 +198,48 @@ func TestAFileWithoutAChecksumIsGivenOneAndThenCopied(t *testing.T) { pb.SyncOnce(t.Context(), a.client(t)) waitPresent(t, b, "old", "v1") } + +// A file removed outside the running mirror is not reported as here, and its +// removal, recorded, reaches a peer instead of the peer's copy coming back. +func TestAFileRemovedByHandIsNotReportedAndItsRemovalIsCarried(t *testing.T) { + blob := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("weights")) + })) + defer blob.Close() + a, b := newTestMirror(t), newTestMirror(t) + e := Entry{Model: "qwen", Version: "v1", File: "m.gguf"} + if _, err := a.m.Start(blob.URL+"/m.gguf", e); err != nil { + t.Fatal(err) + } + waitPresent(t, a, "qwen", "v1") + pb := &Peers{M: b.m, Log: discardLog()} + pb.SyncOnce(t.Context(), a.client(t)) + waitPresent(t, b, "qwen", "v1") + + time.Sleep(10 * time.Millisecond) + if err := Remove(a.root, e); err != nil { + t.Fatal(err) + } + if err := RecordRemoval(a.root, "qwen", "v1"); err != nil { + t.Fatal(err) + } + if a.has("qwen", "v1") { + t.Fatal("reported as here after it was removed") + } + all, _ := a.m.All() + for _, st := range all { + if st.Model == "qwen" { + t.Fatalf("listed after it was removed: %+v", st) + } + } + pa := &Peers{M: a.m, Log: discardLog()} + pa.SyncOnce(t.Context(), b.client(t)) + time.Sleep(200 * time.Millisecond) + if a.has("qwen", "v1") { + t.Fatal("copied back from the peer") + } + pb.SyncOnce(t.Context(), a.client(t)) + if b.has("qwen", "v1") { + t.Error("the peer kept it") + } +} diff --git a/internal/mirror/tombstone.go b/internal/mirror/tombstone.go index bc0dd7a..7390f03 100644 --- a/internal/mirror/tombstone.go +++ b/internal/mirror/tombstone.go @@ -57,3 +57,22 @@ func (m *Manager) clearTombstone(model, version string) { m.writeTombstones(t) } } + +// RecordRemoval keeps a version's removal for a mirror's peers when the +// removal was made outside the running mirror, by llmgw mirror remove: once +// none of its files is left, so a peer removes its copy rather than this +// mirror copying it back. +func RecordRemoval(root, model, version string) error { + entries, err := List(root) + if err != nil { + return err + } + for _, e := range entries { + if e.Model == model && e.Version == version { + return nil + } + } + m := &Manager{root: root} + m.recordTombstone(model, version, time.Now()) + return nil +}