Make x/consistent robust to ambiguous update failures and concurrency, and stop Bucket.Attrs mutating a shared Bucket

Aperta
  • Qualcosa non va?
#186 0 commenti 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@ffumero2003 ci sta già lavorando.

Dal 28/10/2026.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
38/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
go
Ambito
backend

Direzione di ricerca

Start with x/consistent/consistent.go:191-214 and :280-283, then trace save at :313-333 and the Mutex methods at :358-385. Read b2/backend.go:450-458, b2/baseline.go:457-463, and b2/b2.go:538-544 to follow bucket update retries and the shared Bucket race. Use the acceptance criteria to define done: transient update failures preserve readable content, cancellation and missing info return safely, and the stated race checks pass.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug

Context

x/consistent builds compare-and-swap objects and a sync.Locker on bucket info and ifRevisionIs. Defects:

  1. An ambiguous bucket update removes the new object. If Writer.Close's b2_update_bucket is applied but its response is lost (408, 500, 502, 503, 504 or a reset), the b2 layer sends it again and gets 409. save reports a conflict; Close then sees the location pointing at its own new object, treats it as another writer's, deletes it and returns a conflict. The group now points at a deleted object, and Operate reads that as empty state.
  2. Mutex.Lock and Unlock panic on any error, including context cancellation.
  3. info returns (nil, nil) when the bucket has no info map, and every caller dereferences the result.
  4. Data races. Group.info writes g.ba and calls Bucket.Attrs, which replaces the shared Bucket.b without synchronization. This also affects plain b2 users who call Attrs (directly or through SetupReplication and ReplicationStatus) while other goroutines use the same *Bucket.
  5. Cleanup deletes ignore errors, and Mutex has no lease, so a stopped holder blocks others until a manual Unlock.
Current behavior

At a5e0d13:

  • x/consistent/consistent.go:191-214: the conflict path deletes the new object; :313-333: save treats any serial change as a conflict.
  • b2/backend.go:450-458: updateBucket runs under withBackoff; base/base.go:94-100: 408, 429, 500, 502, 503 and 504 are retried for non-upload methods (408, 502 and 504 since Backblaze/blazer#167); b2/baseline.go:457-463: the resulting 409 becomes an update conflict.
  • x/consistent/consistent.go:358-385 (panic(err)), :280-283 (return nil, nil); b2/b2.go:538-544 (b.b = bucket.b at :543). b2/replication.go:94, :141 and :145: ReplicationStatus and SetupReplication call Attrs on the buckets they are given, so they share this race.

Reproduced locally at a5e0d13 against an HTTP fake: Mutex(ctx, "lock").Lock() with a cancelled context panicked; NewReader on a bucket without bucketInfo panicked with a nil dereference; four goroutines calling Group.List reported races at b2/b2.go:543 and x/consistent/consistent.go:279. With one injected 408, 500, 502, 503 or 504, Bucket.Update sends b2_update_bucket twice and returns no error; the full item 1 chain follows from the code and was not run end to end.

Expected behavior

A single transient failure never deletes the object the group points to, cancellation returns an error, and concurrent use is race-free.

Acceptance criteria

  • R1. An updateBucket that is applied and then returns 500, 502 or 504 leaves the group pointing at an existing object with the new content, and Close returns nil.
  • R2. LockContext with a cancelled context returns context.Canceled; a bucket without bucketInfo works with NewReader and Operate.
  • R3. go test -race on concurrent Operate, and on Bucket.Attrs alongside uploads, is clean.

In scope

Proposed change
  1. In Writer.Close, treat "location already equals my suffix" as success; delete the new object only when the location belongs to another writer. The root cause is in the b2 package: b2_update_bucket is a conditional write, so after a 500, 502, 504 or lost response it should not be sent again blindly; re-read the bucket and compare the revision and info first, or return the error to the caller.
  2. Add LockContext(ctx) error and UnlockContext(ctx) error; stop Lock/Unlock panicking on cancellation, or document it.
  3. Treat a nil info map as empty.
  4. Guard Group.ba with a mutex, and have Bucket.Attrs return attributes without replacing Bucket.b (or guard it).
  5. Return or retry cleanup delete errors; consider a lease timestamp for Mutex.

Out of scope

  • Unrelated work outside the components and behavior described in In scope.

Verification

Run the repository regression checks with live credentials unset:

unset B2_APPLICATION_KEY_ID B2_APPLICATION_KEY B2_ACCOUNT_ID B2_ACCOUNT_KEY
go build ./...
go vet ./...
go test -race -count=1 ./x/consistent ./b2/...
  • Reproduce an applied bucket update whose response is lost/500/502/504, and verify the group still points at an existing object containing the new content.
  • Exercise cancelled locking and absent bucket info; run the specified concurrent operations under the race detector.
  • The original live tests additionally need BLAZER_TEST_CONSISTENT; report live verification separately and do not equate a skip with a pass.
  • Expected result: the checks above succeed and the observable outcomes in Acceptance criteria are demonstrated. If a check cannot run, report Could not run because: ...; if it starts but fails, report Could not complete because: .... A skipped check is not a pass.

Links

Backblaze/blazer#108 affects the same Update path. Backblaze/blazer#101 is a different replay in the same retry chain (a 401 unauthorized treated as an expired token). Backblaze/blazer#167 (merged, closes #165) made base.Action retry 408, 502 and 504 on non-upload calls, so b2_update_bucket is now also sent again after those codes; the fix for item 1 should cover them as well. The package's live tests are skipped unless BLAZER_TEST_CONSISTENT is set (x/consistent/consistent_test.go:24-31), so CI does not run them.

Lingua principale
Go
Stelle
66
Fork
18
Merge medio
1g 10h
PR unite (30g)
132

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Backblaze/blazer

Tutte le issue di Backblaze/blazer

Issue simili

Altre issue su Go

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.