Repository navigation
{186374329} Push logs with debug records instead of static table meta writes - #6308
WalidNejmi wants to merge 1 commit into
Conversation
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
ca61426 to
02ae180
Compare
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
blkseq_logdel [failed with core dumped]
comdb2sys_queueodh_generated
consumer_non_atomic_default_consumer_generated **quarantined**
reco-ddlk-sql [timeout] **quarantined**
On a database with a meta table per table, pushlogs wrote its filler into _comdb2_static_table.metalite.dta. That file is created lazily by pushlogs on the master, is never recreated on a replicant, and older comdb2ar did not copy it. A replicant without it cannot register the file, so the first pushlogs write hits REC_INTRO_PANIC and the node aborts with DB_RUNRECOVERY, then again after every restart. Log the filler as debug records in a non-logical transaction instead. They advance the log the same way but are no-ops for recovery and replication, so pushlogs no longer depends on any file existing on replicants. Stop the pushlogs thread on a replicant: debug records are not logged there, so it would loop without moving the log. Remove the on-the-fly creation of the static table meta and the unused put_csc2_stuff. Add diagnostics so an unresolved file is identified at the panic: report the record type, LSN and ufid, whether the ufid-hash knows the file and whether the file on disk is missing or has a different fileid; warn when a replicant cannot open a file named by the master; raise "Mismatched fileid" to a warning; include the open error when a ufid reopen fails. Add sc_rebuild_ufid: restart a replicant without the static table meta, rebuild a table and push the log. It panics before this change. Signed-off-by: Walid Nejmi <wnejmi@bloomberg.net>
02ae180 to
3e449e5
Compare
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
comdb2sys_pagesize_generated [db unavailable at finish]
consumer_non_atomic_default_consumer_generated **quarantined**
reco-ddlk-sql [timeout] **quarantined**
Summary
Fix replica crashes caused by
pushlogswriting filler data into_comdb2_static_table.metalite.dta.pushlogsonly needs to advance the transaction log to a target LSN. Historically, it did this by repeatedly writing junk into the static table metadata file. That created an unnecessary replication dependency on a file that some replicas may legitimately not have due to historical copies or migrations.This change removes that dependency by generating WAL using existing Berkeley DB debug records instead. Debug records are recovery/replication no-ops and do not reference any database file or UFID.
This fixes the failure seen in some databases referenced by the number in the title.
It also adds diagnostics so future unresolved-UFID failures identify the missing or mismatched file directly.
Background (if unfamiliar)
After certain schema changes, Comdb2 calls
pushlogsto advance the transaction log into the next log file.The purpose is simply:
Historically,
pushlogsgenerated that WAL by writing approximately 2 KB of filler data into the database metadata:On databases using a shared
comdb2_metadata.dta, this was generally harmless.However, older databases configured with:
keep separate metadata files for each table.
For the internal static table, this means
pushlogsused:The contents written there were not meaningful metadata. The file was effectively being used as a place to generate log traffic.
The bug
The master and replicas are not guaranteed to all have
_comdb2_static_table.metalite.dta.At startup, a replica only opens the static table metadata file if it already exists. It does not recreate it.
pushlogs, however, would create the file on the master if necessary.That creates this possible state:
flowchart LR M["Master<br/>static meta exists"] R1["Replica A<br/>static meta exists"] R2["Replica B<br/>static meta missing"] M --- R1 M --- R2This happened in
uniqiddb.Several replicas had historically been copied from nodes that were missing the file. Older versions of
comdb2ardid not include_comdb2_static_table.metalite.dtain copies, so that state could survive across disk replacements, recopies, and migrations.The database continued to operate normally because the file itself was not required for normal table data.
The problem only became visible when
pushlogswrote to it.Why the replicas crashed
Berkeley DB log records that modify a database file identify that file using its UFID.
When
pushlogswrote filler into the static metadata file, the master generated normal btree log records referring to the UFID of:Replicas then attempted to apply those records.
A replica with the file could resolve the UFID and apply the update normally.
A replica that had never registered the file could not resolve the UFID.
flowchart TD A["pushlogs writes filler to static meta"] --> B["Master generates btree WAL record"] B --> C["Record contains static meta UFID"] C --> D{"Replica can resolve UFID?"} D -->|"Yes"| E["Open local DB handle"] E --> F["Apply page update"] D -->|"No"| G["REC_INTRO_PANIC"] G --> H["DB_RUNRECOVERY"] H --> I["Replica aborts"]That is what happened to the affected
uniqiddbreplicas.The rebuild itself completed successfully. The crash happened immediately afterward when
pushlogsstarted advancing the log.Why this could become a crash loop
Startup recovery and live replication treat missing files differently.
During startup recovery, a missing file can be tolerated in some recovery paths because the file may have been legitimately removed.
During live replication, however, silently ignoring a log record for an unknown file would risk data divergence.
So a replica could:
flowchart TD A["Replica crashes"] --> B["Restart"] B --> C["Startup recovery"] C --> D["Missing-file record tolerated during recovery"] D --> E["Replica rejoins live replication"] E --> F["Next pushlogs btree record references missing UFID"] F --> G["DB_RUNRECOVERY"] G --> AThis continued until the affected replicas were recopied.
We intentionally do not change this behavior. A missing UFID for a real table should remain fatal rather than risk silently losing replicated updates.
Fix
Instead of writing meaningless data into a real database file,
pushlogsnow writes Berkeley DB debug records.Debug records already exist in Berkeley DB and are recovery/replication no-ops.
The new flow is:
flowchart TD A["pushlogs needs to advance WAL"] --> B["Begin physical transaction"] B --> C["Write debug record with filler payload"] C --> D["Commit transaction"] D --> E["WAL advances"] E --> F{"Target LSN reached?"} F -->|"No"| B F -->|"Yes"| G["pushlogs complete"]The new code uses:
to write a 4 KB payload into the log.
No database file is modified.
No page is touched.
No UFID needs to be resolved.
So the presence or absence of:
is no longer relevant to
pushlogs.Before vs. after
Before
flowchart LR A["pushlogs"] --> B["put_csc2_stuff()"] B --> C["_comdb2_static_table.metalite.dta"] C --> D["Btree WAL record"] D --> E["UFID lookup on replicas"] E --> F["Missing file can panic replica"]After
flowchart LR A["pushlogs"] --> B["bdb_debug_log_data()"] B --> C["DB___db_debug record"] C --> D["Replicated normally"] D --> E["Recovery no-op"]The required behavior remains the same:
but the implementation no longer depends on a physical database file.
Why debug records are appropriate
DB___db_debugis an existing Berkeley DB log record type.Its recovery handler is effectively a no-op:
This makes it suitable for
pushlogs, whose only requirement is to consume log space.The filler records use operation value
0.Other Comdb2 recovery logic gives special meaning to debug records with values such as
op == 2;op == 0is not treated as one of those recovery markers.This also avoids introducing a new log record type purely for padding.
Transaction handling
pushlogsnow uses:rather than
trans_start().trans_start()can create a logical transaction when rowlocks are enabled.These filler records are low-level physical Berkeley DB records, so they should always run inside a physical transaction regardless of rowlock configuration.
Static metadata file creation is removed
Since
pushlogsno longer writes to the static metadata file, it also no longer needs to create it.The following behavior is removed:
put_csc2_stuff()and the associatedMETA_STUFF_RRNfiller path are also removed because their only purpose was to supportpushlogs.Existing
_comdb2_static_table.metalite.dtafiles are left untouched.There is no need to copy, recreate, or delete them as part of this change.
Why we do not repair the file on every replica
The underlying problem is not that every replica needs this file.
The problem is that
pushlogsunnecessarily depended on it.Trying to repair the file everywhere would preserve an unnecessary invariant:
and would require copies, migrations, reclusters, backups, and restores to continue preserving that file indefinitely.
It would also not be enough to simply create a new file with the same name, because Berkeley DB log records reference the file by UFID, not only by pathname.
Removing the dependency entirely is simpler and safer.
Improved UFID diagnostics
The change also improves diagnostics around unresolved UFIDs.
Previously, failures often ended with little more than:
and large DBREG/UFID dumps.
The new diagnostics report:
For example, the
uniqiddbfailure would now identify that the log record referenced a UFID that had never been registered on that replica.This does not change recovery behavior. It only makes the root cause much easier to identify.
Regression test
A new
sc_rebuild_ufidtest reproduces the production condition.The test:
singlemeta 0.pushnextso old code creates and replicates the static metadata file._comdb2_static_table.metalite.dtafrom that replica.pushnext.flowchart TD A["Create table and data"] --> B["pushnext"] B --> C["Stop one replica"] C --> D["Delete static meta file"] D --> E["Restart replica"] E --> F["REBUILD table"] F --> G["Insert"] G --> H["pushnext"] H --> I["Insert"] I --> J["Verify all replicas"]The test verifies:
PANICDB_RUNRECOVERYpushnextactually advances into the next log fileThe test reproduces the crash on the previous production code and passes with this fix.
Compatibility
No new log record type is introduced.
Older replicas already understand
DB___db_debugrecords and treat them as no-ops.Therefore the important behavior change is on the node running
pushlogs: once the master is running the fixed code, newpushlogsoperations no longer produce file-dependent filler records.Files changed
db/pushlogs.cbdb/file.cbdb_debug_log_data()bdb/bdb_api.hdb/glue.cput_csc2_stuff()db/comdb2.hberkdb/dbinc/db_am.hberkdb/dbreg/dbreg_util.cberkdb/dbreg/dbreg_rec.ctests/sc_rebuild_ufid.test