From a94c75252972bdba9830a23d07ee7c76cf2708a4 Mon Sep 17 00:00:00 2001 From: Michael SCHAL Date: Sun, 4 Oct 2026 12:25:39 +0200 Subject: [PATCH] =?UTF-8?q?Optimiseur=20:=20corrections=20de=20relecture?= =?UTF-8?q?=20du=20lot=202=20=E2=80=94=20verrou=20p=C3=A9rim=C3=A9=20recon?= =?UTF-8?q?nu,=20application=20interrompue=20d=C3=A9faite?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le verrou de l'optimiseur ne doit jamais bloquer quand aucune optimisation ne tourne, et ne jamais passer pour périmé quand elle tourne. Plusieurs identités de processus ne tenaient pas ces deux promesses hors Windows. - Linux : l'instant de démarrage est relatif au boot. Après une coupure, un service relancé au même moment du démarrage retrouvait PID et top : le verrou de l'ancien passait pour vivant et tout était refusé jusqu'à son retrait à la main. L'identifiant du boot le préfixe ; un verrou qui porte notre PID sans être l'un des nôtres est périmé. - Linux : un binaire remplacé par une mise à jour (« (deleted) ») faisait passer le propriétaire vivant pour mort. La mise à jour est de toute façon refusée pendant une optimisation (interface et loki update). - macOS : ps rend lstart dans la langue et le fuseau de l'appelant ; le web (launchd) et un terminal en français ne lisaient pas la même identité. LC_ALL=C et TZ=UTC. - Un verrou illisible tout juste créé (vide entre création et écriture) n'est plus retiré : seulement après 10 s. - loki tune : terminal fermé ou kill (SIGTERM, SIGHUP) annulent proprement, comme Ctrl-C ; Ctrl-C retrouve son sens aux questions de fin. - Application : la sauvegarde est notée dans le verrou ; un Loki tué avant la vérification remet l'ancienne version au redémarrage. Deux clics rapprochés ne lancent plus deux applications. Un échec de relance du vrai moteur se voit dans le résultat, plus seulement au journal ; un essai qui survit à son arrêt reste noté dans le verrou. Co-Authored-By: Claude Opus 5.5 --- internal/loki/backend_tune_job.go | 16 +++- internal/loki/backend_tune_lock.go | 76 +++++++++++++++++-- internal/loki/backend_tune_run.go | 29 ++++++- internal/loki/backend_tune_test.go | 117 +++++++++++++++++++++++++++++ internal/loki/sys_procid_linux.go | 16 +++- internal/loki/sys_procid_other.go | 9 ++- internal/loki/sys_update.go | 13 ++++ 7 files changed, 264 insertions(+), 12 deletions(-) diff --git a/internal/loki/backend_tune_job.go b/internal/loki/backend_tune_job.go index 632cfea..7fa44c7 100644 --- a/internal/loki/backend_tune_job.go +++ b/internal/loki/backend_tune_job.go @@ -12,6 +12,7 @@ import ( "path/filepath" "strings" "sync" + "syscall" "time" ) @@ -202,7 +203,15 @@ func handleTuneApply(w http.ResponseWriter, r *http.Request) { sendJSON(w, 409, map[string]any{"ok": false, "error": conv.busyReason().Error()}) return } + // Vérifié et posé d'un seul tenant : deux clics rapprochés passaient tous + // deux le test du début, et le perdant remettait applying à faux pendant + // que le premier écrivait encore. tuneJob.mu.Lock() + if tuneJob.running || tuneJob.applying { + tuneJob.mu.Unlock() + sendJSON(w, 409, map[string]any{"ok": false, "error": errTuneBusy.Error()}) + return + } tuneJob.applying, tuneJob.applyLog, tuneJob.applyMsg, tuneJob.applyErr = true, nil, "", "" tuneJob.mu.Unlock() go func() { @@ -282,8 +291,11 @@ func cmdTune(args []string) error { fmt.Println(dim(" Le chat et les tâches attendent pendant la mesure ; Ctrl-C annule (le moteur est relancé).")) fmt.Println() // Ctrl-C annule proprement : l'essai est arrêté, le vrai moteur relancé. + // Le terminal fermé (SIGHUP ; CTRL_CLOSE_EVENT, livré en SIGTERM sous + // Windows) ou un kill aussi : sans ça, le processus mourait sans ses defers, + // le vrai moteur restait arrêté et l'essai gardait la VRAM. sig := make(chan os.Signal, 1) - signal.Notify(sig, os.Interrupt) + signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) defer signal.Stop(sig) go func() { if _, ok := <-sig; ok { @@ -302,6 +314,8 @@ func cmdTune(args []string) error { if err != nil { return err } + // Les essais sont finis : Ctrl-C reprend son sens ordinaire aux questions. + signal.Stop(sig) printTuneResult(res) if len(res.Set) == 0 { return nil diff --git a/internal/loki/backend_tune_lock.go b/internal/loki/backend_tune_lock.go index 47ef605..aa45f53 100644 --- a/internal/loki/backend_tune_lock.go +++ b/internal/loki/backend_tune_lock.go @@ -39,6 +39,10 @@ import ( const tuneLockName = "tune.lock" +// tuneLockFreshness : un verrou illisible plus jeune que ça est en cours +// d'écriture, pas abandonné. +const tuneLockFreshness = 10 * time.Second + var errTuneBusy = errors.New("optimisation en cours (loki tune) : le moteur est réservé aux essais — " + "attends la fin ou annule-la") @@ -59,6 +63,12 @@ type tuneOwner struct { // MainWasActive : le vrai moteur tournait avant l'optimisation. Un verrou // périmé trouvé au démarrage du processus web le relance. MainWasActive bool `json:"main_was_active"` + // Application en cours (phase « application ») : la version d'avant du + // preset, sauvée avant toute écriture. Un verrou périmé dans cette phase + // veut dire que l'application n'a jamais été vérifiée : elle est défaite. + Backup string `json:"backup,omitempty"` + PresetID string `json:"preset_id,omitempty"` + PresetName string `json:"preset_name,omitempty"` } func tuneLockPath() string { return filepath.Join(LokiHome(), tuneLockName) } @@ -89,7 +99,8 @@ var tuneProcAlive = func(p tuneProc) bool { // que le nom. func sameExe(a, b string) bool { norm := func(s string) string { - s = strings.ToLower(filepath.Base(strings.ReplaceAll(strings.TrimSpace(s), `\`, "/"))) + s = strings.TrimSuffix(strings.TrimSpace(s), " (deleted)") // binaire remplacé (Linux) + s = strings.ToLower(filepath.Base(strings.ReplaceAll(s, `\`, "/"))) return strings.TrimSuffix(s, ".exe") } return a != "" && b != "" && norm(a) == norm(b) @@ -126,7 +137,24 @@ func tuneActive() (tuneOwner, bool) { if !ok { return tuneOwner{}, false } - return o, tuneProcAlive(o.Owner) + return o, tuneOwnerAlive(o) +} + +// tuneHeld : verrous pris par CE processus et pas encore rendus (tuneHeldKey). +var tuneHeld sync.Map + +func tuneHeldKey(o tuneOwner) string { return fmt.Sprintf("%s|%d", o.Via, o.Since) } + +// tuneOwnerAlive : le propriétaire d'un verrou vit-il encore ? Un verrou qui +// porte NOTRE PID sans être l'un des nôtres vient d'un processus mort dont le +// PID nous est échu (redémarrage) : périmé, quoi que dise son identité. +func tuneOwnerAlive(o tuneOwner) bool { + if o.Owner.PID == os.Getpid() { + if _, ours := tuneHeld.Load(tuneHeldKey(o)); !ours { + return false + } + } + return tuneProcAlive(o.Owner) } // tuneGuard : nil si rien ne tient le moteur, errTuneBusy sinon. @@ -151,6 +179,7 @@ type tuneLock struct { mu sync.Mutex path string owner tuneOwner + held bool // inscrit dans tuneHeld, jusqu'à release } // tuneLockAcquire prend le verrou, ou dit qui le tient. Un verrou périmé est @@ -172,6 +201,8 @@ func tuneLockAcquire(via string, mainActive bool) (*tuneLock, error) { _ = os.Remove(path) return nil, fmt.Errorf("verrou de l'optimiseur : %v", errors.Join(werr, cerr)) } + l.held = true + tuneHeld.Store(tuneHeldKey(l.owner), true) return l, nil } if !os.IsExist(err) { @@ -227,10 +258,22 @@ func (l *tuneLock) setPhase(phase string) { _ = l.write() } +// setBackup note la sauvegarde du preset avant son écriture (tuneApply). +func (l *tuneLock) setBackup(backup, id, name string) { + l.mu.Lock() + defer l.mu.Unlock() + l.owner.Backup, l.owner.PresetID, l.owner.PresetName = backup, id, name + _ = l.write() +} + // release rend le verrou — seulement s'il est encore le nôtre. func (l *tuneLock) release() { l.mu.Lock() defer l.mu.Unlock() + if l.held { + l.held = false + tuneHeld.Delete(tuneHeldKey(l.owner)) + } if o, ok := readTuneOwner(l.path); ok && o.Owner.PID == l.owner.Owner.PID && o.Owner.Start == l.owner.Owner.Start { _ = os.Remove(l.path) } @@ -245,13 +288,16 @@ func tuneReapStale() (mainWasActive bool) { path := tuneLockPath() o, ok := readTuneOwner(path) if !ok { - // Illisible (écriture interrompue) : périmé lui aussi, s'il est là. - if _, err := os.Stat(path); err == nil { + // Illisible (écriture interrompue) : périmé lui aussi, s'il est là — + // mais pas tout juste créé : entre sa création exclusive et l'écriture de + // son contenu, un verrou neuf est vide quelques instants, et le retirer + // laisserait passer une seconde optimisation ou un démarrage du moteur. + if fi, err := os.Stat(path); err == nil && time.Since(fi.ModTime()) > tuneLockFreshness { _ = os.Remove(path) } return false } - if tuneProcAlive(o.Owner) { + if tuneOwnerAlive(o) { return false } stale := fmt.Sprintf("%s.stale-%d-%d", path, os.Getpid(), time.Now().UnixNano()) @@ -263,9 +309,29 @@ func tuneReapStale() (mainWasActive bool) { fmt.Fprintf(os.Stderr, "[loki tune] essai orphelin (PID %d) d'une optimisation interrompue : arrêt\n", o.Trial.PID) tuneKillTree(o.Trial.PID, nil) } + tuneUndoApply(o) return o.MainWasActive } +// tuneUndoApply : une application interrompue avant sa vérification (Loki tué +// pendant la sonde) remet la version d'avant du preset — c'est ce que la +// vérification aurait fait en échouant. Sans sauvegarde notée, rien. +func tuneUndoApply(o tuneOwner) { + if o.Phase != "application" || o.Backup == "" || o.PresetID == "" { + return + } + b, err := os.ReadFile(o.Backup) + if err != nil { + fmt.Fprintf(os.Stderr, "[loki tune] application interrompue : sauvegarde illisible (%v)\n", err) + return + } + if _, _, err := SavePresetApplying(o.PresetID, o.PresetName, string(b)); err != nil { + fmt.Fprintf(os.Stderr, "[loki tune] application interrompue : retour impossible (%v) — l'ancienne version est dans %s\n", err, o.Backup) + return + } + fmt.Fprintf(os.Stderr, "[loki tune] application interrompue avant sa vérification : ancienne version du preset rétablie\n") +} + // tuneRecoverAtBoot : au démarrage du processus web, une optimisation // interrompue (Loki tué en plein essai) a laissé le vrai moteur arrêté et peut- // être un essai en VRAM. On arrête l'essai, puis on relance le moteur s'il diff --git a/internal/loki/backend_tune_run.go b/internal/loki/backend_tune_run.go index cf880e1..2e07da3 100644 --- a/internal/loki/backend_tune_run.go +++ b/internal/loki/backend_tune_run.go @@ -351,7 +351,14 @@ func (t *tuneTrialer) load(ctx context.Context, dir string, port int, launch tun stop := func() { before := gpuUsedMB(gpuStats()) tuneKillTree(pid, done) - t.lock.setTrial(nil) + // Noté jusqu'à sa fin constatée : un essai qui survit à l'arrêt reste + // désigné dans le verrou, qu'un Loki redémarré pourra encore arrêter. + select { + case <-done: + t.lock.setTrial(nil) + default: + fmt.Fprintf(os.Stderr, "[loki tune] l'essai (PID %d) ne s'est pas arrêté dans les temps\n", pid) + } if before > 0 { gpuUsedSettled(before) } else { @@ -404,7 +411,7 @@ func tuneBenchSetup(cfg map[string]string, argEnv map[string]string, cpuPlaced b // runTune déroule une optimisation. L'appelant fournit le suivi ; le verrou, // l'arrêt et la relance du vrai moteur sont faits ici. -func runTune(ctx context.Context, opts tuneOpts, tr *tuneTracker) (*tuneResult, error) { +func runTune(ctx context.Context, opts tuneOpts, tr *tuneTracker) (out *tuneResult, retErr error) { if opts.Budget <= 0 { opts.Budget = tuneDefaultBudget } @@ -435,12 +442,23 @@ func runTune(ctx context.Context, opts tuneOpts, tr *tuneTracker) (*tuneResult, } lock.setPhase("relance") tr.setPhase("relance du moteur principal") + // Un échec de relance doit se voir là où l'on regarde (résultat de + // l'interface, sortie de « loki tune »), pas seulement au journal. + failed := func(msg string) { + fmt.Fprintf(os.Stderr, "[loki tune] %s\n", msg) + switch { + case out != nil: + out.Notes = append(out.Notes, "⚠️ "+msg) + case retErr != nil: + retErr = fmt.Errorf("%w — %s", retErr, msg) + } + } if err := preflightEngine(); err != nil { - fmt.Fprintf(os.Stderr, "[loki tune] relance impossible : %s\n", plainErr(err)) + failed("relance du moteur principal impossible : " + plainErr(err)) return } if err := serviceActionOS("start"); err != nil { - fmt.Fprintf(os.Stderr, "[loki tune] relance du moteur : %s\n", plainErr(err)) + failed("relance du moteur principal : " + plainErr(err)) } gpuMonitor.invalidate() }() @@ -903,6 +921,9 @@ func tuneApply(ctx context.Context, res *tuneResult, target string, say func(str if err := os.WriteFile(backup, []byte(content), 0o600); err != nil { return "", fmt.Errorf("sauvegarde du preset : %w", err) } + // Noté dans le verrou AVANT d'écrire : un Loki tué pendant l'application + // la défait au redémarrage (tuneUndoApply). + lock.setBackup(backup, res.PresetID, res.PresetName) revert := func(why string) (string, error) { say("échec (" + why + ") : retour à l'ancienne version") if _, _, err := SavePresetApplying(res.PresetID, res.PresetName, content); err != nil { diff --git a/internal/loki/backend_tune_test.go b/internal/loki/backend_tune_test.go index 7e5aa63..3524b51 100644 --- a/internal/loki/backend_tune_test.go +++ b/internal/loki/backend_tune_test.go @@ -715,3 +715,120 @@ func TestTuneStageWanted(t *testing.T) { } } } + +// Corrections de relecture du lot 2 : un verrou qui porte NOTRE PID sans que +// nous en tenions un vient d'un processus mort (PID réattribué après un +// redémarrage, même instant de démarrage relatif au boot) : il ne bloque rien. +// Un verrou illisible tout juste créé est en cours d'écriture : ni retiré ni +// bloquant ; vieux, il est écarté. +func TestTuneLockStaleSelfAndFresh(t *testing.T) { + testHome(t) + prev := tuneProcAlive + tuneProcAlive = func(tuneProc) bool { return true } // l'identité concorde + t.Cleanup(func() { tuneProcAlive = prev }) + if err := os.MkdirAll(filepath.Dir(tuneLockPath()), 0o755); err != nil { + t.Fatal(err) + } + b, _ := json.Marshal(tuneOwner{Owner: selfProc(), Via: "web", Phase: "essais"}) + if err := os.WriteFile(tuneLockPath(), b, 0o644); err != nil { + t.Fatal(err) + } + if err := tuneGuard(); err != nil { + t.Fatalf("verrou d'un ancien processus au même PID encore bloquant : %v", err) + } + l, err := tuneLockAcquire("cli", false) + if err != nil { + t.Fatalf("verrou périmé non écarté : %v", err) + } + if !errors.Is(tuneGuard(), errTuneBusy) { + t.Error("notre propre verrou ne bloque plus") + } + l.release() + if _, held := tuneHeld.Load(tuneHeldKey(l.owner)); held { + t.Error("verrou rendu encore inscrit comme tenu") + } + + // Illisible et neuf : laissé en place. + if err := os.WriteFile(tuneLockPath(), nil, 0o644); err != nil { + t.Fatal(err) + } + tuneReapStale() + if _, err := os.Stat(tuneLockPath()); err != nil { + t.Fatal("verrou en cours d'écriture retiré") + } + // Illisible et vieux : écarté. + old := time.Now().Add(-2 * tuneLockFreshness) + if err := os.Chtimes(tuneLockPath(), old, old); err != nil { + t.Fatal(err) + } + tuneReapStale() + if _, err := os.Stat(tuneLockPath()); !os.IsNotExist(err) { + t.Fatal("verrou illisible abandonné non retiré") + } +} + +// Les binaires remplacés par une mise à jour (« (deleted) » sous Linux) restent +// le même processus. +func TestTuneSameExeDeleted(t *testing.T) { + if !sameExe("/usr/local/bin/loki", "/usr/local/bin/loki (deleted)") { + t.Error("binaire remplacé pris pour un autre") + } + if sameExe("/usr/local/bin/loki", "/usr/bin/llama-server") { + t.Error("binaires différents confondus") + } +} + +// Une application interrompue avant sa vérification (Loki tué pendant la +// sonde) est défaite quand le verrou périmé est écarté : preset ET +// configuration reviennent à la version sauvegardée. +func TestTuneUndoInterruptedApply(t *testing.T) { + testHome(t) + prev := tuneProcAlive + tuneProcAlive = func(tuneProc) bool { return false } // propriétaire mort + t.Cleanup(func() { tuneProcAlive = prev }) + old := "# NAME=Dense\nMODEL=m.gguf\nUBATCH=512\n" + patched := "# NAME=Dense\nMODEL=m.gguf\nUBATCH=2048\n" + if err := os.MkdirAll(presetsDir(), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(presetsDir(), "Dense.env"), []byte(patched), 0o644); err != nil { + t.Fatal(err) + } + if err := WriteConfig(parseEnv(patched)); err != nil { + t.Fatal(err) + } + backup := filepath.Join(t.TempDir(), "Dense.env") + if err := os.WriteFile(backup, []byte(old), 0o600); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(tuneLockPath()), 0o755); err != nil { + t.Fatal(err) + } + b, _ := json.Marshal(tuneOwner{Owner: tuneProc{PID: 999999, Start: "x"}, Via: "apply", Phase: "application", + Backup: backup, PresetID: "Dense", PresetName: "Dense", MainWasActive: true}) + if err := os.WriteFile(tuneLockPath(), b, 0o644); err != nil { + t.Fatal(err) + } + if !tuneReapStale() { + t.Error("MainWasActive perdu") + } + if got, _ := ReadPreset("Dense"); got != old { + t.Errorf("preset non rétabli : %q", got) + } + if ReadConfig()["UBATCH"] != "512" { + t.Errorf("configuration non rétablie : %v", ReadConfig()) + } + // Hors phase d'application, rien n'est touché. + if err := WriteConfig(parseEnv(patched)); err != nil { + t.Fatal(err) + } + b, _ = json.Marshal(tuneOwner{Owner: tuneProc{PID: 999999, Start: "x"}, Via: "cli", Phase: "essais", + Backup: backup, PresetID: "Dense", PresetName: "Dense"}) + if err := os.WriteFile(tuneLockPath(), b, 0o644); err != nil { + t.Fatal(err) + } + tuneReapStale() + if ReadConfig()["UBATCH"] != "2048" { + t.Error("configuration modifiée hors phase d'application") + } +} diff --git a/internal/loki/sys_procid_linux.go b/internal/loki/sys_procid_linux.go index cba8cb0..55ec38d 100644 --- a/internal/loki/sys_procid_linux.go +++ b/internal/loki/sys_procid_linux.go @@ -12,6 +12,11 @@ import ( // 22 de /proc//stat) et binaire d'un processus vivant. Un PID recyclé // après un redémarrage de Loki a un autre instant de démarrage : l'optimiseur ne // tue un essai orphelin que si les deux concordent (backend_tune_lock.go). +// +// L'instant est relatif au boot : après une coupure de courant, un service +// lancé au même moment du démarrage peut retrouver le même PID ET le même top, +// et un verrou laissé par l'ancien passerait pour vivant — tout serait refusé +// jusqu'à son retrait à la main. L'identifiant du boot le préfixe donc. func procIdentity(pid int) (start, exe string, ok bool) { if pid <= 0 { return "", "", false @@ -34,5 +39,14 @@ func procIdentity(pid int) (start, exe string, ok bool) { return "", "", false } exe, _ = os.Readlink("/proc/" + strconv.Itoa(pid) + "/exe") - return f[19], exe, true + // Binaire remplacé par une mise à jour pendant que le processus tourne : le + // lien porte « (deleted) », c'est pourtant bien le même processus. + exe = strings.TrimSuffix(exe, " (deleted)") + start = f[19] + if b, err := os.ReadFile("/proc/sys/kernel/random/boot_id"); err == nil { + if id := strings.TrimSpace(string(b)); id != "" { + start = id + "/" + start + } + } + return start, exe, true } diff --git a/internal/loki/sys_procid_other.go b/internal/loki/sys_procid_other.go index 15f2d09..14e5a7b 100644 --- a/internal/loki/sys_procid_other.go +++ b/internal/loki/sys_procid_other.go @@ -3,6 +3,7 @@ package loki import ( + "os" "os/exec" "strconv" "strings" @@ -15,7 +16,13 @@ func procIdentity(pid int) (start, exe string, ok bool) { return "", "", false } p := strconv.Itoa(pid) - out, err := exec.Command("ps", "-o", "lstart=", "-p", p).Output() + // lstart passe par strftime(%c) dans la langue et le fuseau de l'appelant : + // le processus web (launchd, sans LANG) et un terminal en français ne liraient + // pas la même chaîne pour le même processus, et un verrou vivant passerait + // pour périmé. Langue et fuseau fixés. + cmd := exec.Command("ps", "-o", "lstart=", "-p", p) + cmd.Env = append(os.Environ(), "LC_ALL=C", "LANG=C", "TZ=UTC") + out, err := cmd.Output() if err != nil { return "", "", false } diff --git a/internal/loki/sys_update.go b/internal/loki/sys_update.go index 86aa87a..e48cf63 100644 --- a/internal/loki/sys_update.go +++ b/internal/loki/sys_update.go @@ -243,6 +243,13 @@ func cmdUpdate(args []string) error { fmt.Println(containerUpdateNote) return nil } + if !checkOnly { + // Même règle que l'interface : pas de remplacement de binaire pendant + // une optimisation. + if err := tuneGuard(); err != nil { + return err + } + } fmt.Println("recherche de la dernière version…") info, err := checkForUpdate() if err != nil { @@ -458,6 +465,12 @@ func handleUpdateApply(w http.ResponseWriter, r *http.Request) { sendJSON(w, 409, map[string]any{"ok": false, "error": containerUpdateNote}) return } + // Optimisation en cours : la mise à jour relance Loki, ce qui couperait + // l'optimisation du processus web — ou ferait passer pour mort le + // propriétaire d'une optimisation en ligne de commande. + if tuneDenyHTTP(w) { + return + } newVer, err := applyUpdate() if err != nil { // 500 : un client script peut tester le statut HTTP ; l'UI, elle, lit le JSON.