Skip to content

Log cksum prev - #6138

Open
markhannum wants to merge 8 commits into
bloomberg:mainfrom
markhannum:log-cksum-prev
Open

Log cksum prev#6138
markhannum wants to merge 8 commits into
bloomberg:mainfrom
markhannum:log-cksum-prev

Conversation

@markhannum

Copy link
Copy Markdown
Contributor

.

@markhannum markhannum added the WIP Work in Progress label Aug 14, 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:
cdb2api_rollback_rc [failed with core dumped]
sc_resume_logicalsc_generated **quarantined**
cdb2jdbc
log_cksum_prev
diskspace_nollmeta
diskspace_nollmeta_nostripe_generated
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

@markhannum
markhannum force-pushed the log-cksum-prev branch 8 times, most recently from 0a1e39c to 5b57ebf Compare August 14, 2026 10:33

@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:
analyze_exit_immediately [failed with core dumped]
cdb2api_rollback_rc [failed with core dumped]
cdb2jdbc
diskspace_nollmeta_nostripe_generated
diskspace_nollmeta
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

@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:
cdb2api_rollback_rc [failed with core dumped]
sc_resume_logicalsc_generated **quarantined**
cdb2jdbc
consumer_non_atomic_default_consumer_generated **quarantined**
manual_partition
sc_downgrade [timeout] **quarantined**
reco-ddlk-sql [timeout] **quarantined**
sql_logfill_autodisable [timeout]

@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:
cdb2api_rollback_rc [failed with core dumped]
cdb2jdbc
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

@markhannum
markhannum force-pushed the log-cksum-prev branch 2 times, most recently from 46cfbfd to 7f10ca4 Compare August 14, 2026 11:56

@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:
cdb2api_rollback_rc [failed with core dumped]
cdb2jdbc
sp_snapshot_generated
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

@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:
sc_resume_logicalsc_generated **quarantined**
cdb2jdbc
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

@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:
analyze_exit_immediately [failed with core dumped]
cdb2api_rollback_rc [failed with core dumped]
sc_resume_logicalsc_generated **quarantined**
cdb2jdbc
comdb2sys_pagesize_generated
consumer_non_atomic_default_consumer_generated **quarantined**
bind_query_plan
sc_downgrade [timeout] **quarantined**

@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:
yast
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**

@markhannum

Copy link
Copy Markdown
Contributor Author

Performance: log_cksum_prev vs main

Benchmarked log-cksum-prev (09cbcfd) against main (7102927) on the same
machine, same build type, using a write-heavy workload aimed at the paths this
feature touches: the log-put critical section, per-record log volume, and
master→replicant shipping.

Result: no measurable throughput or latency regression. The feature's real
cost is log volume: +3.6% on a small-record schema (4 bytes × ~4 log records
per row). With log_cksum_prev off the log format is byte-identical to main.

Workloads

Table: { int a, int b } with one dup index on a.

  • W1 bulk: insert 1M rows in 100 transactions of 10k (server-side generate_series)
  • W2 concurrent commits: 8 parallel clients × 10k single-row autocommit
    transactions (~16k commits/sec; stresses the serialized log-put path where
    the previous-record checksum is stamped)
  • W3 updates: update 100k rows in 10 transactions of 10k
  • W4 log volume: du of the logs directory after W1 (flushed)

Single node (median of 3 runs, seconds; lower is better)

Workload main branch, cksum on branch, cksum off
W1 bulk insert 1M rows 20.2 19.1 21.3
W2 8×10k concurrent commits 6.3 5.5 6.0
W3 update 100k rows 4.9 4.2 4.5
W4 log bytes after 1M rows 455.6 MB 471.8 MB (+3.6%) 455.6 MB

3-node cluster (mean of 2 runs, seconds)

Workload main branch, cksum on
W1 bulk insert 1M rows 64.4 58.9
W2 8×10k concurrent commits 16.1 15.0
W3 update 100k rows 6.2 6.3
W4 log bytes after 1M rows 455.7 MB 471.9 MB (+3.6%)

Notes

  • Run-to-run noise on this host was ±10–15% (worst single-node bulk rep: 31 s
    on the branch, 23 s on main), so sub-10% throughput effects are below the
    noise floor; the branch was as fast or faster than main in every workload's
    median, so a regression hiding in the noise is unlikely. The +3.6% log
    growth, by contrast, is exact and reproducible across every run: 16.2 MB per
    1M rows = 4 bytes × ~4 log records per row. The percentage shrinks as
    records get larger.
  • log_cksum_prev off produces log volume matching main to within a kilobyte,
    i.e. the field is elided entirely, not zero-filled — legacy-defaults keep the
    old format on disk.
  • Cluster numbers were taken with all three nodes co-hosted on one machine, so
    they measure replication protocol overhead, not real network latency.

