Persistance : un résultat d'outil supprimé pendant son écriture ne renaît plus

TestToolResultsSaveLoadDelete échouait de temps en temps en suite
complète (1 fois sur ~300 en isolé). Vraie course dans le code, pas dans
le test : deleteToolResultsFor retirait les résultats EN ATTENTE de
l'écrivain différé, mais pas ceux qu'il avait déjà pris dans son lot.
Si la transaction de suppression passait avant celle de l'écrivain, le
résultat était écrit juste après avoir été effacé, et renaissait.

En production, la suppression d'une discussion passe d'abord par
forget + flush, ce qui masquait le problème ; mais forget non plus ne
pouvait rien contre un lot déjà parti.

- l'écrivain note les résultats du lot en vol ; dropToolRes (donc
  forget et deleteToolResultsFor) les marque annulés
- l'écrivain vérifie l'annulation DANS sa transaction ; la suppression
  annule AVANT d'ouvrir la sienne : bbolt sérialisant les deux, soit le
  résultat est sauté, soit il est écrit puis effacé
- annulations oubliées à la fin du lot : un nouveau résultat s'écrit
- test déterministe (écrivain retenu en plein lot) qui échouait avant
  la correction ; suite complète passée 3 fois, TestToolResults ×400

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
MichaelandClaude Opus 5.5 committed 2026-10-04 10:50:00 +02:00
1 parent 5954d75118
commit 889a85888d
3 files changed
+61

No files matched your search

