feat(stats): populate receiver wire counters on both receive paths
The single-threaded and -m receivers never populated ReceiverStats.matched_data or .deleted_files, so --stats always printed 0 for both even when rsync reported nonzero. Track the bytes reconstructed from the basis file while applying a delta, and tally the delete-commit counts (manifest and per-directory sessions) into the receiver stats. The -m pipeline now carries its own stats/would-delete fields and emits the STATUS_STATS frame before the terminal success, so --threads finally reports the counters and renders -n --delete lines. Also normalize the -n --delete would-delete enumeration's absolute basis prefixes exactly like the real commit path (fixing an over-report) and fix the basis_delete_relative off-by-one when the receive root is '/'. Unit tests cover the root mapping and the basis protection; integration tests cover matched/deleted stats for both receivers and the --threads dry-run delete lines.
This commit is contained in:
@@ -80,3 +80,89 @@ class TestRelativePerDirDeleteScope:
|
||||
assert not os.path.exists(os.path.join(dest, "foo", "extra.txt"))
|
||||
assert not os.path.exists(os.path.join(rdst, "foo", "extra.txt"))
|
||||
assert _tree(dest) == _tree(rdst)
|
||||
|
||||
|
||||
def _stats_value(text, label):
|
||||
for line in text.splitlines():
|
||||
if line.startswith(label + ":"):
|
||||
return int(line.split(":", 1)[1].strip().split()[0].replace(",", ""))
|
||||
return None
|
||||
|
||||
|
||||
def _seed_delta_pair(tag):
|
||||
"""Source file plus a same-size/basis destination file whose mtime differs,
|
||||
and an extra destination file to be deleted."""
|
||||
source = os.path.join(TEST_DATA_DIR, f"stats_{tag}_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, f"stats_{tag}_dst")
|
||||
rdst = os.path.join(TEST_DATA_DIR, f"stats_{tag}_rdst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
clean_dir(rdst)
|
||||
payload = (b"0123456789abcdef" * 16384)[:200000]
|
||||
_write(os.path.join(source, "f.bin"), payload)
|
||||
# Destination basis: same length, one byte changed, deliberately older.
|
||||
basis = bytearray(payload)
|
||||
basis[100000] ^= 0xFF
|
||||
received = get_dest_received_dir(dest, source)
|
||||
for root in (rdst, received):
|
||||
_write(os.path.join(root, "f.bin"), bytes(basis))
|
||||
_write(os.path.join(root, "extra.txt"), b"delete me\n")
|
||||
old = 1000000
|
||||
os.utime(os.path.join(root, "f.bin"), (old, old))
|
||||
return source, dest, rdst
|
||||
|
||||
|
||||
class TestReceiverWireStats:
|
||||
"""Blocker #3/#4: the receiver must populate the STATUS_STATS counters
|
||||
(matched data, deleted files) on both the single-threaded and -m paths."""
|
||||
|
||||
@requires_rsync
|
||||
@pytest.mark.ci
|
||||
@pytest.mark.parametrize("threads", [False, True])
|
||||
def test_stats_reports_matched_and_deleted(self, threads):
|
||||
source, dest, rdst = _seed_delta_pair(f"mt{int(threads)}")
|
||||
rsync_result = _rsync(["-a", "--stats", "--delete", "--no-whole-file", source + "/",
|
||||
rdst + "/"])
|
||||
assert rsync_result.returncode == 0, rsync_result.stderr
|
||||
assert _stats_value(rsync_result.stdout, "Matched data") > 0
|
||||
assert _stats_value(rsync_result.stdout, "Number of deleted files") == 1
|
||||
|
||||
with ServerManager() as server:
|
||||
server.start(extra_args=["--allow-delete"])
|
||||
flags = ["-a", "--stats", "--delete", "--delta", "--incremental"]
|
||||
if threads:
|
||||
flags.append("--threads")
|
||||
result, _ = run_client(source, dest, flags=flags, port=server.port)
|
||||
assert result.returncode == 0, (result.stderr or result.stdout)[:300]
|
||||
assert _stats_value(result.stdout, "Matched data") > 0, result.stdout
|
||||
assert _stats_value(result.stdout, "Number of deleted files") == 1, result.stdout
|
||||
|
||||
@requires_rsync
|
||||
@pytest.mark.ci
|
||||
def test_threads_dry_run_delete_lines_match_rsync(self):
|
||||
"""-n --delete --threads must emit transfer-relative `*deleting` lines."""
|
||||
source = os.path.join(TEST_DATA_DIR, "stats_drydel_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "stats_drydel_dst")
|
||||
rdst = os.path.join(TEST_DATA_DIR, "stats_drydel_rdst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
clean_dir(rdst)
|
||||
_write(os.path.join(source, "a.txt"), b"a\n")
|
||||
for root in (rdst, get_dest_received_dir(dest, source)):
|
||||
_write(os.path.join(root, "extra.txt"), b"x\n")
|
||||
_write(os.path.join(root, "sub", "y.txt"), b"y\n")
|
||||
rsync_result = _rsync(["-a", "-n", "--delete", "-i", source + "/", rdst + "/"])
|
||||
assert rsync_result.returncode == 0, rsync_result.stderr
|
||||
rsync_del = sorted(
|
||||
line for line in rsync_result.stdout.splitlines() if line.startswith("*deleting")
|
||||
)
|
||||
with ServerManager() as server:
|
||||
server.start(extra_args=["--allow-delete"])
|
||||
result, _ = run_client(source, dest,
|
||||
flags=["-a", "-n", "--delete", "-i", "--threads"],
|
||||
port=server.port)
|
||||
assert result.returncode == 0, (result.stderr or result.stdout)[:300]
|
||||
fast_del = sorted(
|
||||
line for line in result.stdout.splitlines() if line.startswith("*deleting")
|
||||
)
|
||||
assert fast_del and fast_del == rsync_del, f"rsync={rsync_del}\nfastsync={fast_del}"
|
||||
|
||||
@@ -2002,6 +2002,100 @@ static void test_manifest_delete_missing_dir_budget_double_count() {
|
||||
rmdir(root);
|
||||
}
|
||||
|
||||
/* Blocker #7: when the receive root is "/", every absolute basis path is below
|
||||
it and its child relative form must drop only the single leading slash. */
|
||||
static void test_basis_delete_relative_root_slash() {
|
||||
Config* cfg = config_create();
|
||||
EXPECT_NOT_NULL(cfg);
|
||||
cfg->receive_root_directory = str_dup("/");
|
||||
|
||||
char* rel = file_receive_basis_delete_relative(cfg, "/a");
|
||||
EXPECT_NOT_NULL(rel);
|
||||
EXPECT_EQ_STR(rel, "a");
|
||||
free(rel);
|
||||
rel = file_receive_basis_delete_relative(cfg, "/a/b");
|
||||
EXPECT_NOT_NULL(rel);
|
||||
EXPECT_EQ_STR(rel, "a/b");
|
||||
free(rel);
|
||||
/* The root itself is not a child. */
|
||||
EXPECT_NULL(file_receive_basis_delete_relative(cfg, "/"));
|
||||
/* A relative entry is already root-relative. */
|
||||
rel = file_receive_basis_delete_relative(cfg, "x/y");
|
||||
EXPECT_NOT_NULL(rel);
|
||||
EXPECT_EQ_STR(rel, "x/y");
|
||||
free(rel);
|
||||
/* An absolute path outside a non-"/" root is unreachable. */
|
||||
free(cfg->receive_root_directory);
|
||||
cfg->receive_root_directory = str_dup("/root");
|
||||
EXPECT_NULL(file_receive_basis_delete_relative(cfg, "/other/a"));
|
||||
rel = file_receive_basis_delete_relative(cfg, "/root/a");
|
||||
EXPECT_NOT_NULL(rel);
|
||||
EXPECT_EQ_STR(rel, "a");
|
||||
free(rel);
|
||||
config_delete(cfg);
|
||||
}
|
||||
|
||||
/* Blocker #6: -n --delete would-delete enumeration must normalize an absolute
|
||||
basis directory under the receive root exactly like the real commit path, so
|
||||
the basis snapshot is protected rather than reported as a deletable extra. */
|
||||
static void test_manifest_would_delete_protects_absolute_basis() {
|
||||
char root[PATH_MAX];
|
||||
snprintf(root, sizeof(root), "/tmp/fastsync_wdbasis_%d", (int)getpid());
|
||||
char* basis = path_cat(root, "basis");
|
||||
char* basis_file = path_cat(basis, "snapshot.bin");
|
||||
char* extra = path_cat(root, "extra.txt");
|
||||
EXPECT_NOT_NULL(basis);
|
||||
EXPECT_NOT_NULL(basis_file);
|
||||
EXPECT_NOT_NULL(extra);
|
||||
mkdir(root, 0755);
|
||||
mkdir(basis, 0755);
|
||||
EXPECT_TRUE(file_write_to_disk(basis_file, "x", 1, false, false));
|
||||
EXPECT_TRUE(file_write_to_disk(extra, "e", 1, false, false));
|
||||
|
||||
Config* cfg = config_create();
|
||||
EXPECT_NOT_NULL(cfg);
|
||||
cfg->receive_root_directory = str_dup(root);
|
||||
cfg->use_delete = true;
|
||||
EXPECT_EQ_INT(config_basis_append(cfg, BASIS_DEST_COMPARE, basis), 0);
|
||||
|
||||
const char* synced[] = {"."};
|
||||
DeleteManifest manifest = {0};
|
||||
manifest.keeps = make_manifest_string_list(NULL, 0);
|
||||
manifest.protected = make_manifest_string_list(NULL, 0);
|
||||
manifest.dirs = make_manifest_string_list(synced, 1);
|
||||
EXPECT_NOT_NULL(manifest.keeps);
|
||||
EXPECT_NOT_NULL(manifest.protected);
|
||||
EXPECT_NOT_NULL(manifest.dirs);
|
||||
ArrayList* out = array_list_create(free);
|
||||
EXPECT_NOT_NULL(out);
|
||||
size_t count = 0;
|
||||
EXPECT_TRUE(manifest_would_delete_list(cfg, &manifest, out, &count));
|
||||
bool saw_basis = false;
|
||||
bool saw_extra = false;
|
||||
for (int i = 0; i < out->size; i++) {
|
||||
const char* p = (const char*)out->items[i];
|
||||
if (strcmp(p, "basis") == 0 || strncmp(p, "basis/", 6) == 0)
|
||||
saw_basis = true;
|
||||
if (strcmp(p, "extra.txt") == 0)
|
||||
saw_extra = true;
|
||||
}
|
||||
EXPECT_FALSE(saw_basis);
|
||||
EXPECT_TRUE(saw_extra);
|
||||
|
||||
array_list_delete(out);
|
||||
array_list_delete(manifest.keeps);
|
||||
array_list_delete(manifest.protected);
|
||||
array_list_delete(manifest.dirs);
|
||||
config_delete(cfg);
|
||||
unlink(basis_file);
|
||||
rmdir(basis);
|
||||
unlink(extra);
|
||||
rmdir(root);
|
||||
free(basis);
|
||||
free(basis_file);
|
||||
free(extra);
|
||||
}
|
||||
|
||||
void test_file() {
|
||||
test_file_create();
|
||||
test_file_special_rdev_valid();
|
||||
@@ -2057,4 +2151,6 @@ void test_file() {
|
||||
test_inplace_refuses_fifo_destination();
|
||||
test_inplace_refuses_device_destination();
|
||||
test_manifest_delete_missing_dir_budget_double_count();
|
||||
test_basis_delete_relative_root_slash();
|
||||
test_manifest_would_delete_protects_absolute_basis();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user