- parser: clause keywords (header, format, status, user, csv, ...) now work
as identifiers everywhere; IMPORT/EXPORT accept FORMAT csv/HEADER true
- nimble test + CI run all 13 test suites (650 checks green)
- ExecutionContext.registry is now {.cursor.} (breaks registry<->ctx cycle)
- ORC crash reproduced and bisected (tests/orc_repro.py); ARC stays the MM
6.3 KiB
BaraDB Stability Hardening — Design & Findings
Date: 2026-07-30 Status: Implemented (phase A). Phases B/C proposed, awaiting decision.
Goal
"Make the database better" — chosen direction: stability first. Establish a verified baseline, fix what is actually broken, and make the whole test suite run with one command before attempting any large refactoring or new features.
Baseline established
- Debug build passes cleanly on Nim 2.2.10 (
nimble build_debug). - The
hunosbuild failure fromHUNOS_ISSUE.mdis no longer an issue: hunos 1.3.3 is installed and contains theurandomfix. The nimble file already allows>= 1.3.0. - Of the 13 test files in
tests/, onlytest_all(+stress_testin CI) ran automatically. Baseline run of the other 11: 10 pass,nimforum_smoke_testfails.
Changes implemented
1. Parser: header usable as a column name (bug fix)
Root cause. header is a keyword token (tkHeader) used by
IMPORT/EXPORT ... HEADER. The parser never accepted it as an identifier, so
any table with a column named header (e.g. the nimforum schema) failed with
Expected identifier but got tkHeader.
Fix (src/barabadb/query/parser.nim):
- Added
tkHeadertoidentLikeKinds(soft-keyword set used byexpectIdent, line 38). - Added
tkHeaderto the identifier branch ofparsePrimary(line 84) soSELECT header ...andWHERE header = ...work. - Dotted-path parsing (
a.b.c) now usesexpectIdentinstead ofexpect(tkIdent)sopost.headerworks too.
IMPORT/EXPORT parsing is unaffected: statement dispatch keys on the leading
tkImport/tkExport and the clause parser peeks for tkHeader explicitly.
Regression tests (tests/bugfix_test.nim, new suite):
- CREATE TABLE / INSERT / SELECT / WHERE with a column named
header. IMPORT FROM ... HEADER noandEXPORT TO ... HEADER yesstill parse.
Note: IMPORT ... FORMAT csv currently fails because csv is also a keyword
(tkCsv) and parseImportFrom expects tkIdent after FORMAT. Pre-existing
limitation, not addressed here (out of scope; recorded for phase B).
2. All 13 test files wired into nimble test and CI
baradadb.nimbletesttask now buildsbuild/baradadb(the smoke test talks to it over TCP) and runs all 13 suites: quick embedded suites first, fuzz/property/stress last..github/workflows/ci.ymlrunsnimble test(was: onlytest_all); the now-redundant separate stress-test steps were removed.- Verified locally: full
nimble testexits 0 with 648 passing checks.
2b. Soft-keyword cleanup (phase B3, implemented)
Extended the header approach to all clause-only keywords, so they work as
table/column names everywhere (DDL, DML, aliases, dotted paths, CTEs, JOINs,
MERGE, GRANT/REVOKE, SET):
format, delimiter, batch, csv, ndjson, status, migration, apply, up, down, dryrun, user, policy, enable, disable, recover, before, after, instead, of.
identLikeKindsand theparsePrimaryidentifier branch now include them.- All 69
expect(tkIdent)call sites now useexpectIdent— a strict superset, so previously valid SQL is unaffected (verified: full suite green). IMPORT/EXPORT:FORMAT csv/ndjson/jsonandHEADER true/falsenow parse (previouslycsv/true/falselexed as keywords and were rejected despite the grammar clearly intending them). Clause table names useexpectIdent.
Structural keywords (where, group, order, join, on, for, using,
view, trigger, import, export, grant, ...) remain reserved.
Regression tests (tests/bugfix_test.nim): a table with 21 keyword-named
columns through CREATE/INSERT/UPDATE/SELECT/qualified refs, plus
IMPORT/EXPORT keyword-value parsing.
3. ORC crash — reproduced, bisected, three root-cause attempts failed
nim.cfg forces --mm:arc because ORC's cycle collector crashed
("markGray/trace SIGSEGV after ~20 sequential INSERTs"). The ARC cycle-breaking
in commit ed5a719 did not fix the ORC path.
Reproduction (tests/orc_repro.py): build the server with --mm:orc,
drive 1000 sequential TCP INSERTs plus 10 concurrent connections. The server
dies with the exact documented signature (handleClient →
nimDecRefIsLastCyclicStatic → collectCyclesBacon → markGray → trace →
SIGSEGV).
Bisect: 200 pings + 200 SELECTs over TCP are fine; the crash lands between 20 and 500 sequential INSERTs — INSERT path only.
Failed root-cause attempts:
ed5a719— callback cycle breaks (shard/gossip).{.cursor.}onExecutionContext.registry— breaks the realDatabaseRegistry ↔ ExecutionContextcycle (kept: it is the correct ownership annotation regardless), but the crash persists unchanged.- Guarding
ctx.onChangeagainst zero WS subscribers (reverted: fixed nothing).
Conclusion: per the 3-strikes rule this is a deep ORC+async issue —
possibly an upstream Nim 2.2.x ORC bug with async closure environments and/or
complex generic types — not a single app-level cycle. ARC remains the
supported memory manager (full suite green under it). The findings are
recorded in nim.cfg and tests/orc_repro.py for a future attempt (e.g.
re-test with a newer Nim runtime, or a minimal repro filed upstream).
Proposed next phases (not started)
- B2. Split
query/executor.nim(5,398 lines) into focused modules (DML, DDL, select pipeline, transactions). Now safe to do: the full suite guards behavior. Large diff, mechanical. - C. Features — real Raft network transport, persistence for graph/FTS/columnar engines, benchmark validation.
- ORC (blocked): re-test
tests/orc_repro.pyagainst a newer Nim runtime; if it persists, distill a minimal repro and file upstream. Not app-actionable today (see "ORC crash" section).
Verification evidence
nimble test(all 13 suites): exit 0, 650[OK], 0 failed — final run after all changes (phase A + B3 + cursor).nimforum_smoke_test(rebuilt server): all suites[OK], includingNimForum schema creation(previously[FAILED]).- B3 TDD: new keyword tests failed first with the expected
tkFormat/tkCsverrors, then passed;test_allstayed green (461[OK]). - ORC investigation: embedded
test_wire_insert_stresspasses even when compiled with--mm:orc; the TCP server compiled with--mm:orccrashes as documented above. All shipped artifacts use ARC and are green.