+34
View File
@@ -74,6 +74,11 @@ type convPersister struct {
queued uint64 // tickets délivrés queued uint64 // tickets délivrés
done uint64 // tickets traités (écrits, abandonnés ou en échec) done uint64 // tickets traités (écrits, abandonnés ou en échec)
running bool running bool
// Résultats d'outils du lot en cours d'écriture, et ceux d'entre eux qu'une
// suppression a annulés depuis : l'écrivain les saute dans sa transaction.
inflight map[string]bool
cancelled map[string]bool
} }
var ( var (
@@ -92,6 +97,9 @@ func newConvPersister() *convPersister {
mem: map[string]string{}, mem: map[string]string{},
written: map[string]uint64{}, written: map[string]uint64{},
deleted: map[string]bool{}, deleted: map[string]bool{},
inflight: map[string]bool{},
cancelled: map[string]bool{},
} }
p.cond = sync.NewCond(&p.mu) p.cond = sync.NewCond(&p.mu)
return p return p
@@ -157,6 +165,7 @@ func (p *convPersister) run() {
for _, j := range p.res { for _, j := range p.res {
if !p.deleted[toolResConv(j.key)] { if !p.deleted[toolResConv(j.key)] {
res = append(res, j) res = append(res, j)
p.inflight[j.key] = true
} }
} }
p.snaps, p.res = map[string]*convSnap{}, nil p.snaps, p.res = map[string]*convSnap{}, nil
@@ -166,6 +175,10 @@ func (p *convPersister) run() {
wrote, err := persistCommitByPath(snaps, res) wrote, err := persistCommitByPath(snaps, res)
p.mu.Lock() p.mu.Lock()
for _, j := range res {
delete(p.inflight, j.key)
delete(p.cancelled, j.key)
}
for _, s := range wrote { for _, s := range wrote {
if s.seq > p.written[s.id] { if s.seq > p.written[s.id] {
p.written[s.id] = s.seq p.written[s.id] = s.seq
@@ -225,6 +238,13 @@ func (p *convPersister) dropToolRes(prefix string) {
} }
func (p *convPersister) dropToolResLocked(prefix string) { func (p *convPersister) dropToolResLocked(prefix string) {
// Déjà pris par l'écrivain : trop tard pour le retirer de la file, pas pour
// l'empêcher d'atteindre la base (voir toolResWanted).
for k := range p.inflight {
if strings.HasPrefix(k, prefix) {
p.cancelled[k] = true
}
}
for k := range p.mem { for k := range p.mem {
if strings.HasPrefix(k, prefix) { if strings.HasPrefix(k, prefix) {
delete(p.mem, k) delete(p.mem, k)
@@ -239,6 +259,17 @@ func (p *convPersister) dropToolResLocked(prefix string) {
p.res = kept p.res = kept
} }
// toolResWanted : ce résultat du lot en cours doit-il encore être écrit ?
// Appelé par l'écrivain DANS sa transaction. Une suppression annule avant
// d'ouvrir la sienne (deleteToolResultsFor) : soit l'écrivain voit
// l'annulation et saute le résultat, soit il l'a déjà écrit et la suppression,
// sérialisée après lui par bbolt, l'efface. Jamais de résultat qui renaît.
func (p *convPersister) toolResWanted(key string) bool {
p.mu.Lock()
defer p.mu.Unlock()
return !p.cancelled[key]
}
// toolResPending relit un résultat pas encore sur disque. // toolResPending relit un résultat pas encore sur disque.
func (p *convPersister) toolResPending(key string) (string, bool) { func (p *convPersister) toolResPending(key string) (string, bool) {
p.mu.Lock() p.mu.Lock()
@@ -338,6 +369,9 @@ func commitPersistBatch(path string, snaps []*convSnap, res []toolResJob) ([]*co
return err return err
} }
for _, j := range res { for _, j := range res {
if !persistQ.toolResWanted(j.key) {
continue // supprimé avec sa discussion pendant que le lot partait
}
enc, err := encode([]byte(j.plain)) enc, err := encode([]byte(j.plain))
if err != nil { if err != nil {
return err return err
+23
View File
@@ -187,6 +187,29 @@ func TestToolResultSupprimeAvantEcriture(t *testing.T) {
} }
} }
// Un résultat que l'écrivain a DÉJÀ pris dans son lot, supprimé avant que ce
// lot n'atteigne la base, ne renaît pas après la suppression. C'était la cause
// des échecs intermittents de TestToolResultsSaveLoadDelete : la suppression ne
// retirait que la file d'attente, et l'écriture en vol passait après elle.
func TestToolResultSupprimePendantEcriture(t *testing.T) {
testHome(t)
entered, release := gatePersist(t)
id := saveToolResult(strings.Repeat("a", 3000))
waitEntered(t, entered) // le lot est pris, pas encore écrit
deleteToolResultsFor(toolResConv(id))
close(release)
persistQ.flush()
if _, ok := loadToolResult(id); ok {
t.Fatal("un résultat supprimé pendant son écriture a été écrit quand même")
}
// L'annulation ne vaut que pour ce lot : un nouveau résultat s'écrit.
again := saveToolResult(strings.Repeat("b", 3000))
persistQ.flush()
if _, ok := loadToolResult(again); !ok {
t.Fatal("annulation restée collée : le résultat suivant n'a pas été écrit")
}
}
// compactLog rend exactement ce que donne la compaction de fin de tour, sans // compactLog rend exactement ce que donne la compaction de fin de tour, sans
// toucher au journal d'origine. // toucher au journal d'origine.
func TestCompactLogPurEgalFinDeTour(t *testing.T) { func TestCompactLogPurEgalFinDeTour(t *testing.T) {
+4
View File
@@ -89,6 +89,10 @@ func deleteToolResultsFor(sid string) {
return return
} }
prefix := sid + "." prefix := sid + "."
// D'abord l'écrivain (chat_persist.go) : ce qui l'attend est jeté, et ce
// qu'il a déjà pris dans son lot est annulé — sans ça, il l'écrirait après
// l'effacement ci-dessous et le résultat renaîtrait. Annulé AVANT notre
// transaction : l'écrivain vérifie dans la sienne, et bbolt les sérialise.
persistQ.dropToolRes(prefix) persistQ.dropToolRes(prefix)
_ = update(bkToolRes, func(b *bolt.Bucket) error { _ = update(bkToolRes, func(b *bolt.Bucket) error {
c := b.Cursor() c := b.Cursor()