Files
seaweedfs/weed/storage/volume_mark_writable_test.go
T
9c427b265f volume tier: accept deletes on a volume as soon as it is tiered (#11894)
* volume tier: accept deletes on a volume as soon as it is tiered

volume.tier.upload marks the volume read-only without deletes before the
upload (markVolumeReplicasWritable(..., false, false), i.e. noWriteOrDelete),
and nothing clears that until the volume server restarts, when load() gives
every tiered volume noWriteCanDelete. Every delete in between is refused with
"volume N is read only": the needle stays live in the index, and its bytes in
the remote object, until someone notices.

After the remote file is loaded, the upload handler now gives the volume the
flags a restart would (MarkTieredDeletable: writes refused, deletes accepted),
with two exceptions that keep the current behaviour:
- a replicated volume: the shell copies the uploaded replica's .idx and .vif
  to the other replicas after the upload, and a tombstone written before that
  copy would reach one replica only;
- a volume in I/O quarantine: that flag is not the upload's to lift.

Tests: TestTierUploadAcceptsDeletesOnASingleCopyVolume fails without the
change ("volume 1 is read only") and checks writes stay refused;
TestTierUploadKeepsRefusingDeletesOnAReplicatedVolume; TestMarkTieredDeletable
(quarantine kept).

* volume tier: check the I/O quarantine under noWriteLock

MarkTieredDeletable read the quarantine state before taking noWriteLock.
CollectHeartbeat could quarantine the volume and set noWriteOrDelete in
between, and the upload then cleared that refusal, so a quarantined volume
accepted deletes until the next heartbeat. Take noWriteLock first: the
heartbeat marks the quarantine before it takes noWriteLock, so either the
check sees it or the heartbeat's refusal lands afterwards and stays.

Also leave alone a volume past IoErrorTolerance that the next heartbeat
will quarantine, as the heartbeat itself does.

* volume: read noWriteOrDelete under noWriteLock in DeleteVolumeNeedle

DeleteVolumeNeedle checked v.noWriteOrDelete without the lock that
MarkVolumeReadonly, CollectHeartbeat and now MarkTieredDeletable hold when
they write it, so a delete racing a tier upload was a data race. Copy the
flag under the read lock; the delete path is otherwise unchanged.

* volume tier: mirror the delete acceptance rules in the Rust volume server

The Rust tier upload already promoted a freshly tiered volume to
no-write-can-delete via refresh_remote_write_mode, and did so for every
volume -- including the replicated and I/O-quarantined ones the Go
MarkTieredDeletable deliberately refuses. A live tier-up now goes
through mark_tiered_deletable (replicated volumes and quarantines keep
their upload-time mark until the next load), while load() keeps
promoting every tiered volume unconditionally, as Go does.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: keep a tiered volume delete-only when marked writable

MarkVolumeWritable preserved noWriteCanDelete on a remote-tiered volume
only when it was already set. A tiered volume still holding the
upload-time mark (a replicated volume, or one whose tier upload ran
before MarkTieredDeletable existed) had it clear, so marking writable
published the volume as fully writable while its .dat lives remotely --
writes then failed against a backend that cannot append, after the
master was told the volume is writable again.

load() gives every remote-tiered volume noWriteCanDelete; deriving the
same flag here keeps mark-writable consistent with a restart: "writable"
on a remote volume resolves to can-delete.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume: keep a tiered volume delete-only when marked writable (rust)

set_writable preserved the remote delete-only mode only when the flag was
already set, so a tiered volume holding the upload-time read-only mark
(replicated, or quarantined under mark_tiered_deletable) was published
fully writable with its .dat remote. Derive no_write_can_delete from
has_remote_file() instead, matching load() and the Go MarkVolumeWritable.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* volume tier: serialize the quarantine check with the flag swap (rust)

A heartbeat refusal landing between should_quarantine() and the store
could be clobbered until the next heartbeat re-asserted it — the same
race the Go side closed by checking under noWriteLock. A
compare-exchange gives the same ordering: the swap fails, and the retry
sees the quarantine mark the heartbeat sets first.

* volume tier: MarkTieredDeletable refuses a non-tiered volume

A stray call on a local single-copy volume would pin it
read-only-can-delete. The only caller promotes right after
LoadRemoteFile, but the guard makes the method safe to reuse; the Rust
mark_tiered_deletable gets the same check, though its caller already
only reaches it under has_remote.

---------

Co-authored-by: Javier de la Torre <jatorre@lakehousebox.com>
Co-authored-by: chrislusf <chris.lu@gmail.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-10-21 14:01:10 -07:00

117 lines
4.2 KiB
Go

package storage
import (
"errors"
"os"
"testing"
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
"github.com/seaweedfs/seaweedfs/weed/storage/super_block"
)
// A volume booted with .vif ReadOnly=true used to come back stuck: load()
// opens .idx as O_RDONLY and builds a SortedFileNeedleMap whose Put returns
// os.ErrInvalid, and MarkVolumeWritable only flipped the in-memory flag and
// rewrote .vif. Subsequent writes failed at v.nm.Put.
func TestMarkVolumeWritable_ReopensPersistedReadOnly(t *testing.T) {
dir := t.TempDir()
v, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
if err != nil {
t.Fatalf("create volume: %v", err)
}
if _, _, _, err := v.writeNeedle2(newRandomNeedle(1), true, false, false); err != nil {
t.Fatalf("initial write: %v", err)
}
// Persist read-only state into .vif, then simulate a server restart by
// closing and re-opening the volume from the same directory.
if err := v.PersistReadOnly(true, false); err != nil {
t.Fatalf("persist read-only: %v", err)
}
v.Close()
v2, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
if err != nil {
t.Fatalf("reload volume: %v", err)
}
defer v2.Close()
if !v2.noWriteOrDelete {
t.Fatalf("reloaded volume should be in noWriteOrDelete=true after .vif ReadOnly=true")
}
if _, ok := v2.nm.(*SortedFileNeedleMap); !ok {
t.Fatalf("reloaded readonly volume should use SortedFileNeedleMap, got %T", v2.nm)
}
// Pre-fix behaviour: SortedFileNeedleMap.Put returns os.ErrInvalid even
// once noWriteOrDelete is cleared. Confirm the failure mode the issue
// describes — flipping only the flag is not enough.
v2.noWriteOrDelete = false
_, _, _, writeErr := v2.writeNeedle2(newRandomNeedle(2), true, false, false)
if !errors.Is(writeErr, os.ErrInvalid) {
t.Fatalf("expected write through SortedFileNeedleMap to fail with os.ErrInvalid, got %v", writeErr)
}
v2.noWriteOrDelete = true // restore so reopenIdxForWrite reflects the real entry condition
if err := v2.reopenIdxForWrite(); err != nil {
t.Fatalf("reopenIdxForWrite: %v", err)
}
if _, stillSorted := v2.nm.(*SortedFileNeedleMap); stillSorted {
t.Fatalf("reopenIdxForWrite left SortedFileNeedleMap in place")
}
v2.noWriteOrDelete = false
if _, _, _, err := v2.writeNeedle2(newRandomNeedle(3), true, false, false); err != nil {
t.Fatalf("write after reopen: %v", err)
}
}
// A remote-tiered volume marked writable keeps the delete-only mode load()
// derives — the remote .dat cannot take appends, so "writable" resolves to
// can-delete even when the mark left noWriteOrDelete without noWriteCanDelete.
func TestMarkVolumeWritableOnATieredVolumeKeepsDeletesOnly(t *testing.T) {
b := newLocalDirBackend(t)
registerTestBackend(t, b)
v, _ := tierUpVolumeLive(t, t.TempDir(), 86, b)
defer v.Close()
// the upload-time mark as a refused MarkTieredDeletable leaves it
v.noWriteOrDelete = true
v.noWriteCanDelete = false
location := newTestLocation(10, false, 0)
location.SetVolume(86, v)
s := &Store{Locations: []*DiskLocation{location}}
if err := s.MarkVolumeWritable(86); err != nil {
t.Fatalf("mark writable: %v", err)
}
readOnly, orDelete, canDelete, _ := v.ReadOnlyReasons()
if orDelete || !canDelete || !readOnly {
t.Fatalf("a tiered volume resolves writable to delete-only, got readOnly=%v orDelete=%v canDelete=%v",
readOnly, orDelete, canDelete)
}
}
// reopenIdxForWrite must be a no-op when the volume already has a writable
// needle map — otherwise repeated MarkVolumeWritable calls would churn the
// index file handle for no reason.
func TestReopenIdxForWrite_NoopWhenAlreadyWritable(t *testing.T) {
dir := t.TempDir()
v, err := NewVolume(dir, dir, "", 2, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
if err != nil {
t.Fatalf("create volume: %v", err)
}
defer v.Close()
before := v.nm
if err := v.reopenIdxForWrite(); err != nil {
t.Fatalf("reopenIdxForWrite on writable volume: %v", err)
}
if v.nm != before {
t.Fatalf("reopenIdxForWrite replaced nm on a writable volume (before=%p after=%p)", before, v.nm)
}
}