From f07aabb39f89c6646be14d3129601435331e140d Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 10 Aug 2026 18:45:49 -0700 Subject: [PATCH] filer: repack verifies the entry against the store at commit time The entry lock is filer-local, so a writer on another filer could commit between repack's read and its swap, and repack then restored the old bytes over an acknowledged update. Re-read the entry and revalidate the chunk fingerprint and WORM right before the swap, answering 409 on any change, and build the new entry from the fresh read so concurrent metadata-only updates are carried forward. This shrinks the unguarded window from the whole repack to the commit itself; closing it entirely needs owner routing. --- weed/server/filer_server_format.go | 42 ++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/weed/server/filer_server_format.go b/weed/server/filer_server_format.go index d334ccc05..830570cfa 100644 --- a/weed/server/filer_server_format.go +++ b/weed/server/filer_server_format.go @@ -307,6 +307,7 @@ func (fs *FilerServer) formatRepack(ctx context.Context, w http.ResponseWriter, return } oldChunks := entry.GetChunks() + oldIdentity := formatChunkIdentity(oldChunks) if len(oldChunks) == 0 { writeJsonError(w, r, http.StatusBadRequest, errors.New("entry has no chunks to repack")) return @@ -406,10 +407,41 @@ func (fs *FilerServer) formatRepack(ctx context.Context, w http.ResponseWriter, return } - newEntry := *entry + // The entry lock is filer-local, so a writer on another filer is not + // blocked by it. Re-read from the store and revalidate right before the + // swap: the unguarded window shrinks from the whole repack to this + // commit. Full enforcement needs owner routing. + current, err := fs.filer.FindEntry(ctx, fullPath) + if err != nil { + cleanup() + if errors.Is(err, filer_pb.ErrNotFound) { + writeJsonError(w, r, http.StatusConflict, errors.New("entry was deleted during repack")) + } else { + writeJsonError(w, r, http.StatusInternalServerError, err) + } + return + } + if !bytes.Equal(formatChunkIdentity(current.GetChunks()), oldIdentity) { + cleanup() + writeJsonError(w, r, http.StatusConflict, errors.New("entry changed during repack")) + return + } + if enforced, wormErr := fs.wormEnforcedForEntry(ctx, r.URL.Path); wormErr != nil { + cleanup() + writeJsonError(w, r, http.StatusInternalServerError, wormErr) + return + } else if enforced { + cleanup() + writeJsonError(w, r, http.StatusForbidden, errors.New("cannot repack WORM-enforced entry")) + return + } + + // build from the fresh read so a concurrent metadata-only update on + // another filer is carried forward, not clobbered + newEntry := *current newEntry.Chunks = newChunks newEntry.Extended = make(map[string][]byte) - for k, v := range entry.Extended { + for k, v := range current.Extended { newEntry.Extended[k] = v } newEntry.Extended[format.LayoutKey] = encoded @@ -417,15 +449,15 @@ func (fs *FilerServer) formatRepack(ctx context.Context, w http.ResponseWriter, if len(newEntry.Md5) == 0 { newEntry.Md5 = md5Hash.Sum(nil) } - if err := fs.filer.UpdateEntry(context.WithoutCancel(ctx), entry, &newEntry); err != nil { + if err := fs.filer.UpdateEntry(context.WithoutCancel(ctx), current, &newEntry); err != nil { cleanup() writeJsonError(w, r, http.StatusInternalServerError, err) return } - fs.filer.DeleteChunks(context.WithoutCancel(ctx), fullPath, oldChunks) + fs.filer.DeleteChunks(context.WithoutCancel(ctx), fullPath, current.GetChunks()) // Filer.UpdateEntry only writes the store; notify subscribers (sync, // backup, replication) of the new chunk ids like the gRPC path does. - fs.filer.NotifyUpdateEvent(ctx, entry, &newEntry, true, false, nil) + fs.filer.NotifyUpdateEvent(ctx, current, &newEntry, true, false, nil) writeJsonQuiet(w, r, http.StatusOK, map[string]interface{}{ "name": entry.Name(), "size": size, "extents": len(layout.ExtentSizes), })