Skip to content

{186374329} Push logs with debug records instead of static table meta writes - #6308

Open
WalidNejmi wants to merge 1 commit into
bloomberg:mainfrom
WalidNejmi:drqs-186374329-ufid-rebuild
Open

WalidNejmi wants to merge 1 commit into
bloomberg:mainfrom
WalidNejmi:drqs-186374329-ufid-rebuild

Conversation

@WalidNejmi

@WalidNejmi WalidNejmi commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Summary

Fix replica crashes caused by pushlogs writing filler data into _comdb2_static_table.metalite.dta.

pushlogs only 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 pushlogs to advance the transaction log into the next log file.

The purpose is simply:

current LSN < target LSN
        |
        v
generate WAL
        |
        v
reach target LSN

Historically, pushlogs generated that WAL by writing approximately 2 KB of filler data into the database metadata:

pushlogs
   |
   v
begin transaction
   |
   v
put_csc2_stuff()
   |
   v
write META_STUFF_RRN
   |
   v
commit
   |
   v
repeat until target LSN is reached

On databases using a shared comdb2_metadata.dta, this was generally harmless.

However, older databases configured with:

singlemeta 0

keep separate metadata files for each table.

For the internal static table, this means pushlogs used:

_comdb2_static_table.metalite.dta

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 --- R2
Loading

This happened in uniqiddb.

Several replicas had historically been copied from nodes that were missing the file. Older versions of comdb2ar did not include _comdb2_static_table.metalite.dta in 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 pushlogs wrote to it.


Why the replicas crashed

Berkeley DB log records that modify a database file identify that file using its UFID.

When pushlogs wrote filler into the static metadata file, the master generated normal btree log records referring to the UFID of:

_comdb2_static_table.metalite.dta

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"]
Loading

That is what happened to the affected uniqiddb replicas.

The rebuild itself completed successfully. The crash happened immediately afterward when pushlogs started 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 --> A
Loading

This 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, pushlogs now 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"]
Loading

The new code uses:

bdb_debug_log_data(...)

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:

_comdb2_static_table.metalite.dta

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"]
Loading

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"]
Loading

The required behavior remains the same:

generate enough WAL to reach the target LSN

but the implementation no longer depends on a physical database file.


Why debug records are appropriate

DB___db_debug is an existing Berkeley DB log record type.

Its recovery handler is effectively a no-op:

read record
update previous-LSN traversal
perform no database operation

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 == 0 is not treated as one of those recovery markers.

This also avoids introducing a new log record type purely for padding.


Transaction handling

pushlogs now uses:

trans_start_nonlogical()

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 pushlogs no longer writes to the static metadata file, it also no longer needs to create it.

The following behavior is removed:

if static meta is missing:
    create _comdb2_static_table.metalite.dta

put_csc2_stuff() and the associated META_STUFF_RRN filler path are also removed because their only purpose was to support pushlogs.

Existing _comdb2_static_table.metalite.dta files 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 pushlogs unnecessarily depended on it.

Trying to repair the file everywhere would preserve an unnecessary invariant:

every node must always contain the same static meta file

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:

PANIC: DB_RUNRECOVERY

and large DBREG/UFID dumps.

The new diagnostics report:

  • log record type
  • LSN
  • expected UFID
  • whether the UFID was ever registered on the node
  • known filename, when available
  • whether the file is missing from disk
  • the UFID actually stored on disk if it exists
  • the real file-open error

For example, the uniqiddb failure 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_ufid test reproduces the production condition.

The test:

  1. Starts a database with singlemeta 0.
  2. Creates a table, index, and rows.
  3. Runs pushnext so old code creates and replicates the static metadata file.
  4. Stops one replica.
  5. Deletes _comdb2_static_table.metalite.dta from that replica.
  6. Restarts the replica.
  7. Rebuilds the table.
  8. Inserts additional data.
  9. Runs pushnext.
  10. Inserts again.
  11. Verifies every node remains healthy and consistent.
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"]
Loading

The test verifies:

  • no PANIC
  • no DB_RUNRECOVERY
  • identical row counts on all nodes
  • identical data sums on all nodes
  • index reads work on every node
  • pushnext actually advances into the next log file

The 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_debug records 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, new pushlogs operations no longer produce file-dependent filler records.


Files changed

  • db/pushlogs.c

    • replace static-meta filler writes with debug log records
    • stop creating the static metadata file
    • use a physical/nonlogical transaction
  • bdb/file.c

    • add bdb_debug_log_data()
  • bdb/bdb_api.h

    • expose the new BDB helper
  • db/glue.c

    • remove put_csc2_stuff()
  • db/comdb2.h

    • remove the unused declaration/update metadata comments
  • berkdb/dbinc/db_am.h

    • report unresolved UFIDs before panicking
  • berkdb/dbreg/dbreg_util.c

    • add detailed unresolved-UFID reporting
  • berkdb/dbreg/dbreg_rec.c

    • improve file-open and file-ID mismatch warnings
  • tests/sc_rebuild_ufid.test

    • add regression coverage for a replica missing the static metadata file

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**

@WalidNejmi WalidNejmi changed the title DO NOT MERGE - Push logs with debug records instead of static table meta writes {186374329} DO NOT MERGE - Push logs with debug records instead of static table meta writes Oct 8, 2026
@WalidNejmi
WalidNejmi force-pushed the drqs-186374329-ufid-rebuild branch from ca61426 to 02ae180 Compare October 8, 2026 14:41
@WalidNejmi
WalidNejmi marked this pull request as ready for review October 8, 2026 14:41
@WalidNejmi WalidNejmi changed the title {186374329} DO NOT MERGE - Push logs with debug records instead of static table meta writes {186374329} Push logs with debug records instead of static table meta writes Oct 8, 2026

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@WalidNejmi
WalidNejmi force-pushed the drqs-186374329-ufid-rebuild branch from 02ae180 to 3e449e5 Compare October 8, 2026 17:45

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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**

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants