diff --git a/src/fenris/derive.py b/src/fenris/derive.py index 2470d5f..ab62889 100644 --- a/src/fenris/derive.py +++ b/src/fenris/derive.py @@ -217,25 +217,28 @@ def _add_unattributed_bytes( bw_delta: int, br_delta: int, ) -> None: - """Add unattributed byte deltas to day aggregates for each day touched.""" - prev_day = prev_ts.strftime("%Y-%m-%d") - next_day = next_ts.strftime("%Y-%m-%d") + """Store unattributed byte deltas once as shared boundary evidence. - days = {prev_day, next_day} - for day in days: - existing = conn.execute( - "SELECT id FROM day_aggregates WHERE day = ?", (day,) - ).fetchone() - if existing is None: - conn.execute( - "INSERT INTO day_aggregates (day, unattributed_bytes_written, unattributed_bytes_read) " - "VALUES (?, ?, ?)", - (day, bw_delta, br_delta), - ) - else: - conn.execute( - "UPDATE day_aggregates SET unattributed_bytes_written = unattributed_bytes_written + ?, " - "unattributed_bytes_read = unattributed_bytes_read + ? WHERE day = ?", - (bw_delta, br_delta, day), - ) + A cross-midnight interval's delta is preserved on the day where it + STARTS (the earlier day). It is not duplicated into both days; + the spec requires preserving the measured volume once as shared + unallocated boundary evidence (issue #88). + """ + day = prev_ts.strftime("%Y-%m-%d") + + existing = conn.execute( + "SELECT id FROM day_aggregates WHERE day = ?", (day,) + ).fetchone() + if existing is None: + conn.execute( + "INSERT INTO day_aggregates (day, unattributed_bytes_written, unattributed_bytes_read) " + "VALUES (?, ?, ?)", + (day, bw_delta, br_delta), + ) + else: + conn.execute( + "UPDATE day_aggregates SET unattributed_bytes_written = unattributed_bytes_written + ?, " + "unattributed_bytes_read = unattributed_bytes_read + ? WHERE day = ?", + (bw_delta, br_delta, day), + ) conn.commit() diff --git a/tests/test_cross_day_duplication.py b/tests/test_cross_day_duplication.py new file mode 100644 index 0000000..7de3cb8 --- /dev/null +++ b/tests/test_cross_day_duplication.py @@ -0,0 +1,145 @@ +"""Cross-day interval unattributed-byte preservation (issue #88). + +Verifies that a cross-hour interval spanning midnight is stored ONCE as +shared boundary evidence, not duplicated into both days. + +Seam: derive._add_unattributed_bytes() → day_aggregates.unattributed_bytes_* +""" +import sqlite3 +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import pytest +import sys +sys.path.insert(0, str(Path(__file__).parent.parent / "src")) + +from fenris.collector import run_collection +from fenris.store import init_store +from fenris.monitoring_periods import ensure_period_open + + +def _make_smartctl(duw: int, dur: int): + return { + "json_format_version": [1, 0], + "smartctl": {"version": [7, 3], "svn_revision": "5155", + "build_info": "(local build)"}, + "nvme_smart_health_information_log": { + "critical_warning": 0, "temperature": 35, + "available_spare": 100, "available_spare_threshold": 10, + "percentage_used": 5, "data_units_written": duw, + "data_units_read": dur, "power_on_hours": 8765, + "power_cycles": 1234, "unsafe_shutdowns": 5, + "media_errors": 0, "num_err_log_entries": 0, + }, + "user_capacity": {"bytes": 1024000000000, "units": "bytes"}, + "model_name": "Samsung SSD 970 EVO Plus 1TB", + "serial_number": "S4EWNX0N123456", + "firmware_version": "2B2QEXM7", + } + + +@pytest.fixture +def sysfs_tree(tmp_path: Path) -> Path: + ctrl_dir = tmp_path / "sys" / "class" / "nvme" / "nvme0" + ctrl_dir.mkdir(parents=True) + (ctrl_dir / "subsysnqn").write_text( + "nqn.2014-08.org.nvmexpress:uuid:12345678-1234-1234-1234-123456789abc\n" + ) + (ctrl_dir / "model").write_text("Samsung SSD 970 EVO Plus 1TB\n") + (ctrl_dir / "serial").write_text("S4EWNX0N123456\n") + (ctrl_dir / "firmware_rev").write_text("2B2QEXM7\n") + transport_dir = ctrl_dir / "transport" + transport_dir.mkdir() + (transport_dir / "address").write_text("0000:03:00.0") + (transport_dir / "trstring").write_text("pcie") + return tmp_path + + +class _Clock: + def __init__(self, initial): + self.now = initial + def utcnow(self): + return self.now + + +class TestCrossDayUnattributedNoDuplication: + """Unattributed bytes from a midnight-spanning interval must be stored + once, not duplicated into both days (issue #88).""" + + def test_midnight_spanning_interval_not_duplicated( + self, tmp_path, sysfs_tree, + ): + """Two samples spanning midnight: 23:55 UTC day1 → 00:05 UTC day2. + + The unattributed bytes should appear once, not in both days. + """ + store = str(tmp_path / "obs.db") + cfg = {"device": "/dev/nvme0", "store_path": store} + sysfs_nvme = sysfs_tree / "sys" / "class" / "nvme" / "nvme0" + + t1 = datetime(2026, 9, 1, 23, 55, 0, tzinfo=timezone.utc) + t2 = datetime(2026, 9, 2, 0, 5, 0, tzinfo=timezone.utc) + + # +100 DUW, +60 DUR across midnight + r1 = run_collection(_make_smartctl(10000000, 8000000), sysfs_nvme, cfg, _Clock(t1)) + assert r1["ok"] + r2 = run_collection(_make_smartctl(10000100, 8000060), sysfs_nvme, cfg, _Clock(t2)) + assert r2["ok"] + + conn = sqlite3.connect(store) + + # Get unattributed bytes for each day + unattr_w_day1 = conn.execute( + "SELECT unattributed_bytes_written FROM day_aggregates WHERE day = ?", + ("2026-09-01",) + ).fetchone() + unattr_w_day2 = conn.execute( + "SELECT unattributed_bytes_written FROM day_aggregates WHERE day = ?", + ("2026-09-02",) + ).fetchone() + unattr_r_day1 = conn.execute( + "SELECT unattributed_bytes_read FROM day_aggregates WHERE day = ?", + ("2026-09-01",) + ).fetchone() + unattr_r_day2 = conn.execute( + "SELECT unattributed_bytes_read FROM day_aggregates WHERE day = ?", + ("2026-09-02",) + ).fetchone() + + expected_bw = 100 * 512000 # 51200000 + expected_br = 60 * 512000 # 30720000 + + # BUG: Current code adds the SAME bytes to BOTH days. + # After fix: only ONE day should have the unattributed bytes. + # The spec says: preserve once as shared boundary evidence. + + # At least one day must have the unattributed bytes + total_unattr_w = (unattr_w_day1[0] if unattr_w_day1 else 0) + (unattr_w_day2[0] if unattr_w_day2 else 0) + total_unattr_r = (unattr_r_day1[0] if unattr_r_day1 else 0) + (unattr_r_day2[0] if unattr_r_day2 else 0) + + # The total unattributed bytes across both days must equal + # the actual delta (not double) + assert total_unattr_w == expected_bw, ( + f"Unattributed writes across both days should be {expected_bw}, " + f"got {total_unattr_w} (duplication detected)" + ) + assert total_unattr_r == expected_br, ( + f"Unattributed reads across both days should be {expected_br}, " + f"got {total_unattr_r} (duplication detected)" + ) + + # Neither day should have MORE than the actual delta + for day_label, uw, ur in [ + ("day1", unattr_w_day1, unattr_r_day1), + ("day2", unattr_w_day2, unattr_r_day2), + ]: + if uw is not None: + assert uw[0] <= expected_bw, ( + f"{day_label} unattributed writes {uw[0]} exceeds delta {expected_bw}" + ) + if ur is not None: + assert ur[0] <= expected_br, ( + f"{day_label} unattributed reads {ur[0]} exceeds delta {expected_br}" + ) + + conn.close()