docs(parity): correct delete-delay budget, fuzzy, and test-review gaps
- --delete-delay: state that the reported count advances on actual removal while --max-delete is charged at plan/snapshot time (defer_add/planned). A new differential shows rsync instead charges on actual removals and recursively removes a queued directory, so the row moves to Caveat (matrix 111/13/33) and the residual is pinned by tests. - Add a deterministic unit test (plan-time budget charge), a FastSync integration test (byte-barrier refill + --max-delete), and an rsync differential for a refilled deferred directory. - Soften the --fuzzy summary: the tree is byte-exact by design, so it is pinned by the threshold suite, not a byte-level differential. - README: describe what -m parity actually selects; fix the allowlist example to the real max_delete entry. - xdist-safe delete-timing fixture names (timing and --threads mode). - Renumber the duplicate HANDOFF item 9 to 10; add the missing final newline to test_checksum.c. - Expose ignore_errors_allows_delete and unit-test the deletion gate without a privileged source directory; update the stale deleted-count doc comment. No production behavior changes.
This commit is contained in:
@@ -38,11 +38,17 @@ FastSync mirrors the absolute source path under its receive root (see
|
||||
# Fast subset that guards the ✅ surface on pull requests
|
||||
python3 -m pytest tests/integration/test_differential_parity.py -n 4 --dist=load -m parity_ci
|
||||
|
||||
# Full set (all ✅ cases plus the documented ⚠️/❌ residuals)
|
||||
# Full differential case table (`_CASES`): every row is marked `parity`, and a
|
||||
# case with an allowlisted residual in `parity_caveats.py` is included too.
|
||||
python3 -m pytest tests/integration/test_differential_parity.py -n 4 --dist=load -m parity
|
||||
```
|
||||
|
||||
The suite skips cleanly when `rsync` is not installed.
|
||||
`-m parity` selects only the `_CASES` table in this module. Differential
|
||||
coverage for options outside that table (`--temp-dir`, `--delay-updates`,
|
||||
`--dry-run`, `--fuzzy`, the basis-dir options, `-M` over daemon/TCP, and
|
||||
receiver filter-protect) lives in dedicated modules (`test_option_parity.py`,
|
||||
`test_parity_blockers.py`, `test_parity_quickwins.py`, ...) and is not part of
|
||||
this gate. The suite skips cleanly when `rsync` is not installed.
|
||||
|
||||
## Allowlist (`parity_caveats.py`)
|
||||
|
||||
@@ -52,10 +58,11 @@ Each entry maps a case id to the aspects that may differ (`tree`, `stdout`,
|
||||
|
||||
```python
|
||||
CAVEATS = {
|
||||
"min_size": {
|
||||
"tree": "recursive transfer does not create a source directory that "
|
||||
"becomes empty after --min-size filtering. ref: RSYNC_COMPAT.md "
|
||||
"`-d/--dirs` row and completion-wave residual.",
|
||||
"max_delete": {
|
||||
"tree": "which destination extras survive a partial --max-delete abort "
|
||||
"is deletion-order dependent and unspecified; rc=25 and the "
|
||||
"number of survivors match rsync. ref: RSYNC_COMPAT.md "
|
||||
"`--max-delete=NUM` row.",
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
@@ -0,0 +1,191 @@
|
||||
"""Differential coverage for ``--delete-delay`` + ``--max-delete`` with a
|
||||
refilled deferred directory.
|
||||
|
||||
FastSync snapshots a directory's extras at plan time (``defer_add``) and charges
|
||||
``--max-delete`` then, and its deferred commit only removes the snapshot path, so
|
||||
a directory refilled before the commit survives ``ENOTEMPTY``. rsync computes
|
||||
the deferred deletions during the transfer, charges ``--max-delete`` on actual
|
||||
removals, and recursively removes a queued directory -- so content created after
|
||||
the plan inside an extra directory is removed too.
|
||||
|
||||
These tests run both tools on the same fixture and pin the shared budget bound
|
||||
(the later extra survives, both exit 25) plus the documented residual (the
|
||||
refilled directory's late content survives under FastSync, not rsync). They are
|
||||
not part of the fast PR gate because the rsync side needs a wide real-time
|
||||
injection window (a throttled transfer), while the FastSync side uses the
|
||||
existing byte-deterministic slicing proxy.
|
||||
|
||||
The refilled directory sits at the transfer ROOT, whose delete plan is always
|
||||
processed before any subdirectory's, so the budget is deterministically charged
|
||||
to the refilled entry; the second extra lives under ``b`` and is skipped.
|
||||
"""
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, os.path.dirname(__file__))
|
||||
from common import ( # noqa: E402
|
||||
TEST_DATA_DIR,
|
||||
ServerManager,
|
||||
clean_dir,
|
||||
get_dest_received_dir,
|
||||
run_client,
|
||||
)
|
||||
from test_delete_timing_parity import _SlicingProxy # noqa: E402
|
||||
|
||||
RSYNC = shutil.which("rsync")
|
||||
requires_rsync = pytest.mark.skipif(RSYNC is None, reason="rsync 3.4.1 not installed")
|
||||
|
||||
BIG_BYTES = 8 * 1024 * 1024
|
||||
MID_TRANSFER_BYTES = 256 * 1024
|
||||
PROXY_THROTTLE = 0.001
|
||||
# rsync is driven locally, so the refill is injected on a wall-clock delay while
|
||||
# a throttled ~8 s transfer is in flight. 1.5 s is safely after rsync's plan
|
||||
# scan (t=0) and well before the deferred commit at the end.
|
||||
RSYNC_BWLIMIT = 1024 # 1 MiB/s
|
||||
RSYNC_INJECT_DELAY = 1.5
|
||||
|
||||
|
||||
def _write(path, content):
|
||||
os.makedirs(os.path.dirname(path), exist_ok=True)
|
||||
with open(path, "wb") as fh:
|
||||
fh.write(content)
|
||||
|
||||
|
||||
def _seed_source(tag):
|
||||
source = os.path.join(TEST_DATA_DIR, f"ddb_{tag}_src")
|
||||
clean_dir(source)
|
||||
_write(os.path.join(source, "a", "keep.bin"), b"B" * BIG_BYTES)
|
||||
_write(os.path.join(source, "b", "keep.txt"), b"keep\n")
|
||||
return source
|
||||
|
||||
|
||||
def _seed_fastsync(tag):
|
||||
"""FastSync mirrors the absolute source path under its receive root, so the
|
||||
extras live below ``received``."""
|
||||
source = _seed_source(tag)
|
||||
dest = os.path.join(TEST_DATA_DIR, f"ddb_{tag}_dst")
|
||||
clean_dir(dest)
|
||||
received = get_dest_received_dir(dest, source)
|
||||
os.makedirs(os.path.join(received, "xdir"), exist_ok=True)
|
||||
os.makedirs(os.path.join(received, "b", "ydir"), exist_ok=True)
|
||||
return source, dest, received
|
||||
|
||||
|
||||
def _seed_rsync(tag):
|
||||
"""rsync mirrors the source contents directly into the destination, so the
|
||||
extras are flat under ``rsync_dst``."""
|
||||
source = _seed_source(tag)
|
||||
rsync_dst = os.path.join(TEST_DATA_DIR, f"ddb_{tag}_dst")
|
||||
clean_dir(rsync_dst)
|
||||
os.makedirs(os.path.join(rsync_dst, "xdir"), exist_ok=True)
|
||||
os.makedirs(os.path.join(rsync_dst, "b", "ydir"), exist_ok=True)
|
||||
return source, rsync_dst
|
||||
|
||||
|
||||
def _deleted_count(text):
|
||||
for line in text.splitlines():
|
||||
if line.startswith("Number of deleted files:"):
|
||||
return int(line.split(":", 1)[1].split()[0])
|
||||
return None
|
||||
|
||||
|
||||
def _rsync(args, timeout=120):
|
||||
env = dict(os.environ, LC_ALL="C")
|
||||
return subprocess.run([RSYNC] + args, capture_output=True, text=True, env=env, timeout=timeout)
|
||||
|
||||
|
||||
class TestDeleteDelayRefilledDirVsRsync:
|
||||
"""The shared budget bound and the documented recursive-removal residual."""
|
||||
|
||||
def _fastsync_refilled(self, tag, max_delete=None):
|
||||
"""Run FastSync with the refill injected deterministically by the proxy
|
||||
hook (fired once the receiver has processed the plan frames)."""
|
||||
source, dest, received = _seed_fastsync(tag)
|
||||
late = os.path.join(received, "xdir", "new.txt")
|
||||
|
||||
def hook():
|
||||
_write(late, b"created mid-transfer\n")
|
||||
|
||||
flags = ["--delete-delay", "--incremental", "--ignore-times", "--stats"]
|
||||
if max_delete is not None:
|
||||
flags.append(f"--max-delete={max_delete}")
|
||||
with ServerManager() as server:
|
||||
server.start(extra_args=["--allow-delete"])
|
||||
proxy = _SlicingProxy(server.port, hook=hook, hook_after=MID_TRANSFER_BYTES,
|
||||
throttle=PROXY_THROTTLE, wait_for_reply=True)
|
||||
result, _ = run_client(source, dest, flags=flags, port=proxy.port)
|
||||
proxy.finish()
|
||||
assert proxy.hook_called.is_set(), "refill hook never fired"
|
||||
return result, received, late
|
||||
|
||||
@requires_rsync
|
||||
def test_max_delete_budget_bound_matches_and_residual_pinned(self):
|
||||
# --- FastSync: budget charged at snapshot; late content preserved ---
|
||||
result, received, late = self._fastsync_refilled("budget_fs", max_delete=1)
|
||||
assert result.returncode == 25, (result.stderr or result.stdout)[:300]
|
||||
assert _deleted_count(result.stdout) == 0, result.stdout
|
||||
assert os.path.exists(late), "FastSync removed the late content of a snapshotted dir"
|
||||
assert os.path.isdir(os.path.join(received, "xdir"))
|
||||
assert os.path.isdir(os.path.join(received, "b", "ydir")), (
|
||||
"FastSync did not charge the plan-time budget: b/ydir was removed"
|
||||
)
|
||||
|
||||
# --- rsync: budget charged on actual removals; dirs removed recursively ---
|
||||
source, rsync_dst = _seed_rsync("budget_rsync")
|
||||
|
||||
def inject():
|
||||
time.sleep(RSYNC_INJECT_DELAY)
|
||||
_write(os.path.join(rsync_dst, "xdir", "new.txt"), b"created mid-transfer\n")
|
||||
|
||||
t = threading.Thread(target=inject)
|
||||
t.start()
|
||||
rsync_result = _rsync(
|
||||
["-a", "--delete-delay", "--max-delete=1", "--stats",
|
||||
f"--bwlimit={RSYNC_BWLIMIT}", source + "/", rsync_dst + "/"]
|
||||
)
|
||||
t.join()
|
||||
assert rsync_result.returncode == 25, rsync_result.stderr
|
||||
# rsync removes the late content (recursive deferred removal); FastSync
|
||||
# keeps it and charges the snapshot directive instead.
|
||||
assert not os.path.exists(os.path.join(rsync_dst, "xdir", "new.txt")), (
|
||||
"rsync kept late content inside a queued directory"
|
||||
)
|
||||
# The shared observable: the later extra survives in both tools under
|
||||
# --max-delete=1, and both report the capped run with exit 25.
|
||||
assert os.path.isdir(os.path.join(rsync_dst, "b", "ydir")), (
|
||||
"rsync did not bound the deletion with --max-delete=1"
|
||||
)
|
||||
assert os.path.isdir(os.path.join(received, "b", "ydir"))
|
||||
|
||||
@requires_rsync
|
||||
def test_refilled_extra_dir_recursive_removal_residual(self):
|
||||
"""Without --max-delete the residual is a plain tree difference: rsync
|
||||
removes the refilled extra directory (and its late content), FastSync
|
||||
leaves the snapshot path in place on ENOTEMPTY."""
|
||||
result, received, late = self._fastsync_refilled("recur_fs")
|
||||
assert result.returncode == 0, (result.stderr or result.stdout)[:300]
|
||||
assert os.path.exists(late), "FastSync removed the refilled directory's late content"
|
||||
|
||||
source, rsync_dst = _seed_rsync("recur_rsync")
|
||||
|
||||
def inject():
|
||||
time.sleep(RSYNC_INJECT_DELAY)
|
||||
_write(os.path.join(rsync_dst, "xdir", "new.txt"), b"created mid-transfer\n")
|
||||
|
||||
t = threading.Thread(target=inject)
|
||||
t.start()
|
||||
rsync_result = _rsync(
|
||||
["-a", "--delete-delay", "--stats", f"--bwlimit={RSYNC_BWLIMIT}",
|
||||
source + "/", rsync_dst + "/"]
|
||||
)
|
||||
t.join()
|
||||
assert rsync_result.returncode == 0, rsync_result.stderr
|
||||
assert not os.path.exists(os.path.join(rsync_dst, "xdir")), (
|
||||
"rsync did not recursively remove the refilled extra directory"
|
||||
)
|
||||
@@ -229,10 +229,13 @@ class TestDeleteTimingFinalStateParity:
|
||||
@pytest.mark.parametrize("timing", ["--delete-during", "--delete-delay"])
|
||||
@requires_rsync
|
||||
def test_success_final_state_matches_rsync(self, timing):
|
||||
# Worker-safe names: xdist may run both parametrizations concurrently, so
|
||||
# the timing is part of every fixture path.
|
||||
label = timing.lstrip("-")
|
||||
# Build the rsync fixture from the same seed so both sides start equal.
|
||||
source, dest, received = _seed_pair("parity_rsync")
|
||||
source, dest, received = _seed_pair(f"parity_rsync_{label}")
|
||||
source2 = source
|
||||
rsync_dst = os.path.join(TEST_DATA_DIR, "dtp_parity_rsync_dst")
|
||||
rsync_dst = os.path.join(TEST_DATA_DIR, f"dtp_rsync_{label}_dst")
|
||||
clean_dir(rsync_dst)
|
||||
# rsync mirrors src/ into dst/; seed the same extra.
|
||||
_write(os.path.join(rsync_dst, "d", "old_extra"), b"stale extra\n")
|
||||
@@ -258,7 +261,8 @@ class TestDeleteTimingTypeConflictParity:
|
||||
@pytest.mark.parametrize("timing", ["--delete-during", "--delete-delay"])
|
||||
@requires_rsync
|
||||
def test_type_conflicts_match_rsync(self, timing):
|
||||
source = os.path.join(TEST_DATA_DIR, "dtc_src")
|
||||
label = timing.lstrip("-")
|
||||
source = os.path.join(TEST_DATA_DIR, f"dtc_{label}_src")
|
||||
clean_dir(source)
|
||||
_write(os.path.join(source, "foo"), b"now a file\n")
|
||||
_write(os.path.join(source, "bar", "inner.txt"), b"now a dir\n")
|
||||
@@ -268,13 +272,13 @@ class TestDeleteTimingTypeConflictParity:
|
||||
_write(os.path.join(root, "foo", "inner.txt"), b"was a dir\n")
|
||||
_write(os.path.join(root, "bar"), b"was a file\n")
|
||||
|
||||
rsync_dst = os.path.join(TEST_DATA_DIR, "dtc_rsync_dst")
|
||||
rsync_dst = os.path.join(TEST_DATA_DIR, f"dtc_{label}_rsync_dst")
|
||||
seed_dest(rsync_dst)
|
||||
rsync_result = _rsync(["-a", timing, source + "/", rsync_dst + "/"])
|
||||
assert rsync_result.returncode == 0, rsync_result.stderr
|
||||
rsync_tree = _tree(rsync_dst)
|
||||
|
||||
dest = os.path.join(TEST_DATA_DIR, "dtc_dst")
|
||||
dest = os.path.join(TEST_DATA_DIR, f"dtc_{label}_dst")
|
||||
clean_dir(dest)
|
||||
received = get_dest_received_dir(dest, source)
|
||||
seed_dest(received)
|
||||
@@ -292,7 +296,7 @@ class TestDeleteTimingFailure:
|
||||
|
||||
@pytest.mark.parametrize("mt", [False, True])
|
||||
def test_during_removes_delay_preserves_on_failure(self, mt):
|
||||
source, dest, received = _seed_pair("failure", big=True)
|
||||
source, dest, received = _seed_pair(f"failure_mt{int(mt)}", big=True)
|
||||
extra = os.path.join(received, "d", "old_extra")
|
||||
assert os.path.exists(extra)
|
||||
with ServerManager() as server:
|
||||
@@ -404,7 +408,7 @@ class TestDeleteDelayVsAfterSnapshot:
|
||||
|
||||
@pytest.mark.parametrize("mt", [False, True])
|
||||
def test_late_created_extra_survives_delay_not_after(self, mt):
|
||||
source, dest, received = _seed_pair("latecreate", big=True)
|
||||
source, dest, received = _seed_pair(f"latecreate_mt{int(mt)}", big=True)
|
||||
old_extra = os.path.join(received, "d", "old_extra")
|
||||
new_extra = os.path.join(received, "d", "new_extra")
|
||||
with ServerManager() as server:
|
||||
@@ -441,3 +445,48 @@ class TestDeleteDelayVsAfterSnapshot:
|
||||
f"{timing} (mt={mt}): new_extra present="
|
||||
f"{os.path.exists(new_extra)}, expected survives={new_survives}"
|
||||
)
|
||||
|
||||
|
||||
class TestDeleteDelayMaxDeleteRefilledDir:
|
||||
"""--delete-delay charges the --max-delete budget at plan/snapshot time, so a
|
||||
refilled snapshotted directory that survives ENOTEMPTY still spends its slot
|
||||
and a later extra is skipped, while the reported count stays at actual
|
||||
removals.
|
||||
|
||||
The refilled directory is at the destination ROOT (its plan is always sent
|
||||
first) and the skipped extra is under a separate source directory, so the
|
||||
ordering that decides the budget charge is deterministic -- not readdir
|
||||
order. The refill is injected through the byte-barrier proxy so it is
|
||||
causally after the plan frame."""
|
||||
|
||||
def test_budget_charged_at_plan_time(self):
|
||||
source = os.path.join(TEST_DATA_DIR, "ddmb_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "ddmb_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
_write(os.path.join(source, "a", "keep.bin"), b"B" * BIG_BYTES)
|
||||
_write(os.path.join(source, "b", "keep.txt"), b"keep\n")
|
||||
received = get_dest_received_dir(dest, source)
|
||||
refilled_dir = os.path.join(received, "xdir")
|
||||
os.makedirs(refilled_dir, exist_ok=True)
|
||||
later_dir = os.path.join(received, "b", "ydir")
|
||||
os.makedirs(later_dir, exist_ok=True)
|
||||
|
||||
def hook():
|
||||
_write(os.path.join(refilled_dir, "new.txt"), b"created mid-transfer\n")
|
||||
|
||||
with ServerManager() as server:
|
||||
server.start(extra_args=["--allow-delete"])
|
||||
proxy = _SlicingProxy(server.port, hook=hook, hook_after=MID_TRANSFER_BYTES,
|
||||
throttle=PROXY_THROTTLE, wait_for_reply=True)
|
||||
flags = ["--delete-delay", "--max-delete=1", "--incremental", "--ignore-times", "--stats"]
|
||||
result, _ = run_client(source, dest, flags=flags, port=proxy.port)
|
||||
proxy.finish()
|
||||
assert result.returncode == 25, (result.stderr or result.stdout)[:400]
|
||||
assert proxy.hook_called.is_set(), "hook never fired"
|
||||
# The refilled directory still consumes the plan-time budget, so the
|
||||
# later extra is skipped...
|
||||
assert os.path.exists(os.path.join(refilled_dir, "new.txt")), "late file vanished"
|
||||
assert os.path.isdir(later_dir), "later extra was not skipped by the plan-time budget"
|
||||
# ...while the reported count reflects only actual removals (none here).
|
||||
assert _deleted_count(result.stdout) == 0, result.stdout
|
||||
|
||||
Reference in New Issue
Block a user