Skip write-only flush on read-side catalog serialize - #1787
Merged
Conversation
doltliteFlushAndSerializeCatalog always called flushAllPending, which asserts a write transaction. Read-side callers such as doltliteHasUncommittedChanges serialize outside a write txn and abort under asserts (seen as regression crashes / no summary under PROLLY_CHECK). Only flush pending maps when inTrans is TRANS_WRITE; otherwise serialize the already-consistent catalog.
Contributor
Version-Control Performance CeilingsRuns: median of 3 executions per benchmark, excluding fixture setup. The
|
Contributor
Sysbench-Style Benchmark (TEXT PK): Doltlite vs SQLiteCompanion to the classic Sysbench-Style Benchmark. Every workload here In-MemoryReads
Writes
File-BackedReads
Writes
File-Backed (autocommit)Each statement runs as its own transaction — exposes per-commit ReadsReads have no commit cost; these are the same SQL files as the
Writes
100000 rows, median of 5 invocations per test; autocommit writes use 9, workload-only timing via host monotonic clock when available. Performance Ceiling Check (2.5x individual, 2x average; autocommit writes: 10x / 5x)All tests within ceilings. |
Contributor
Sysbench-Style Benchmark: Doltlite vs SQLiteIn-MemoryReads
Writes
File-BackedReads
Writes
File-Backed (autocommit)Each statement runs as its own transaction — exposes per-commit ReadsReads have no commit cost; these are the same SQL files as the
Writes
100000 rows, median of 5 invocations per test; autocommit writes use 9, workload-only timing via host monotonic clock when available. Performance Ceiling Check (2.5x individual, 2x average; autocommit writes: 10x / 5x)All tests within ceilings. |
Contributor
Sysbench-Style Benchmark (composite PK): Doltlite vs SQLiteCompanion to the classic Sysbench-Style Benchmark. Every workload here In-MemoryReads
Writes
File-BackedReads
Writes
File-Backed (autocommit)Each statement runs as its own transaction — exposes per-commit ReadsReads have no commit cost; these are the same SQL files as the
Writes
100000 rows, median of 5 invocations per test; autocommit writes use 9, workload-only timing via host monotonic clock when available. Performance Ceiling Check (2.5x individual, 2x average; autocommit writes: 10x / 5x)All tests within ceilings. |
development-builds only compiled mdevtest configs with --buildonly, so PROLLY_ASSERT_* never executed. Add a --with-debug build of catalog_serialize_determinism_test, which exercises doltliteHasUncommittedChanges after read-side work and would have caught the flushAllPending write-txn assert.
Contributor
Sysbench-Style Benchmark (BLOB PK): Doltlite vs SQLiteCompanion to the classic Sysbench-Style Benchmark. Every workload here In-MemoryReads
Writes
File-BackedReads
Writes
File-Backed (autocommit)Each statement runs as its own transaction — exposes per-commit ReadsReads have no commit cost; these are the same SQL files as the
Writes
100000 rows, median of 5 invocations per test; autocommit writes use 9, workload-only timing via host monotonic clock when available. Performance Ceiling Check (2.5x individual, 2x average; autocommit writes: 10x / 5x)All tests within ceilings. |
added 3 commits
July 23, 2026 17:56
csFileLock jumped from NO_LOCK to EXCLUSIVE. That works under NDEBUG but trips unixLock's SQLITE_DEBUG asserts (NO_LOCK may only step to SHARED). Acquire SHARED first, then EXCLUSIVE, so assert-enabled builds (development-builds dirty-check smoke) can open a database.
The extra SHARED lock before EXCLUSIVE shifts malloc-injection indices in backup_malloc / fkey_malloc / vtab_err. Drop fixed syscall-5.1.5/7 entries that now pass under the legal lock ladder.
delete_100k_to_1m flaked on ubuntu CI at 30ms→362ms (12x over a 10x cap). Short single-op timings are dominated by process start and runner jitter; floor the small side at 50ms so the ratio measures scale.
|
SummaryCoverage spans debug-time safety checks, catalog reading and writing, persistence and clean-state behavior, file-lock ownership and recovery, fault-handling boundaries, and performance scaling across small to large data sets. It also exercises adversarial failure paths and exact boundary conditions, with healthy results across functional, reliability, and performance behavior. Safe to merge — the exercised application behaviors show no PR-attributable regressions or new failures, and all retained fault divergences are expected rather than unexpected defects. No merge blocker is indicated; the lock-escalation fault path was source-checked because no direct fault-injection path is available, which is only a flag for later. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
doltliteFlushAndSerializeCatalogno longer callsflushAllPending/flushDeferredEditsunless the btree is in a write transaction.doltliteHasUncommittedChanges) that serialize the catalog without a write txn and hitPROLLY_ASSERT_WRITE_TXNinflushAllPending.Problem
Found while testing #1781 / sqlite-regression under
DOLTLITE_PROLLY_CHECK=1. The dirty-state path indoltliteHasUncommittedChangescallsdoltliteFlushAndSerializeCatalog, which always invokedflushAllPending. That routine assertsinTrans==TRANS_WRITE, so read-side serialize aborts before the test summary.Verification
EXIT=134) onflushAllPendingwrite-txn assert after a read-onlySELECT+ serialize/dirty check.serialize_rc=0,dirty_rc=0 dirty=0.catalog_serialize_determinism_test: 80 passed, 0 failed.Test plan
catalog_serialize_determinism_test