Skip to content

Comments

Fix transaction durability#590

Open
Stanpol wants to merge 3 commits intotonbo-io:mainfrom
Stanpol:fix/transaction-durability
Open

Fix transaction durability#590
Stanpol wants to merge 3 commits intotonbo-io:mainfrom
Stanpol:fix/transaction-durability

Conversation

@Stanpol
Copy link

@Stanpol Stanpol commented Feb 12, 2026

I found an issue where transactional writes remained in memory indefinitely, failing to trigger memtable sealing or SSTable flushing. This is particularly impactful for S3-backed storage where data visibility and persistence rely on regular SSTable creation. I added the ability to configure sealing policies via DbBuilder and improve WASM compatibility.

Key Changes

  1. Transaction Persistence (src/transaction/mod.rs)
  • I added calls to maybe_seal_after_insert() and maybe_run_minor_compaction() immediately after a transaction commit.
  • Previously, only direct db.ingest() calls triggered maintenance. Transactional writes stayed in the mutable memtable regardless of sealing policies, leading to data loss on web page refresh/process exit in serverless environments.
  1. Sealing Policy API (src/db/builder.rs and src/db/mod.rs)
  • I introduced DbBuilder::with_seal_policy() to allow configuring memtable behavior during database initialization.
  • Made DB::set_seal_policy() public.
  • This changes allow users to set low-latency thresholds (BatchesThreshold { batches: 1 }) for example in wasm web applications.
  1. WASM and S3 Compatibility (src/db/mod.rs and src/ondisk/sstable.rs)
  • I replaced std::time::Instant with executor::Instant in the L0StatsCache.
  • std::time::Instant is often unreliable or panics in WASM environments, using the executor's clock ensures correct time-based sealing logic in the browser.
  • I added to Parquet validation to allow files with 0 row groups. This prevents crashes during minor compactions that resulted in delete-only or empty batches, an edge case in transactional workflows.
  1. Added tests

Happy to answer questions, fix something or discuss the issues.

@ethe ethe requested a review from belveryin February 16, 2026 07:43
@ethe
Copy link
Member

ethe commented Feb 16, 2026

Thank you @Stanpol ! I'll be reviewing this PR.

@ethe ethe self-requested a review February 16, 2026 07:44
Copy link
Collaborator

@belveryin belveryin left a comment

Choose a reason for hiding this comment

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

@Stanpol thank you so much for your contribution. I have two comments to the PR:

  • Record WAL range before post-commit sealing in fast mode — src/transaction/mod.rs:679
    In fast mode, WAL range publication is deferred to async finalize, but sealing now runs immediately after commit. That allows a segment to be sealed before its WAL range is attached, which breaks WAL floor/GC bookkeeping assumptions. WAL range should be attached before any operation that can seal.

  • Keep seal policy configuration build-time; avoid exposing runtime setter yet — src/db/mod.rs:336, src/db/builder.rs:950 DbBuilder::with_seal_policy(...) already provides the needed configuration path at database creation time.
    The new public DB::set_seal_policy(...) has surprising semantics: it only works with an exclusive handle (Arc::get_mut) and otherwise becomes a no-op (false). Since DB is cloneable, this is easy to misuse and leaks internal ownership behavior into the public API.
    Recommendation: for this PR, maybe keep seal policy as builder configuration and remove (or make internal/test-only) the public runtime setter. If runtime reconfiguration is needed later, introduce a shared-safe API with explicit Result errors.

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.

3 participants