3 Commits

Include renames

Author	SHA1	Message	Date
Chris LuandDevin	514c0c1e99	topology: stop a stale duplicate-vid report from re-quarantining the survivor (#12032) ... * topology: drop a duplicate-vid report whose own copy is already gone flagDuplicateVolumeId runs on a goroutine after registration, so by the time it executes the reporter's copy may have been unregistered (a finished move drops the losing copy). The surviving copies are then legitimate, yet the stale report still quarantined them and stored a phantom (node, vid) key for the departed reporter. Skip the marks when the reporter no longer serves the volume under its own layout key. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(topology): cover a stale duplicate report against a lone survivor The existing tests only flag the conflict before the losing copy is unregistered. Run the delayed duplicate check after the reporter's copy is already gone and assert nothing is quarantined and the survivor stays writable, so removing the stale-report guard fails the test outright instead of needing a lucky schedule. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(topology): cover the enforceQuarantine-driven release retry A failed writable push re-arms the record, and the next volume report drains that retry through enforceQuarantine itself - a different entry point than the pulse-driven pass the other tests exercise. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-10-24 07:05:46 -07:00
Chris LuandDevin	3e5aa8c3b3	topology: mark replica quarantined before recording the pair (#11983) ... * 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>	2026-10-24 07:00:45 -07:00
Chris Lu	4d06742664	Detect and quarantine duplicate volume ids (#11763) (#11828) ... * master: detect and quarantine duplicate volume ids across layouts (#11763) * shell: volume.check.duplicate_ids audit command (#11763) * master: persist duplicate-vid quarantine across heartbeats (#11763) * master: quarantine all copies of a duplicated volume id Near-simultaneous registrations let each side quarantine only the other, so arrival order decided which copy stayed writable. A confirmed cross-layout collision means either copy could be the impostor, so flag every colliding copy read-only regardless of registration order. * topology: return a copy from VolumeLayout.Lookup Callers iterating the returned slice raced with registrations and removals replacing the location list in place. * master: release duplicate-vid quarantine when the conflict resolves A cross-node volume.move briefly shows the same id on two nodes under different disk layouts; permanent quarantine would pin a valid target read-only. Keep the first-reported state per quarantined copy and lift the marks once the conflicting copy unregisters, so only a persistent collision stays read-only. * shell: include ec shards in volume.check.duplicate_ids Ec-only volumes share the global id space. Compare shard collections against regular ones while allowing shards and regular copies of the same encoded volume. * master: keep quarantine release current and cover all removal paths Track the latest server-reported read-only flag while quarantined so a release restores the server s true state, and run the release check on collection deletion and data-node removal too, not only explicit volume unregistration. * shell: do not flag ec-only volumes as duplicates An ec-only volume has no regular copy, so its shard collections had nothing to compare against and always read as foreign. * master: quarantine every conflicting layout at the layout level (#11763) - collect all conflicting layouts, not just the first, so a vid spanning three layouts does not leave one writable - mark quarantine on the location list only: folding it into the stored report made the volume digest permanently disagree with the server and forced a full volume list every heartbeat - isAllWritable now honors the location read-only mark so a quarantined copy stays out of writables through resyncs and re-registration - re-check the conflict before keeping marks, and do not clear a node's quarantine while it still holds the vid in another layout - log the full layout keys on both sides of the conflict * master: push duplicate-vid quarantine to the volume server (#11763) A quarantined copy stayed writable on the volume server itself for a client holding an existing file id, since the master-only mark never reached the server's local write path. Quarantine now also calls VolumeMarkReadonly (can_delete so expiry still drains), is re-pushed if the server comes back reporting it writable, and VolumeMarkWritable runs when the conflict resolves and the server had reported writable. * master: retry unenforced quarantine marks and restore server writability on release (#11763) Dial the advertised grpc port, return mark errors, and keep a quarantine entry until the writable push succeeds so a released copy does not stay read-only on the server. * master: treat rp-only layout differences as an in-flight replication change (#11763) volume.configure.replication rolls through nodes one at a time, so a volume legitimately reports the old and new replica placement while it settles. Layouts differing only in rp within the same collection are a supported transition, not a reused id. * shell: group ec shards by collection in the duplicate id audit (#11763) EC shards of one volume legitimately span disk types when DiskTypePrefer spills, so grouping by disk type reports healthy volumes as duplicates.	2026-10-20 07:37:33 -07:00