fix(listing): preserve readable keys across pool and metadata disagreements

Continue PR #221 while retaining its original commits and authorship.
Route latest-only fallback reads through the namespace lock used by normal
writes, propagate terminal scan errors before cache completion, and retain
delete markers until the cross-pool merge has resolved them.

Recount narrowly eligible latest null-version metadata when mixed erasure
layouts prevent selection. Keep each EC group separate and preserve the
requested quorum, strict resolution, and version-listing behavior.

Add regression coverage for locking, terminal errors, cache completion,
delete markers, mixed EC layouts, and HTTP behavior; run focused race tests
in CI. Verified with make test, focused race tests, and local distributed
single- and multi-pool rolling-restart workloads with exact final oracles.

Refs: https://github.com/pgsty/silo/issues/218
Refs: https://github.com/pgsty/silo/pull/221
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-10-16 00:58:46 +08:00
19 changed files with 1775 additions and 46 deletions
+406
View File
@@ -52,6 +52,37 @@ type nullQuorumWalkDisk struct {
index int
}
type failedMetadataWalkDisk struct {
StorageAPI
}
func (d *failedMetadataWalkDisk) WalkDir(context.Context, WalkDirOptions, io.Writer) error {
return errDiskNotFound
}
// offlineNullQuorumDisk models a drive of a node that is away.
type offlineNullQuorumDisk struct {
StorageAPI
}
func (d *offlineNullQuorumDisk) IsOnline() bool { return false }
func (d *offlineNullQuorumDisk) DiskInfo(context.Context, DiskInfoOptions) (DiskInfo, error) {
return DiskInfo{}, errDiskNotFound
}
func (d *offlineNullQuorumDisk) WalkDir(context.Context, WalkDirOptions, io.Writer) error {
return errDiskNotFound
}
func (d *offlineNullQuorumDisk) ReadXL(context.Context, string, string, bool) (RawFileInfo, error) {
return RawFileInfo{}, errDiskNotFound
}
func (d *offlineNullQuorumDisk) ReadVersion(context.Context, string, string, string, string, ReadOptions) (FileInfo, error) {
return FileInfo{}, errDiskNotFound
}
func (d *nullQuorumWalkDisk) WalkDir(ctx context.Context, opts WalkDirOptions, out io.Writer) error {
if opts.Bucket != d.schedule.bucket {
return d.StorageAPI.WalkDir(ctx, opts, out)
@@ -119,6 +150,30 @@ func nullQuorumRequest(t *testing.T, router http.Handler, method, target string)
return rec
}
func nullQuorumPut(t *testing.T, z *erasureServerPools, bucket, object string, body []byte, opts ObjectOptions) {
t.Helper()
if _, err := z.PutObject(t.Context(), bucket, object, mustGetPutObjReader(t, bytes.NewReader(body), int64(len(body)), "", ""), opts); err != nil {
t.Fatal(err)
}
}
func nullQuorumListV2Keys(t *testing.T, router http.Handler, bucket string) (int, []string) {
t.Helper()
rec := nullQuorumRequest(t, router, http.MethodGet, getListObjectsV2URL("", bucket, "", "1000", "", "", ""))
if rec.Code != http.StatusOK {
return rec.Code, nil
}
var list ListObjectsV2Response
if err := xml.Unmarshal(rec.Body.Bytes(), &list); err != nil {
t.Fatal(err)
}
keys := make([]string, 0, len(list.Contents))
for _, object := range list.Contents {
keys = append(keys, object.Key)
}
return rec.Code, keys
}
func TestListObjectsSingleNullQuorumHTTP(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
oldBody := bytes.Repeat([]byte{'a'}, 8192)
@@ -227,6 +282,357 @@ func TestListObjectsSingleNullQuorumHTTP(t *testing.T) {
}
}
func TestListObjectsFallsBackToReadableMetadata(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
set := z.serverPools[0].sets[0]
disks := set.getDisks()
const object = "object"
oldBody := bytes.Repeat([]byte{'a'}, 8192)
newBody := bytes.Repeat([]byte{'b'}, 8192)
oldTime := time.Now().UTC().Add(-time.Hour)
if _, err := z.PutObject(t.Context(), bucket, object, mustGetPutObjReader(t, bytes.NewReader(oldBody), int64(len(oldBody)), "", ""), ObjectOptions{MTime: oldTime}); err != nil {
t.Fatal(err)
}
oldMeta := make([][]byte, len(disks))
for i, disk := range disks {
oldMeta[i] = mustReadNullQuorumMeta(t, disk, bucket, object)
}
if _, err := z.PutObject(t.Context(), bucket, object, mustGetPutObjReader(t, bytes.NewReader(newBody), int64(len(newBody)), "", ""), ObjectOptions{MTime: oldTime.Add(time.Minute)}); err != nil {
t.Fatal(err)
}
// The eight readable walk streams are split 3/5 across two generations;
// the other eight streams fail. Neither generation is proven readable by
// the listing inputs, so returning HTTP 200 would silently omit the key.
for i := range 3 {
if err := disks[i].WriteAll(t.Context(), bucket, object+"/"+xlStorageFormatFile, oldMeta[i]); err != nil {
t.Fatal(err)
}
}
wrapped := make([]StorageAPI, len(disks))
for i, disk := range disks {
wrapped[i] = disk
if i >= 8 {
wrapped[i] = &failedMetadataWalkDisk{StorageAPI: disk}
}
}
set.getDisks = func() []StorageAPI { return wrapped }
rec := nullQuorumRequest(t, router, http.MethodGet, getListObjectsV2URL("", bucket, "", "1000", "", "", ""))
var list ListObjectsV2Response
if rec.Code != http.StatusOK || xml.Unmarshal(rec.Body.Bytes(), &list) != nil {
t.Fatalf("LIST failed to resolve readable metadata: %d %s", rec.Code, rec.Body.String())
}
if list.KeyCount != 1 || len(list.Contents) != 1 || list.Contents[0].Key != object {
t.Fatalf("LIST omitted the readable key: %+v", list)
}
// The object itself still has a read quorum outside the failed walk
// streams, demonstrating why the LIST must report uncertainty, not absence.
get := nullQuorumRequest(t, router, http.MethodGet, getGetObjectURL("", bucket, object))
if get.Code != http.StatusOK || !bytes.Equal(get.Body.Bytes(), newBody) {
t.Fatalf("GET lost the readable generation: %d %q", get.Code, get.Body.String())
}
}
// The fallback must read under the object read lock. A lock-free read of an
// object being overwritten stably reports a live object as absent, which is
// the omission this repair exists to remove. Hold the write lock and assert
// the listing waits for it rather than resolving straight through.
func TestListFallbackTakesObjectReadLock(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
set := z.serverPools[0].sets[0]
disks := set.getDisks()
const object = "object"
oldBody, newBody := bytes.Repeat([]byte{'a'}, 8192), bytes.Repeat([]byte{'b'}, 8192)
oldTime := time.Now().UTC().Add(-time.Hour)
if _, err := z.PutObject(t.Context(), bucket, object, mustGetPutObjReader(t, bytes.NewReader(oldBody), int64(len(oldBody)), "", ""), ObjectOptions{MTime: oldTime}); err != nil {
t.Fatal(err)
}
oldMeta := make([][]byte, len(disks))
for i, disk := range disks {
oldMeta[i] = mustReadNullQuorumMeta(t, disk, bucket, object)
}
if _, err := z.PutObject(t.Context(), bucket, object, mustGetPutObjReader(t, bytes.NewReader(newBody), int64(len(newBody)), "", ""), ObjectOptions{MTime: oldTime.Add(time.Minute)}); err != nil {
t.Fatal(err)
}
// Split the readable walk streams so the entry cannot be resolved from
// the snapshots and the fallback has to run.
for i := range 3 {
if err := disks[i].WriteAll(t.Context(), bucket, object+"/"+xlStorageFormatFile, oldMeta[i]); err != nil {
t.Fatal(err)
}
}
wrapped := make([]StorageAPI, len(disks))
for i, disk := range disks {
wrapped[i] = disk
if i >= 8 {
wrapped[i] = &failedMetadataWalkDisk{StorageAPI: disk}
}
}
set.getDisks = func() []StorageAPI { return wrapped }
lock := set.NewNSLock(bucket, object)
lkctx, err := lock.GetLock(t.Context(), globalOperationTimeout)
if err != nil {
t.Fatal(err)
}
locked := true
defer func() {
if locked {
lock.Unlock(lkctx)
}
}()
target := getListObjectsV2URL("", bucket, "", "1000", "", "", "")
req, err := newTestSignedRequestV4(http.MethodGet, target, 0, nil, globalActiveCred.AccessKey, globalActiveCred.SecretKey, nil)
if err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
done := make(chan struct{})
go func() {
defer close(done)
router.ServeHTTP(rec, req)
}()
waitForListingReadLock(t, set.nsMutex, bucket, object, done)
lock.Unlock(lkctx)
locked = false
<-done
var list ListObjectsV2Response
if rec.Code != http.StatusOK || xml.Unmarshal(rec.Body.Bytes(), &list) != nil {
t.Fatalf("LIST failed after the lock was released: %d %s", rec.Code, rec.Body.String())
}
if list.KeyCount != 1 || len(list.Contents) != 1 || list.Contents[0].Key != object {
t.Fatalf("LIST omitted the readable key: %+v", list)
}
}
// A truncated xl.meta is what a concurrent overwrite looks like on one drive,
// and the walker skips that entry. Skipping is safe and deliberate: the key is
// only omitted when no generation reaches quorum, which listPath now resolves
// against the read path. Pin that, so the walker is not "hardened" into
// failing a whole listing over a minority of unreadable drives.
func TestListObjectsSurvivesTruncatedMetadataMinority(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
set := z.serverPools[0].sets[0]
disks := set.getDisks()
const object = "object"
body := bytes.Repeat([]byte{'a'}, 8192)
if _, err := z.PutObject(t.Context(), bucket, object, mustGetPutObjReader(t, bytes.NewReader(body), int64(len(body)), "", ""), ObjectOptions{}); err != nil {
t.Fatal(err)
}
// Truncate xl.meta on a minority of drives, as an overwrite in flight does.
for i := range 4 {
meta := mustReadNullQuorumMeta(t, disks[i], bucket, object)
if err := disks[i].WriteAll(t.Context(), bucket, object+"/"+xlStorageFormatFile, meta[:len(meta)/2]); err != nil {
t.Fatal(err)
}
}
rec := nullQuorumRequest(t, router, http.MethodGet, getListObjectsV2URL("", bucket, "", "1000", "", "", ""))
var list ListObjectsV2Response
if rec.Code != http.StatusOK || xml.Unmarshal(rec.Body.Bytes(), &list) != nil {
t.Fatalf("a truncated minority failed the listing: %d %s", rec.Code, rec.Body.String())
}
if list.KeyCount != 1 || len(list.Contents) != 1 || list.Contents[0].Key != object {
t.Fatalf("a truncated minority omitted the key: %+v", list)
}
}
func TestListStaleMinorityIgnored(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
disks := z.serverPools[0].sets[0].getDisks()
body := bytes.Repeat([]byte{'a'}, 8192)
nullQuorumPut(t, z, bucket, "gone", body, ObjectOptions{})
nullQuorumPut(t, z, bucket, "keep", body, ObjectOptions{})
for _, disk := range disks[4:] {
if err := disk.Delete(t.Context(), bucket, "gone", DeleteOptions{Recursive: true, Immediate: true}); err != nil {
t.Fatal(err)
}
}
if code, keys := nullQuorumListV2Keys(t, router, bucket); code != http.StatusOK || !slices.Equal(keys, []string{"keep"}) {
t.Errorf("ListObjectsV2: got %d %v; want 200 [keep]", code, keys)
}
results := make(chan itemOrErr[ObjectInfo], 16)
if err := z.Walk(t.Context(), bucket, "", results, WalkOptions{}); err != nil {
t.Fatal(err)
}
var names []string
for result := range results {
if result.Err != nil {
t.Fatal(result.Err)
}
names = append(names, result.Item.Name)
}
if !slices.Equal(names, []string{"keep"}) {
t.Errorf("Walk: got %v; want [keep]", names)
}
rec := nullQuorumRequest(t, router, http.MethodGet, getListObjectVersionsURL("", bucket, "", "1000", ""))
var list struct {
Versions []struct {
Key string `xml:"Key"`
} `xml:"Version"`
}
if rec.Code != http.StatusOK {
t.Errorf("ListObjectVersions: got %d; want 200", rec.Code)
} else if err := xml.Unmarshal(rec.Body.Bytes(), &list); err != nil {
t.Fatal(err)
} else if len(list.Versions) != 1 || list.Versions[0].Key != "keep" {
t.Errorf("ListObjectVersions: got %+v; want [keep]", list.Versions)
}
if _, err := z.DeleteObject(t.Context(), bucket, "keep", ObjectOptions{}); err != nil {
t.Fatal(err)
}
if err := z.DeleteBucket(t.Context(), bucket, DeleteBucketOptions{}); err != nil {
t.Errorf("DeleteBucket with only a stale minority: %v", err)
}
}
func TestIAMListingIgnoresStaleMinority(t *testing.T) {
z, _, _ := nullQuorumBackend(t)
body := []byte(`{"version":1}`)
alive := iamConfigUsersPrefix + "alive/identity.json"
ghost := iamConfigUsersPrefix + "ghost/identity.json"
nullQuorumPut(t, z, minioMetaBucket, alive, body, ObjectOptions{})
nullQuorumPut(t, z, minioMetaBucket, ghost, body, ObjectOptions{})
for _, disk := range z.serverPools[0].getHashedSet(ghost).getDisks()[4:] {
if err := disk.Delete(t.Context(), minioMetaBucket, ghost, DeleteOptions{Recursive: true, Immediate: true}); err != nil {
t.Fatal(err)
}
}
items, err := newIAMObjectStore(z, MinIOUsersSysType).listAllIAMConfigItems(t.Context())
if err != nil {
t.Fatalf("IAM listing failed on a stale minority (retriable=%v): %v", configRetriableErrors(err), err)
}
if got := items["users/"]; !slices.Equal(got, []string{"alive/identity.json"}) {
t.Errorf("IAM users: got %v; want [alive/identity.json]", got)
}
}
func TestListSplitDirectoryObject(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
set := z.serverPools[0].sets[0]
disks := set.getDisks()
const object = "dir/"
encoded := encodeDirObject(object)
mtime := time.Now().UTC().Add(-time.Hour)
nullQuorumPut(t, z, bucket, object, nil, ObjectOptions{MTime: mtime})
old := make([][]byte, 3)
for i := range old {
old[i] = mustReadNullQuorumMeta(t, disks[i], bucket, encoded)
}
nullQuorumPut(t, z, bucket, object, nil, ObjectOptions{MTime: mtime.Add(time.Minute)})
for i := range old {
if err := disks[i].WriteAll(t.Context(), bucket, encoded+"/"+xlStorageFormatFile, old[i]); err != nil {
t.Fatal(err)
}
}
walkers := slices.Clone(disks)
for i := 8; i < len(walkers); i++ {
walkers[i] = &failedMetadataWalkDisk{StorageAPI: disks[i]}
}
set.getDisks = func() []StorageAPI { return walkers }
if code, keys := nullQuorumListV2Keys(t, router, bucket); code != http.StatusOK || !slices.Equal(keys, []string{object}) {
t.Errorf("ListObjectsV2: got %d %v; want 200 [%s]", code, keys, object)
}
}
func TestListFallbackDoesNotTrustNotFound(t *testing.T) {
z, bucket, _ := nullQuorumBackend(t)
set := z.serverPools[0].sets[0]
disks := set.getDisks()
const object = "object"
body := bytes.Repeat([]byte{'a'}, 8192)
mtime := time.Now().UTC().Add(-time.Hour)
nullQuorumPut(t, z, bucket, object, body, ObjectOptions{MTime: mtime})
old := make([][]byte, len(disks))
for i := range disks {
old[i] = mustReadNullQuorumMeta(t, disks[i], bucket, object)
}
nullQuorumPut(t, z, bucket, object, body, ObjectOptions{MTime: mtime.Add(time.Minute)})
newer := make([][]byte, len(disks))
for i := range disks {
newer[i] = mustReadNullQuorumMeta(t, disks[i], bucket, object)
if err := disks[i].Delete(t.Context(), bucket, object, DeleteOptions{Recursive: true, Immediate: true}); err != nil {
t.Fatal(err)
}
}
entries := make(metaCacheEntries, len(disks))
errs := make([]error, len(disks))
for i := range 3 {
entries[i] = metaCacheEntry{name: object, metadata: old[i]}
}
for i := 3; i < 8; i++ {
entries[i] = metaCacheEntry{name: object, metadata: newer[i]}
}
for i := 8; i < len(errs); i++ {
errs[i] = errDiskNotFound
}
resolver := metadataResolutionParams{dirQuorum: 8, objQuorum: 8, bucket: bucket, requestedVersions: 1}
entry, err := set.resolveListEntry(t.Context(), bucket, entries, errs, &resolver, z.listingNSLock(set))
if entry != nil || !isErrReadQuorum(err) {
t.Fatalf("ambiguous not-found: got entry=%v err=%v; want read quorum error", entry, err)
}
}
// G0 was written 8+8 while a node was away. G1 was later written 12+4 but
// missed drive 0, which still holds G0. Then a node holding four G1 drives
// goes away: eleven G1 and one G0 remain online. G1 exists but lacks one of
// its twelve data shards until the node returns, so HEAD must not report 404
// and LIST must still show the key.
func TestListCrossParityMinorityStaysVisible(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
set := z.serverPools[0].sets[0]
disks := set.getDisks()
const object = "object"
mtime := time.Now().UTC().Add(-time.Hour)
nullQuorumPut(t, z, bucket, object, bytes.Repeat([]byte{'a'}, 256<<10), ObjectOptions{MTime: mtime, MaxParity: true})
g0 := mustReadNullQuorumMeta(t, disks[0], bucket, object)
var x xlMetaV2
if err := x.LoadOrConvert(g0); err != nil {
t.Fatal(err)
}
if h := x.versions[0].header; h.EcM != 8 || h.EcN != 8 {
t.Fatalf("G0 is %d+%d, want 8+8", h.EcM, h.EcN)
}
g1Body := bytes.Repeat([]byte{'b'}, 256<<10)
nullQuorumPut(t, z, bucket, object, g1Body, ObjectOptions{MTime: mtime.Add(time.Minute)})
if err := disks[0].WriteAll(t.Context(), bucket, object+"/"+xlStorageFormatFile, g0); err != nil {
t.Fatal(err)
}
online := slices.Clone(disks)
for i := 12; i < len(online); i++ {
online[i] = &offlineNullQuorumDisk{StorageAPI: disks[i]}
}
set.getDisks = func() []StorageAPI { return online }
if code := nullQuorumRequest(t, router, http.MethodHead, getHeadObjectURL("", bucket, object)).Code; code == http.StatusNotFound {
t.Errorf("HEAD with a node away: 404 for an existing object")
}
if code, keys := nullQuorumListV2Keys(t, router, bucket); code != http.StatusOK || !slices.Equal(keys, []string{object}) {
t.Errorf("ListObjectsV2 with a node away: got %d %v; want 200 [%s]", code, keys, object)
}
set.getDisks = func() []StorageAPI { return disks }
if get := nullQuorumRequest(t, router, http.MethodGet, getGetObjectURL("", bucket, object)); get.Code != http.StatusOK || !bytes.Equal(get.Body.Bytes(), g1Body) {
t.Fatalf("GET after the node returns: %d", get.Code)
}
}
func TestSingleNullQuorumReadAndConsumers(t *testing.T) {
z, bucket, router := nullQuorumBackend(t)
set := z.serverPools[0].sets[0]