fix: recover inconsistent multi-pool copies before acknowledging PUT

Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-10-25 15:56:28 +08:00
parent 6fdea3b8d9
commit f54c04846e
3 changed files with 62 additions and 10 deletions
@@ -136,7 +136,7 @@ func TestPoolsConditionalPutHTTP(t *testing.T) {
opts.MTime = UTCNow().Add(-time.Minute) opts.MTime = UTCNow().Add(-time.Minute)
current := putConsistencyObject(t, z, bucket, object, currentPool, "current", opts) current := putConsistencyObject(t, z, bucket, object, currentPool, "current", opts)
defer conditionalPutPool(t, z, object, target)() defer conditionalPutPool(t, z, object, target)()
idx, err := z.getWritePoolIdx(t.Context(), bucket, object, 11, false) idx, _, err := z.getWritePoolIdx(t.Context(), bucket, object, 11, false, true)
if err != nil || idx != target { if err != nil || idx != target {
t.Fatalf("allocation target=%d: idx=%d err=%v", target, idx, err) t.Fatalf("allocation target=%d: idx=%d err=%v", target, idx, err)
} }
+15 -1
View File
@@ -1,5 +1,19 @@
// Copyright (c) 2026 Feng Ruohang // Copyright (c) 2026 Feng Ruohang
// SPDX-License-Identifier: AGPL-3.0-or-later //
// This file is part of Silo Object Storage stack
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.
package cmd package cmd
+46 -8
View File
@@ -493,6 +493,10 @@ type poolErrs struct {
} }
func (z *erasureServerPools) getPoolInfoExistingWithOpts(ctx context.Context, bucket, object string, opts ObjectOptions) (PoolObjInfo, []poolErrs, error) { func (z *erasureServerPools) getPoolInfoExistingWithOpts(ctx context.Context, bucket, object string, opts ObjectOptions) (PoolObjInfo, []poolErrs, error) {
return z.getPoolInfoExisting(ctx, bucket, object, opts, false)
}
func (z *erasureServerPools) getPoolInfoExisting(ctx context.Context, bucket, object string, opts ObjectOptions, forWrite bool) (PoolObjInfo, []poolErrs, error) {
var noReadQuorumPools []poolErrs var noReadQuorumPools []poolErrs
poolObjInfos := make([]PoolObjInfo, len(z.serverPools)) poolObjInfos := make([]PoolObjInfo, len(z.serverPools))
poolOpts := make([]ObjectOptions, len(z.serverPools)) poolOpts := make([]ObjectOptions, len(z.serverPools))
@@ -520,6 +524,27 @@ func (z *erasureServerPools) getPoolInfoExistingWithOpts(ctx context.Context, bu
} }
wg.Wait() wg.Wait()
if forWrite && !z.SinglePool() {
// Reuse the metadata already read for allocation. A healthy duplicate
// must not hide an unreadable pool that will veto the next GET.
recovery := -1
for i, info := range poolObjInfos {
if info.Err == nil || isErrObjectNotFound(info.Err) || isErrVersionNotFound(info.Err) {
continue
}
rq, inconsistent := info.Err.(InsufficientReadQuorum)
if !inconsistent || rq.Type != RQInconsistentMeta || recovery >= 0 || z.IsSuspended(i) || z.IsPoolRebalancing(i) {
return info, nil, info.Err
}
recovery = i
}
if recovery >= 0 {
// Repair the sole unreadable copy in place. One write cannot repair
// several pools, and draining pools cannot accept recovery writes.
return poolObjInfos[recovery], nil, nil
}
}
// Sort the objInfos such that we always serve latest // Sort the objInfos such that we always serve latest
// this is a defensive change to handle any duplicate // this is a defensive change to handle any duplicate
// content that may have been created, we always serve // content that may have been created, we always serve
@@ -636,20 +661,21 @@ func (z *erasureServerPools) getPoolIdxNoLock(ctx context.Context, bucket, objec
// if none are found falls back to most available space pool, this function is // if none are found falls back to most available space pool, this function is
// designed to be only used by PutObject, CopyObject (newObject creation) and NewMultipartUpload. // designed to be only used by PutObject, CopyObject (newObject creation) and NewMultipartUpload.
func (z *erasureServerPools) getPoolIdx(ctx context.Context, bucket, object string, size int64) (idx int, err error) { func (z *erasureServerPools) getPoolIdx(ctx context.Context, bucket, object string, size int64) (idx int, err error) {
return z.getWritePoolIdx(ctx, bucket, object, size, false) idx, _, err = z.getWritePoolIdx(ctx, bucket, object, size, false, false)
return idx, err
} }
// getWritePoolIdx keeps the write-allocation policy when the caller already // getWritePoolIdx keeps the write-allocation policy when the caller already
// holds the pools-layer object lock and must not reacquire a set read lock. // holds the pools-layer object lock and must not reacquire a set read lock.
func (z *erasureServerPools) getWritePoolIdx(ctx context.Context, bucket, object string, size int64, noLock bool) (idx int, err error) { func (z *erasureServerPools) getWritePoolIdx(ctx context.Context, bucket, object string, size int64, noLock, recoverWrite bool) (idx int, recovery bool, err error) {
pinfo, _, err := z.getPoolInfoExistingWithOpts(ctx, bucket, object, ObjectOptions{ pinfo, _, err := z.getPoolInfoExisting(ctx, bucket, object, ObjectOptions{
NoLock: noLock, NoLock: noLock,
SkipDecommissioned: true, SkipDecommissioned: true,
SkipRebalancing: true, SkipRebalancing: true,
}) }, recoverWrite)
if err != nil && !isErrObjectNotFound(err) { if err != nil && !isErrObjectNotFound(err) {
return -1, err return -1, false, err
} }
idx = pinfo.Index idx = pinfo.Index
@@ -657,11 +683,11 @@ func (z *erasureServerPools) getWritePoolIdx(ctx context.Context, bucket, object
// will generate a temp object // will generate a temp object
idx = z.getAvailablePoolIdx(ctx, bucket, object, size) idx = z.getAvailablePoolIdx(ctx, bucket, object, size)
if idx < 0 { if idx < 0 {
return -1, toObjectErr(errDiskFull) return -1, false, toObjectErr(errDiskFull)
} }
} }
return idx, nil return idx, recoverWrite && isErrReadQuorum(pinfo.Err), nil
} }
func (z *erasureServerPools) Shutdown(ctx context.Context) error { func (z *erasureServerPools) Shutdown(ctx context.Context) error {
@@ -1203,7 +1229,8 @@ func (z *erasureServerPools) PutObject(ctx context.Context, bucket string, objec
opts.replicaObjectInfo = resolve opts.replicaObjectInfo = resolve
} }
idx, err := z.getWritePoolIdx(ctx, bucket, object, data.Size(), true) recoverWrite := !opts.ReplicationRequest && !opts.ReplicaLockReconcile && !opts.DataMovement
idx, recovery, err := z.getWritePoolIdx(ctx, bucket, object, data.Size(), true, recoverWrite)
if err != nil { if err != nil {
return ObjectInfo{}, err return ObjectInfo{}, err
} }
@@ -1226,6 +1253,17 @@ func (z *erasureServerPools) PutObject(ctx context.Context, bucket string, objec
opts.replicaObjectInfo = z.replicaObjectInfo opts.replicaObjectInfo = z.replicaObjectInfo
} }
oi, err := z.serverPools[idx].PutObject(ctx, bucket, object, data, opts) oi, err := z.serverPools[idx].PutObject(ctx, bucket, object, data, opts)
if err == nil && recovery {
// Only recovery pays for a second read. A partial repair or a new
// unreadable pool must not be acknowledged as recovered. Keep any
// committed write on failure; a read failure is not permission to undo.
var latest ObjectInfo
var latestIdx int
latest, latestIdx, err = z.getLatestObjectInfoWithIdx(ctx, bucket, object, ObjectOptions{NoLock: true, NoAuditLog: true})
if err == nil && (latestIdx != idx || latest.VersionID != oi.VersionID || latest.ETag != oi.ETag || !latest.ModTime.Equal(oi.ModTime)) {
err = InsufficientReadQuorum{Bucket: bucket, Object: decodeDirObject(object), Err: errErasureReadQuorum, Type: RQInconsistentMeta}
}
}
if err == nil && opts.ReplicaLockReconcile && !opts.DataMovement { if err == nil && opts.ReplicaLockReconcile && !opts.DataMovement {
err = z.retireReplicaCopies(ctx, bucket, object, idx, oi) err = z.retireReplicaCopies(ctx, bucket, object, idx, oi)
} }