backups: a revision chain belongs to a PC, not just a machine

Dedup compared a posted config against the latest revision for the ASSET,
which is only correct when a machine number means one PC. Several PCs share one
here: the part markers on 0613, 0615 and WJPRT are separate devices, differing
by COM port, filed under one machine number. Each marker's post therefore
differed from whichever marker had posted last, nothing ever deduped, and the
table grew by one row per PC per collection cycle.

A chain is now (asset, kind, source hostname). An unchanged config is a no-op
again, and each PC keeps its own history against the machine. NULL sources -
rows written before the column was populated, and hand-loaded ones - form their
own chain via IS NULL; `column == None` never matches in SQL, so without that
those rows would have re-posted forever.

Two consumers assumed the old key and are fixed with it. Retention pruned per
asset, so a busy marker's revisions could evict a quiet marker's only backup;
it now prunes each chain separately, protecting the newest and oldest of each.
The revision diff compared against the previous revision on the machine, which
across two markers reported one device's COM port as a change on the other; it
now compares within the source's own chain.

scripts/collapse_duplicate_backup_revisions.py cleans up what the old rule
wrote. It removes only a revision whose hash repeats the one before it in the
same chain - rows the fixed code would never have written - and keeps every
genuine change, every chain's newest and oldest, and every source. Dry run by
default. Its --report mode explains what grew each chain, which separates a
legitimately shared machine number from two PCs wrongly carrying the same one,
and from a value inside the config that changes on its own.
This commit is contained in:
cproudlock
2026-08-10 15:02:38 -04:00
parent 5108ba8aaa
commit d429c882b4
6 changed files with 376 additions and 16 deletions

View File

@@ -757,3 +757,93 @@ def test_a_marker_export_shows_the_mark_tab():
assert any('MARK' in label for label in labels)
mark = next(s for s in card['sections'] if 'MARK' in s['label'])
assert any(f['label'] == 'Port Id' and f['value'] == 'COM4' for f in mark['fields'])
# =============================================================================
# Several PCs sharing one machine number
# =============================================================================
def _markerreg(comport):
"""A marker config that differs from its sibling only by COM port.
Which is what the real ones do: the part markers sharing 0613, 0615 and
WJPRT are separate devices on separate ports, filed under one machine
number.
"""
return CONFIGUREDREG.replace('"MachineNo"="3204"',
'"MachineNo"="3204"\r\n'
'"Port Id"="{}"'.format(comport))
def test_two_pcs_on_one_machine_number_do_not_thrash(bk_app, bk_plugin):
"""The bug that filled the table: dedup was against the LATEST revision for
the asset, so with two PCs on one machine number each post differed from
whichever PC posted last. Nothing ever deduped and the chain grew by a row
per PC per collection cycle, forever."""
from plugins.backups.models import BackupRevision
with bk_app.app_context():
for _ in range(5):
bk_plugin.apply_collector_payload(
_payload(reg=_markerreg('COM3'), sourcehostname='MARKERA'))
bk_plugin.apply_collector_payload(
_payload(reg=_markerreg('COM4'), sourcehostname='MARKERB'))
# One revision each, not ten.
assert _db.session.query(BackupRevision).count() == 2
def test_each_pc_keeps_its_own_history_on_a_shared_machine(bk_app, bk_plugin):
"""Both markers' configs matter, so both chains are kept in full."""
from plugins.backups.models import BackupRevision
with bk_app.app_context():
bk_plugin.apply_collector_payload(
_payload(reg=_markerreg('COM3'), sourcehostname='MARKERA'))
bk_plugin.apply_collector_payload(
_payload(reg=_markerreg('COM4'), sourcehostname='MARKERB'))
# MARKERA is re-cabled. That is a real change on A, and none on B.
bk_plugin.apply_collector_payload(
_payload(reg=_markerreg('COM5'), sourcehostname='MARKERA'))
result = bk_plugin.apply_collector_payload(
_payload(reg=_markerreg('COM4'), sourcehostname='MARKERB'))
assert result['action'] == 'noop'
rows = _db.session.query(BackupRevision).all()
bysource = {}
for row in rows:
bysource.setdefault(row.sourcehostname, []).append(row)
assert len(bysource['MARKERA']) == 2
assert len(bysource['MARKERB']) == 1
def test_a_null_source_still_forms_a_chain(bk_app, bk_plugin):
"""Rows written before sourcehostname was populated, and hand-loaded ones,
carry NULL. `column == None` never matches in SQL, so without an IS NULL
those rows would dedupe against nothing and re-post every cycle."""
from plugins.backups.models import BackupRevision
with bk_app.app_context():
first = bk_plugin.apply_collector_payload(_payload(sourcehostname=None))
second = bk_plugin.apply_collector_payload(_payload(sourcehostname=None))
assert second['action'] == 'noop'
assert second['backuprevisionid'] == first['backuprevisionid']
assert _db.session.query(BackupRevision).count() == 1
def test_retention_prunes_each_pc_separately(bk_app, bk_plugin):
"""Pruning per asset let a busy marker's revisions evict a quiet marker's
only backup, which is the one row that mattered on that PC."""
from plugins.backups.models import BackupRevision
from plugins.backups.services.retention import prune
with bk_app.app_context():
bk_plugin.apply_collector_payload(
_payload(reg=_markerreg('COM9'), sourcehostname='QUIET'))
for port in ('COM3', 'COM4', 'COM5', 'COM6', 'COM7'):
bk_plugin.apply_collector_payload(
_payload(reg=_markerreg(port), sourcehostname='BUSY'))
asset = _db.session.query(BackupRevision).first().assetid
prune(asset, 'ntlars', retentioncount=2)
_db.session.commit()
rows = _db.session.query(BackupRevision).all()
sources = [row.sourcehostname for row in rows]
assert 'QUIET' in sources, 'the quiet PC lost its only backup'