Physrep fail copy on recovery - #6114

Open

markhannum wants to merge 2 commits into

bloomberg:mainfrom

markhannum:physrep-fail-copy-on-recovery

markhannum commented Aug 6, 2026 •

edited

Loading

Contributor

Abort a copy when recovery could rewind pages during it

A copy (copycomdb2/comdb2ar, via the logdelete appsock) can be silently corrupted if recovery rewinds the log while the copy is in progress — routine on a physical replicant that truncates to resync, and on a normal replicant that connects to a new master.

Make the copy and recovery mutually exclusive, and fail the copy (rather than stall it) when recovery needs to run. A new logdelete4 appsock has the copy hold recoverlk and the bdb lock in read mode for its duration; when recovery (or a node upgrade/downgrade) waits on either, the handler releases the locks and marks the copy aborted. A copy_complete handshake reports ok/aborted, and comdb2ar discards the archive (non-zero exit) on anything but ok.

What changed

bdb/rep.c, bdb/bdb_api.h: bdb_readlock_recovery / bdb_unlock_recovery wrappers over recoverlk.
plugins/logdelete: new logdelete4 appsock (read-lock hold, poll-and-yield, copy_complete); logdelete/2/3 unchanged.
tools/comdb2ar: negotiate logdelete4 (fallback to 3/2), add LogHolder::copy_ok(), fail serialise_database when it returns false.

Compatibility

Safe either direction: a new comdb2ar falls back to logdelete3/2 against an old DB, and an old comdb2ar uses logdelete3 against a new DB. Full protection needs both sides new. Companion client PR: comdb2ar logdelete4-fail-copy-on-recovery.

Testing

tests/abort_copy_on_recovery.test forces a rewind during a held copy and asserts it aborts, under both online_recovery on and off, plus a no-recovery run that still succeeds.

🤖 Generated with Claude Code

roborivers approved these changes

View reviewed changes

roborivers left a comment

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
sc_truncate_multiddl_generated [db unavailable at finish] **quarantined**
sc_truncate [db unavailable at finish]
abort_copy_on_recovery [db unavailable at finish]
sc_resume_logicalsc_generated **quarantined**
sc_resume
cdb2jdbc
cldeadlock
noresetgen
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

markhannum force-pushed the physrep-fail-copy-on-recovery branch from 738efde to 6c10a3d Compare

August 7, 2026 00:10

roborivers approved these changes

View reviewed changes

roborivers left a comment

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
comdb2sys_pagesize_generated [db unavailable at finish]
sc_resume_logicalsc_generated **quarantined**
cdb2jdbc
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
phys_rep_tiered_nosource_generated [timeout]
phys_rep_tiered_firstfile_generated [timeout]
phys_rep_tiered [timeout]

markhannum force-pushed the physrep-fail-copy-on-recovery branch from 6c10a3d to 1701846 Compare

August 7, 2026 08:38

roborivers suggested changes

View reviewed changes

roborivers left a comment

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
osql_cleanup [failed with core dumped]
cdb2jdbc
ssl_san
consumer_non_atomic_default_consumer_generated **quarantined**
ssl_set_cmd
ssl_prefer
ssl_dbname
sc_downgrade [timeout] **quarantined**
phys_rep_tiered_firstfile_generated [timeout]
phys_rep_tiered_nosource_generated [timeout]

markhannum force-pushed the physrep-fail-copy-on-recovery branch 2 times, most recently from eadc085 to 1d06d94 Compare

August 11, 2026 15:11

roborivers suggested changes

View reviewed changes

roborivers left a comment

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
abort_copy_on_recovery [db unavailable at finish]
cdb2jdbc
reco-ddlk-sql **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
phys_rep_tiered [timeout]
phys_rep_tiered_firstfile_generated [timeout]
phys_rep_tiered_nosource_generated [timeout]

roborivers suggested changes

View reviewed changes

roborivers left a comment

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
sc_resume_logicalsc_generated **quarantined**
cdb2jdbc
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
truncatesc_offline_generated [timeout] **quarantined**

markhannum force-pushed the physrep-fail-copy-on-recovery branch 3 times, most recently from 19f65f8 to cfd4fa3 Compare

August 11, 2026 19:23

roborivers suggested changes

View reviewed changes

roborivers left a comment

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
sc_resume_logicalsc_generated **quarantined**
cdb2jdbc
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

roborivers suggested changes

View reviewed changes

roborivers left a comment

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
cdb2jdbc
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
truncatesc_offline_generated [timeout] **quarantined**

markhannum added 2 commits

August 13, 2026 01:54


          Abort a copy when recovery could rewind pages during it

edbfaf7

comdb2ar reads data files live; it tolerates forward page writes but not
recovery that rewinds the log (__log_vtruncate), which silently corrupts
the archive.  Make copy and recovery mutually exclusive: a new logdelete4
appsock holds recoverlk and the bdb lock in read mode for the copy's
duration (bdb before recoverlk, matching recovery's order, so no
deadlock), polls to yield if an exclusive op waits, and reports ok/aborted
via a copy_complete handshake so comdb2ar discards a doomed copy.

comdb2ar negotiates logdelete4 then falls back to logdelete3/2 on an
"unknown command" reply; a handshake timeout instead means the server is
busy draining a recovery, so it fails hard rather than downgrade to an
unprotected copy.  debug_block_comdb2ar is gated to v4 for the test.

Signed-off-by: Mark Hannum <mhannum@bloomberg.net>


          Test: abort a copy when recovery runs during the copy

8c35d3c

Holds a logdelete4 copy via debug_block_comdb2ar, forces a rewind with
sys.cmd.truncate_time, and verifies comdb2ar aborts non-zero; also checks
a clean copy still succeeds.  Runs with online_recovery on and off.  Plus:
a stalled v4 handshake must fail hard, not silently downgrade to logdelete3.

Signed-off-by: Mark Hannum <mhannum@bloomberg.net>

markhannum force-pushed the physrep-fail-copy-on-recovery branch from cfd4fa3 to 8c35d3c Compare

August 13, 2026 05:55

roborivers approved these changes

View reviewed changes

roborivers left a comment

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
cdb2jdbc
reco-ddlk-sql **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet