astelm/minio
1
0
Fork 0
mirror of https://github.com/pgsty/minio.git synced 2026-11-09 17:21:51 +03:00
Code Issues Packages Projects Releases Wiki Activity

test(restore): release the handed-over restore lock before reusing options

Browse Source 
Three restore tests record the options of a POST's background restore
with a stub and restore with them later: TestRestoreAttemptGenerationFencing,
TestExpireRestoredUsesQuorumGeneration and TestRestoreHealNeedsRestoreQuorum.
The options carry the restore lock that POST hands to its worker. The
handler releases that lock only after the stub returns, while the test
proceeds as soon as the stub has signaled. When the test's restore
checked the lock before that release, it kept the lock, and the release
then canceled its context: every drive read failed with "context
canceled" and the restore reported insufficient read quorum.

Under -race this failed 11 times in 60 runs of the three tests on
69a4f938a, in the worker and old-reset modes of the fencing test and in
the multi-pool expiry test.

A new helper, restoreCaptureOptions, releases the lock before it records
the options, so a later restore takes it again, as a queued worker whose
lock ended does. TestRestoreSupersededAttempt already does this. With
the helper, the three tests passed 60 runs under -race.

Production code does not change: the handler's goroutine runs the
restore and releases the lock after it, and a lock lost during a
restore stops it as intended.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-11-06 16:42:01 +08:00
parent 69a4f938a2
commit 54186040ca
1 changed files with 18 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/restore-native-generation_test.go
+18 -12
View File
@@ -98,6 +98,21 @@ func (d restoreGenerationOffline) WriteMetadata(ctx context.Context, orig, bucke
return d.StorageAPI.WriteMetadata(ctx, orig, bucket, object, fi)
}
// restoreCaptureOptions returns a background restore that records its
// options instead of restoring. When the background restore returns, the POST
// handler releases the restore lock it handed over. A test that restores later
// with the recorded options would race that release and could run with a lock
// context canceled under it. So the lock is released here, before the options
// are recorded, and the later restore takes it again, as a queued worker whose
// lock ended does.
func restoreCaptureOptions(captured chan<- ObjectOptions) func(context.Context, string, string, ObjectOptions) error {
return func(_ context.Context, _, _ string, opts ObjectOptions) error {
opts.Transition.owner.release()
captured <- opts
return nil
}
}
// restoreGenerationMetadata returns the version's metadata on each drive.
func restoreGenerationMetadata(t *testing.T, disks []StorageAPI, r *restoreTest, versionID string) []map[string]string {
t.Helper()
@@ -196,10 +211,7 @@ func TestRestoreAttemptGenerationFencing(t *testing.T) {
er.getDisks = func() []StorageAPI { return wrapped }
captured := make(chan ObjectOptions, 2)
layer := newRestoreIsolationLayer(r)
layer.restore = func(_ context.Context, _, _ string, opts ObjectOptions) error {
captured <- opts
return nil
}
layer.restore = restoreCaptureOptions(captured)
var older ObjectOptions
if mode != "worker" {
if rec := restoreIsolationPOST(t, r, layer, oi.VersionID); rec.Code != http.StatusOK {
@@ -541,10 +553,7 @@ func TestExpireRestoredUsesQuorumGeneration(t *testing.T) {
}
captured := make(chan ObjectOptions, 1)
layer := newRestoreIsolationLayer(r)
layer.restore = func(_ context.Context, _, _ string, opts ObjectOptions) error {
captured <- opts
return nil
}
layer.restore = restoreCaptureOptions(captured)
metaWrites.Store(true)
rec := restoreIsolationPOST(t, r, layer, oi.VersionID)
metaWrites.Store(false)
@@ -634,10 +643,7 @@ func TestRestoreHealNeedsRestoreQuorum(t *testing.T) {
versionID = oi.VersionID
captured := make(chan ObjectOptions, 1)
layer := newRestoreIsolationLayer(r)
layer.restore = func(_ context.Context, _, _ string, opts ObjectOptions) error {
captured <- opts
return nil
}
layer.restore = restoreCaptureOptions(captured)
if rec := restoreIsolationPOST(t, r, layer, versionID); rec.Code != http.StatusOK {
t.Fatal(rec.Body.String())
}