@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**
bind_query_plan
sc_downgrade [timeout] **quarantined**
reco-ddlk-sql [timeout] **quarantined**

@markhannum markhannum removed the WIP Work in Progress label Aug 16, 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:
osql_cleanup [failed with core dumped]
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
truncatesc_offline_generated [timeout] **quarantined**
reco-ddlk-sql [timeout] **quarantined**

@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: Error ⚠.
Regression testing: Success ✓.

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

diskspace_nollmeta compared 'stat size' output verbatim, including the N%
share each table holds of total space.  That denominator counts the log
files, so the ratio moves whenever the log format changes and the test
fails for reasons that have nothing to do with table sizing.  Strip the
percentage and compare sizes only.

Signed-off-by: Mark Hannum <[email protected]>
Rectype tags (ufid +1000, utxnid +2000) were decoded by open-coded range
tests in ten files, each correct only for the tags that existed when it
was written.  Replace them with one decomposition.  No behaviour change:
normalize_rectype() agrees with the old version on every live rectype.

Signed-off-by: Mark Hannum <[email protected]>
Will gate writing the previous log record's checksum into each record.
Nothing reads it yet.  READEARLY like utxnid_log, since a log-format
tunable must be settled before the log opens.  Off under legacy_defaults.

Signed-off-by: Mark Hannum <[email protected]>
Adds rectype tag +4000: the record carries a u_int32_t prev_cksum after
the utxnid in its common prefix.  Reserved as zero here; the value is not
known until the log region lock is held.

In the prefix rather than at the end because generated readers get a
recbuf with no length and can only parse forward.  txn_auto.c tag tests
move off "== DB___x + 2000", which the new tag breaks.

Signed-off-by: Mark Hannum <[email protected]>
prev_cksum widens the common prefix, so hand-computed
"4 + 4 + 8 + (utxnid_logged ? 8 : 0)" offsets are short by four bytes.

Walking the prefix field by field was worse: it steps over only the
fields it knows, so a new one is skipped silently -- __lc_cache_feed()
lost prev_cksum that way.  Consumers now read fixed fields at
DB_REC_OFF_* and reach the body via __rectype_prefix_len().

Also fixes __log_put_next() reading __txn_dist_commit's generation at
4+4+8+4, which landed in the utxnid tail.

Signed-off-by: Mark Hannum <[email protected]>
__log_put_next() stamps lp->last_cksum into the reserved field under the
region lock; __log_putr() then advances it.  Both dbt (our log) and udbt
(what a master ships) are stamped.

lp->last_cksum is in-memory only, re-derived by __log_recover() and
__log_vtruncate() from the last real record at or before their position,
crossing into the previous file when needed.  LOGP headers carry
log_size, which may differ between nodes, so they stay out of the chain.

Also exposes prevcksum/cksum on comdb2_transaction_logs, and adds the
record generator to llog_auto.c's CMake DEPENDS.

Signed-off-by: Mark Hannum <[email protected]>
A chained record's checksum transitively covers the whole log prefix, so
matching it byte-for-byte at the same LSN implies identical history --
the guarantee the commit-only rule provided.  Decided from the record's
own tag, so an unchained source falls back to commit-only.

DB___txn_ckp_recovery stays excluded, and is never used for a generation:
recovering a copy emits one above the cluster's generation.

A non-commit anchor carries no generation, so find_match_lsn() asks the
source for the one in effect there -- passing maxlsn as well as minlsn,
or the vtab scans from the end of the log.  A missing generation is only
a bad logstream if the record should have carried one.

Signed-off-by: Mark Hannum <[email protected]>
Checks each record's prevcksum against the previous record's cksum, in
steady state, across a log file rollover, and across a bounce.

pushlogs is asynchronous and master-only, so the rollover is polled for
rather than assumed.  Every node is flushed at the end: a flush that
hangs is itself a bug, and failing here beats letting the harness's own
silent flush time out and blame the datafiles.

Signed-off-by: Mark Hannum <[email protected]>

@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: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
ssl_san
consumer_non_atomic_default_consumer_generated **quarantined**
ssl_set_cmd
ssl_prefer
ssl_dbname
sc_downgrade [timeout] **quarantined**
reco-ddlk-sql [timeout] **quarantined**

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