mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-25 14:30:43 +01:00
Chris LuandDevin
3e5aa8c3b3
* topology: serialize quarantine records with their layout marks A quarantine publishes a (node, vid) record and marks the replica read-only at the layout level; release and heartbeat flag updates must observe that pair as one state. Store-then-mark let a concurrent release delete the record before the mark landed, stranding the surviving replica read-only; mark-then-store let a heartbeat clear the mark before the record was visible, returning a conflicting replica to the writable list. quarantineLock now covers every record<->mark pair: flagDuplicateVolumeId's quarantine, releaseResolvedQuarantine's undo, the isQuarantined-gated marks on registration, and the quarantine-folded read-only flag writes on heartbeat paths. Remote server pushes stay outside the lock; a push that lands after the record was released is undone, and a failed release push re-adds the record for a later retry. The release test now waits for in-flight async flags to settle instead of asserting on a single read. * topology: fold quarantine into RegisterVolume's flag rebuild RegisterVolume rewrites every replica's location read-only flag from what its node last reported, and reported flags deliberately exclude quarantine. Any registration of a shared vid -- a reconnected replica, a new copy, the self-heal and delta-heartbeat re-register paths -- therefore cleared the quarantine mark on every replica in the location, while the gate afterwards only re-marked the registering node. Two call sites had no gate at all. RegisterVolume now takes an isQuarantined predicate and evaluates it under accessLock for every replica it touches, so the mark is never dropped in the first place: a release unmarks after the rebuild, and a quarantine mark re-applies on the next rebuild, with no window between. * topology: keep quarantine when a vacuum commit re-adds a replica SetVolumeAvailable re-adds the replica into the location list; a re-added slot starts with a cleared read-only flag, so a quarantined replica came back flag-free and the next ensureCorrectWritables could return a conflicting volume to the writable list. Pass the quarantine state in and set the flag back under the same accessLock, with the caller holding quarantineLock so release cannot interleave between the check and the mark. * topology: merge quarantine records instead of overwriting them enforceQuarantine wrote back the state copy it loaded before its remote push, so a concurrent writer's enforced=true was lost: the server stayed read-only while the record said unenforced, and a later release skipped the writable push it owed. The write-back now reloads the record under quarantineLock and merges -- enforced wins from either side, reported is taken from the newer pre-enforcement observation only while no push has landed yet. All three compensating writable pushes had the same hole in reverse: a failed un-quarantine left the server read-only with no record to retry from, and release's restore blindly overwrote any record a fresh flag pass had created in between. They now go through restoreQuarantineRecord, which adds the record only when absent and otherwise sets enforced on the current one, preserving its reported flag. * topology: hold quarantineLock over registration's quarantine check-and-mark RegisterVolume reads the quarantine record and marks the replica read-only under accessLock, but the pair ran without quarantineLock. A release could push writable, watch the record get re-read, and then delete it: the replica stayed read-only with no record left to clear, and unchanged writable heartbeats skip SetReplicaReadOnlyFlag so the mark never repaired. Holding quarantineLock around the check-and-mark in all three production callers serializes it against the release clear-and-delete. * topology: serialize remote quarantine pushes behind a record recheck releaseResolvedQuarantine deletes the record under quarantineLock and sends the writable request after unlock. A conflict flagged while that request is in flight creates a fresh record and marks the replica read-only again, and the older writable request can still land last, leaving the server writable under an active quarantine. The reverse flap strands a stale read-only mark the same way. Route every QuarantineVolumeFn call through pushQuarantine, which holds quarantinePushLock across the remote call and re-checks the record under quarantineLock right before sending: a read-only push requires the pair to exist, a writable push requires it to be gone. A skipped push means the decision already moved on, so the final server state always follows the newest record. * topology: serialize quarantine pushes per replica, not globally A single quarantinePushLock held across the remote mark call let one slow volume server (up to the 10s client timeout) delay pushes for unrelated replicas. Keying the lock by (node, vid) keeps pushes to one replica ordered in record-decision order while pushes to others run in parallel. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * topology: restore the layout read-only mark with a released record A failed writable push re-armed the quarantine record but left the replica marked writable in the layout, so PickForWrite could route to a replica the server still holds read-only. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * topology: wait out async duplicate checks in the release-retry test TestFailedReleasePushRestoresQuarantineMark asserts layout marks around a release pass, but the two RegisterVolumeLayout calls each start a background flagDuplicateVolumeId that can still be mid-flight. A late mark could land after the final release and fail the writable assertion even though the code behaved correctly. Track the async passes in a WaitGroup and drain them before the explicit flag call so the rest of the test is deterministic. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * topology: keep quarantine when a replica's layout key is unreadable * topology: keep a failed writable push's restore ordered under the push lock * topology: retry the quarantine release on every duplicate check * ci: isolate Java and S3 test servers on persistent runners * ci: drop the duplicated -ip flags in the lifecycle test server The bind flags were already on the command; the isolation pass added a second pair. * topology: retry a failed quarantine writable push on the next report * topology: retry pending quarantine releases off the leader pulse A failed writable push re-arms the quarantine record with a retry that only enforceQuarantine fires, and that runs only for volumes a heartbeat carries. Once the server has reported the replica read-only it drops out of the delta reports, and restoreQuarantineRecord keeps the stored flag matching the server's so the digests agree and no full list is requested - the pending release then never retries and the idle volume stays read-only forever. Run the same release pass from the leader's pulse loop so a pending retry no longer waits on a report. * topology: exercise the pulse-driven quarantine release retry --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>