volume: test that only the disk holding the replaced replica gets the free-slot credit (#11486)

* volume server: split volume_copy into phases and type the delete-after-status gate

volume_copy was one ~400-line handler, and the rule that an existing
local replica is deleted only after the source's ReadVolumeFileStatus
succeeded was held by statement order alone. The keep_remote_data=true
that the pre-copy delete and the failed-copy rollback must share was
kept in sync by a comment pointing from one to the other.

The handler is now a ~60-line orchestrator over connect_to_copy_source,
SourceVolumeStatus::fetch, delete_existing_replica,
plan_copy_destination and a VolumeCopyJob whose run() drives
preallocate_dat, transfer_files, finish_copied_files and
mount_and_reply, with cleanup_failed_copy on error.
delete_existing_replica takes a &SourceVolumeStatus, which only fetch
can construct (private field in a child module), so the delete cannot
be called before the status RPC. Both deletes go through
delete_replica_keep_remote.

Pure refactor: call order, status codes and messages, cancellation
checks, throttling, progress reports and cleanup are unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* volume server: find space for a VolumeCopy before deleting the replica it replaces

VolumeCopy deleted an existing local replica as soon as the source answered
ReadVolumeFileStatus and only then looked for a location with room for the
copy. With no usable location (disk full, low-disk, wrong disk type) the call
errored after the delete, leaving the node with neither the old replica nor
the new one.

Plan the destination first, as Go does: find_free_location_replacing credits
the location holding the replaced volume with that volume's slot, so a disk at
its volume limit that holds the replica still accepts the copy. Only then
delete the replica and write the .note (still after the delete, as in Go).
delete_existing_replica now takes the planned CopyDestination, so the delete
cannot precede the plan. find_free_location_predicate keeps its behaviour.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* volume: describe the replace-credit test against the current VolumeCopy flow

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
This commit is contained in:
authored and GitHub committed 2026-10-01 23:13:17 +08:00
1 parent 90f8c4378f
commit 8a9563e53d
1 file changed
+46
+46
View File
@@ -2483,6 +2483,52 @@ mod tests {
assert_eq!(selected, Some(0));
}
// VolumeCopy picks a disk before deleting the replica it replaces, so only
// the location actually holding that replica may count its slot as free.
#[test]
fn test_find_free_location_predicate_credits_only_the_holding_location() {
let tmp1 = TempDir::new().unwrap();
let dir1 = tmp1.path().to_str().unwrap();
let tmp2 = TempDir::new().unwrap();
let dir2 = tmp2.path().to_str().unwrap();
let mut store = Store::new(NeedleMapKind::InMemory);
for dir in [dir1, dir2] {
store
.add_location(
dir,
dir,
1,
DiskType::HardDrive,
MinFreeSpace::Percent(0.0),
Vec::new(),
)
.unwrap();
}
for vid in [81, 82] {
store
.add_volume(VolumeId(vid), DiskType::HardDrive, &VolumeSpec::default())
.unwrap();
}
let loc_of = |vid| store.find_volume(VolumeId(vid)).unwrap().0;
assert_ne!(loc_of(81), loc_of(82), "fixture must fill both locations");
let hdd = |loc: &DiskLocation| loc.disk_type == DiskType::HardDrive;
assert_eq!(store.find_free_location_predicate(hdd, None), None);
assert_eq!(
store.find_free_location_predicate(hdd, Some(VolumeId(99))),
None
);
assert_eq!(
store.find_free_location_predicate(hdd, Some(VolumeId(81))),
Some(loc_of(81))
);
assert_eq!(
store.find_free_location_predicate(hdd, Some(VolumeId(82))),
Some(loc_of(82))
);
}
#[test]
fn test_delete_expired_ec_volumes_removes_expired_entries() {
let tmp = TempDir::new().unwrap();