Splitting leadership and db logic into two workers #25
Conversation
WalkthroughIntroduces a coordinator/DB-worker architecture with WorkerConfig, CoordinatorState, DbWorkerState, channel-based leadership and worker messaging, DB job queuing/execution, runtime-mode dispatch (Coordinator vs DbOnly), and added LeaderReady channel message plus worker-template embedding for query timeout and embedded worker. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Thread
participant Coord as Coordinator
participant Lock as navigator.locks
participant Chan as BroadcastChannel
participant DBW as DB Worker
Main->>Coord: initialize & setup_channel_listener()
Coord->>Chan: subscribe
Coord->>Coord: start_leader_probe()
Coord->>Lock: request lock (try_become_leader)
alt Lock Acquired
Lock->>Coord: on_lock_granted
Coord->>Coord: promote to Leader
Coord->>Chan: broadcast "leader-ready"
Coord->>DBW: spawn_db_worker()
DBW->>DBW: init SQLiteDatabase
DBW->>Coord: post worker-ready
Coord->>Coord: mark db_worker_ready
else Lock Not Acquired
Coord->>Coord: remain Follower
Coord->>Chan: wait for "leader-ready"
end
Main->>Coord: forward_query_to_db(origin, request)
Coord->>Coord: enqueue or forward based on role
Coord->>DBW: post QueryRequest
DBW->>DBW: exec_on_db
DBW->>Coord: post QueryResponse (query-result)
Coord->>Main: deliver result (direct or forwarded)
sequenceDiagram
participant Main as Main Thread
participant Worker as Worker (DB-Only)
participant DB as SQLiteDatabase
Main->>Worker: init (DB-only)
Worker->>Worker: detect __SQLITE_DB_ONLY
Worker->>Worker: create DbWorkerState
Worker->>DB: initialize DB
Worker->>Main: send worker-ready
Main->>Worker: send WorkerMessage (query)
Worker->>Worker: enqueue DbJob
Worker->>DB: exec_on_db(job)
Worker->>Main: send QueryResponse (query-result)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
📚 Learning: 2025-09-15T06:11:31.781ZApplied to files:
📚 Learning: 2025-08-27T05:55:40.481ZApplied to files:
📚 Learning: 2025-09-17T06:55:20.178ZApplied to files:
📚 Learning: 2025-09-19T07:06:20.173ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (8)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locksvelte-test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
packages/sqlite-web-core/src/coordination.rs(4 hunks)packages/sqlite-web-core/src/messages.rs(2 hunks)packages/sqlite-web-core/src/worker.rs(1 hunks)packages/sqlite-web/src/worker_template.rs(2 hunks)pkg/package.json(1 hunks)svelte-test/package.json(1 hunks)svelte-test/src/routes/sql/+page.svelte(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/sqlite-web-core/src/messages.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use structured message types for worker communication as defined in messages.rs
Files:
packages/sqlite-web-core/src/messages.rs
packages/sqlite-web-core/src/worker.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Keep worker.rs as the main worker entry point invoked by worker_main()
Files:
packages/sqlite-web-core/src/worker.rs
svelte-test/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
In the Svelte test app, depend on the sqlite-web package via the locally packed tarball produced by npm pack
Files:
svelte-test/package.json
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/worker.rs : Keep worker.rs as the main worker entry point invoked by worker_main()
📚 Learning: 2025-09-17T08:04:44.062Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 6
File: svelte-test/tests/database-functions/float-sum.test.ts:78-86
Timestamp: 2025-09-17T08:04:44.062Z
Learning: In svelte-test/tests/database-functions/float-sum.test.ts, the schema defines `amount TEXT NOT NULL` but the NULL value handling tests attempt to insert NULL values, creating a constraint violation that prevents the tests from passing.
Applied to files:
svelte-test/src/routes/sql/+page.svelte
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/worker_template.rs : Ensure the worker JavaScript generated by worker_template.rs embeds the WASM (base64) and does not fetch external WASM files
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/worker.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/embedded_worker.js : Do not manually edit embedded_worker.js; it is generated by the build and should be treated as read-only
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/worker.rssvelte-test/package.jsonpackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/worker.rs : Keep worker.rs as the main worker entry point invoked by worker_main()
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/worker.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/messages.rs : Use structured message types for worker communication as defined in messages.rs
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/messages.rspackages/sqlite-web-core/src/worker.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Maintain self-contained workers with no external WASM file dependencies across the project
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/worker.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Applied to files:
packages/sqlite-web/src/worker_template.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/lib.rs : Expose an async, promise-based query interface from the SQLiteWasmDatabase public API
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-19T07:06:20.173Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:39-57
Timestamp: 2025-09-19T07:06:20.173Z
Learning: PRAGMA function_list is not available in the WASM SQLite test environment used by svelte-test, so availability testing should rely on direct function calls rather than PRAGMA queries.
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-15T06:11:31.781Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 9
File: packages/sqlite-web-core/src/coordination.rs:52-65
Timestamp: 2025-09-15T06:11:31.781Z
Learning: In packages/sqlite-web-core/src/coordination.rs, the pattern of using `db.borrow_mut().take()` followed by async database operations and then putting the database back with `*db.borrow_mut() = Some(database)` is safe in their use case and doesn't cause concurrent access issues or "Database not initialized" errors.
Applied to files:
packages/sqlite-web-core/src/worker.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to svelte-test/package.json : In the Svelte test app, depend on the sqlite-web package via the locally packed tarball produced by npm pack
Applied to files:
svelte-test/package.json
📚 Learning: 2025-08-27T05:59:04.254Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 5
File: svelte-test/tests/database-functions/bigint-sum.test.ts:2-7
Timestamp: 2025-08-27T05:59:04.254Z
Learning: In the sqlite-web repository, the user prefers to keep .js extensions when importing TypeScript files in test files, even in Vitest environments. This is acceptable and should not be flagged as an issue.
Applied to files:
svelte-test/package.json
📚 Learning: 2025-08-21T15:09:22.737Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 2
File: flake.nix:59-68
Timestamp: 2025-08-21T15:09:22.737Z
Learning: In the sqlite-web project, the build-submodules task in flake.nix successfully builds the lib/rain.math.float submodule with `forge build` even if foundry.toml is not immediately visible in the expected location. The build process works as intended according to the project maintainer.
Applied to files:
svelte-test/package.json
🧬 Code graph analysis (2)
packages/sqlite-web-core/src/worker.rs (1)
packages/sqlite-web-core/src/coordination.rs (5)
send_worker_error(743-751)worker_config_from_global(33-70)new(112-128)new(617-624)js_sys(893-894)
packages/sqlite-web-core/src/coordination.rs (3)
packages/sqlite-web-core/src/util.rs (4)
sanitize_identifier(5-19)js_value_to_string(39-60)value(44-44)value(50-50)packages/sqlite-web-core/src/worker.rs (1)
serde_wasm_bindgen(34-34)packages/sqlite-web/src/worker.rs (1)
serde_wasm_bindgen(57-57)
🔇 Additional comments (31)
pkg/package.json (1)
15-15: Trailing newline cleanup looks good.Minor formatting improvement with no functional impact.
svelte-test/src/routes/sql/+page.svelte (1)
76-79: Minor indentation changes look fine.The formatting adjustments in the query section header are consistent with the rest of the file structure.
svelte-test/package.json (1)
44-44: Version bump to alpha.8 aligns with the new coordinator architecture.The dependency correctly uses the local tarball format as per coding guidelines.
packages/sqlite-web/src/worker_template.rs (2)
7-15: Embedding worker body twice is intentional for DB worker spawning.The
embedded_worker.jsis included both as a JSON-encoded string (forself.__SQLITE_EMBEDDED_WORKER) and as the executable body. This enables the coordinator to spawn embedded DB workers from the stored blob while also executing as the main worker. The approach aligns with the self-contained worker requirement.
36-39: Test coverage for the new embedded worker global is adequate.packages/sqlite-web-core/src/worker.rs (7)
13-20: Clean runtime enum and thread-local storage design.The
WorkerRuntimeenum withCoordinatorandDbOnlyvariants cleanly separates the two runtime modes. Usingthread_local!withconstinitialization is idiomatic for WASM single-threaded contexts.
22-28: Correct fallback handling for missing flag.The
is_db_only_mode()function safely defaults tofalsewhen the global is missing or not a boolean.
30-55: Well-structured message dispatch with proper error handling.The handler correctly:
- Parses incoming messages into structured
WorkerMessagetypes (per coding guidelines)- Routes messages to the appropriate runtime via pattern matching
- Reports parsing errors back to the main thread
- Uses
Closure::forget()appropriately for persistent WASM event handlers
57-69: Coordinator runtime initialization sequence is well-ordered.The startup correctly initializes state and channel communication before installing the message handler, ensuring the runtime is ready to process messages.
71-79: DB-only runtime startup is appropriately simpler.The initialization correctly skips leadership coordination, focusing only on database setup.
82-91: Entry point correctly maintains worker.rs as main entry invoked by worker_main().The
main()function cleanly separates configuration retrieval from runtime selection, following the coding guidelines. Based on learnings, this keeps worker.rs as the main worker entry point.
101-116: Tests adequately cover the db_only_mode flag behavior.The tests verify both the default (false) and explicit (true) cases, with proper cleanup using
Reflect::delete_property.packages/sqlite-web-core/src/messages.rs (2)
25-29: LeaderReady variant follows established patterns.The new
LeaderReadyvariant correctly mirrors theNewLeaderstructure with consistent serde attributes (leader-readytag,leaderIdcamelCase field). This enables the coordinator to signal when the leader is ready, supporting the PR's goal of decoupling leadership from database initialization.
222-227: Test coverage for LeaderReady serialization is adequate.The roundtrip test verifies correct JSON structure and deserialization. The
assert_serialization_roundtriphelper ensures both directions work correctly.packages/sqlite-web-core/src/coordination.rs (17)
1-31: LGTM!The imports are appropriate for the coordinator functionality, and the
LeadershipRoleenum andWorkerConfigstruct are well-defined with clear semantics.
33-70: LGTM!The configuration extraction logic properly validates inputs with sensible defaults. The test fallback for
db_nameand the finite/non-negative check forfollower_timeout_msare appropriate.
72-104: LGTM!The data structures are well-designed.
DbRequestOrigincleanly separates local vs forwarded query tracking, and the use ofRc<RefCell<...>>is appropriate for the single-threaded WASM async context.
111-141: LGTM!The initialization and channel listener setup follow standard patterns for WASM. The
Closure::forget()pattern is appropriate for long-lived event handlers.
143-201: LGTM!The leader probe logic correctly handles both finite and infinite timeouts, with proper polling intervals and timeout error reporting.
203-238: LGTM!The Web Locks API is used correctly for leader election. The never-resolving promise pattern (
Promise::new(&mut |_, _| {})) ensures the lock is held for the lifetime of the leader worker.
240-297: LGTM!The lock grant handler and DB worker spawning logic are well-implemented. The Blob + URL pattern for worker creation with injected configuration is appropriate, and the URL is correctly revoked after use.
299-347: LGTM!The DB worker event handling and failure recovery are well-designed. The failure handler properly resets state, terminates the failed worker, fails pending queries, and attempts automatic recovery through respawn.
403-472: LGTM!The channel message handling is comprehensive. The optimization where followers can also respond to
LeaderPingwith cachedLeaderReadyinformation helps reduce leader discovery time.
474-538: LGTM!The query forwarding logic is well-implemented with proper ID generation (avoiding 0 via
wrapping_add(1).max(1)) and comprehensive error handling that properly routes failures back to their origin.
540-614: LGTM!The helper methods are well-implemented.
fail_originproperly routes errors based on query origin, andsignal_ready_oncecorrectly implements idempotent ready signaling.
616-694: LGTM!The
DbWorkerStateimplementation correctly handles serial query processing. The queue processor pattern usingCell<bool>for the processing flag is appropriate for the single-threaded WASM context, and the post-loop check (lines 689-691) handles items enqueued during processing. Based on learnings, thedb.borrow_mut().take()pattern is safe in this codebase.
696-770: LGTM!The messaging helper functions provide a clean abstraction for worker communication with consistent error handling and message formatting.
772-841: LGTM!The query result message construction is well-structured, and the test double for
deliver_db_result(lines 828-841) enables unit testing without actual worker message posting.
843-884: LGTM!The
exec_on_dbimplementations are well-designed. The production version correctly handles the database lifecycle, and the test double provides useful concurrent access detection for testing the queue serialization.
886-923: LGTM!The
sleep_msimplementation robustly handles different execution contexts (worker vs window) with an immediate fallback if no timer API is available.
925-1206: LGTM!The tests provide good coverage of key scenarios including leader election, ping responses based on readiness state, failure recovery, and queue serialization. The test isolation techniques (stubbing
navigator.locks.request, using global flags for fake DB) are appropriate.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
svelte-test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
packages/sqlite-web-core/src/coordination.rs(4 hunks)packages/sqlite-web/src/tests.rs(1 hunks)packages/sqlite-web/src/worker_template.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/worker.rs : Keep worker.rs as the main worker entry point invoked by worker_main()
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 9
File: packages/sqlite-web-core/src/coordination.rs:52-65
Timestamp: 2025-09-15T06:11:31.781Z
Learning: In packages/sqlite-web-core/src/coordination.rs, the pattern of using `db.borrow_mut().take()` followed by async database operations and then putting the database back with `*db.borrow_mut() = Some(database)` is safe in their use case and doesn't cause concurrent access issues or "Database not initialized" errors.
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/messages.rs : Use structured message types for worker communication as defined in messages.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/worker_template.rs : Ensure the worker JavaScript generated by worker_template.rs embeds the WASM (base64) and does not fetch external WASM files
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web/src/tests.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/embedded_worker.js : Do not manually edit embedded_worker.js; it is generated by the build and should be treated as read-only
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web/src/tests.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/worker.rs : Keep worker.rs as the main worker entry point invoked by worker_main()
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web/src/tests.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/messages.rs : Use structured message types for worker communication as defined in messages.rs
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web/src/tests.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web-core/src/database_functions.rs : Integrate and expose rain.math.float-backed custom database functions for SQLite queries
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web/src/tests.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Maintain self-contained workers with no external WASM file dependencies across the project
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-17T06:55:20.178Z
Learnt from: CR
Repo: rainlanguage/sqlite-web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T06:55:20.178Z
Learning: Applies to packages/sqlite-web/lib.rs : Expose an async, promise-based query interface from the SQLiteWasmDatabase public API
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web/src/tests.rspackages/sqlite-web-core/src/coordination.rs
📚 Learning: 2025-09-19T07:06:20.173Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 14
File: svelte-test/tests/database-functions/float-is-zero.test.ts:39-57
Timestamp: 2025-09-19T07:06:20.173Z
Learning: PRAGMA function_list is not available in the WASM SQLite test environment used by svelte-test, so availability testing should rely on direct function calls rather than PRAGMA queries.
Applied to files:
packages/sqlite-web/src/worker_template.rspackages/sqlite-web/src/tests.rs
📚 Learning: 2025-09-15T10:01:17.743Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 12
File: svelte-test/tests/fixtures/test-helpers.ts:11-11
Timestamp: 2025-09-15T10:01:17.743Z
Learning: In the sqlite-web project, test database names in createTestDatabase function do not need timestamp suffixes for uniqueness. The maintainer has confirmed that test isolation via unique naming is not required.
Applied to files:
packages/sqlite-web/src/tests.rs
📚 Learning: 2025-09-15T06:11:31.781Z
Learnt from: findolor
Repo: rainlanguage/sqlite-web PR: 9
File: packages/sqlite-web-core/src/coordination.rs:52-65
Timestamp: 2025-09-15T06:11:31.781Z
Learning: In packages/sqlite-web-core/src/coordination.rs, the pattern of using `db.borrow_mut().take()` followed by async database operations and then putting the database back with `*db.borrow_mut() = Some(database)` is safe in their use case and doesn't cause concurrent access issues or "Database not initialized" errors.
Applied to files:
packages/sqlite-web-core/src/coordination.rs
🧬 Code graph analysis (1)
packages/sqlite-web-core/src/coordination.rs (3)
packages/sqlite-web-core/src/util.rs (4)
sanitize_identifier(5-19)js_value_to_string(39-60)value(44-44)value(50-50)packages/sqlite-web-core/src/worker.rs (1)
serde_wasm_bindgen(34-34)packages/sqlite-web/src/worker.rs (1)
serde_wasm_bindgen(57-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (7)
packages/sqlite-web/src/tests.rs (1)
136-139: LGTM! Test correctly validates query timeout embedding.The assertion properly verifies that the new
__SQLITE_QUERY_TIMEOUT_MSconfiguration is embedded in the generated worker template, consistent with the broader PR changes introducing separate query timeout handling.packages/sqlite-web-core/src/coordination.rs (5)
30-81: LGTM! Configuration extraction is well-structured.The
WorkerConfigandworker_config_from_globalproperly extract configuration from JS globals with appropriate validation (finite, non-negative checks) and sensible defaults (5s follower timeout, 30s query timeout). The separatequery_timeout_msfield addresses the past review concern about query timeout reusing the leader election timeout.
393-445: LGTM! Query handling correctly uses separate timeouts.The query routing logic properly:
- Returns
InitializationPendingimmediately when the DB worker or leader isn't ready (lines 401-406, 411-416), providing fast feedback instead of blocking- Uses
query_timeout_msfor follower query forwarding (line 423), addressing the past review concern about separating leader election timeout from query execution timeout- Tracks pending requests appropriately for both local and forwarded queries
This implementation aligns with issue #21's objective of detecting "no leader yet" and rejecting immediately with a dedicated initialization error.
Based on learnings, this addresses the past feedback about timeout separation.
660-746: LGTM! DB worker queue ensures serial execution.The
DbWorkerStateimplementation:
- Uses a queue with a
db_processingflag (lines 717-719) to ensure queries execute serially, preventing concurrent access issues- Employs the
db.borrow_mut().take()pattern inexec_on_db(lines 883-891), which is documented as safe in this codebase per learnings- Provides a hook-based design enabling testability, as demonstrated in the comprehensive test at lines 1193-1254
Based on learnings, the
db.borrow_mut().take()pattern is safe in this use case.
769-871: LGTM! Comprehensive messaging infrastructure.The worker messaging functions provide:
- Structured error payloads with type discrimination (lines 824-838), enabling downstream handling of
WORKER_ERROR_TYPE_INITIALIZATION_PENDINGvs generic errors- Consistent query result format (lines 840-863) with nullable result/error fields
- Proper error propagation through serialization/posting failures
This supports the issue #21 objective of providing explicit initialization errors rather than generic timeouts.
937-1254: Excellent test coverage of coordinator and DB worker scenarios.The test suite comprehensively validates:
- Configuration extraction with custom and default values (lines 964-984)
- Leadership messaging flow including
NewLeaderandLeaderReady(lines 987-1026)- Conditional responses based on DB readiness state (lines 1029-1078)
- Lock acquisition failure handling (lines 1081-1121)
- DB worker failure recovery, including pending request cleanup and state reset (lines 1124-1190)
- Serial query execution without concurrency violations (lines 1193-1254)
The use of hook-based testing (lines 1196-1217) is particularly good, allowing verification of serial execution without requiring a real database.
packages/sqlite-web/src/worker_template.rs (1)
7-12: The 30-second default query timeout is appropriate.The 30-second timeout aligns with industry standards for SQLite WASM applications. Web research shows typical query timeout ranges from 5–60 seconds depending on use case, with 30 seconds being a common choice for interactive web applications. The implementation supports custom timeouts via
__SQLITE_QUERY_TIMEOUT_MS, allowing adjustment if needed for specific use cases.
| fn handle_db_worker_failure(self: &Rc<Self>, error: String) { | ||
| *self.db_worker_ready.borrow_mut() = false; | ||
| *self.leader_ready.borrow_mut() = false; | ||
| *self.ready_signaled.borrow_mut() = false; | ||
| if let Some(worker) = self.db_worker.borrow_mut().take() { | ||
| worker.terminate(); | ||
| } | ||
| let _ = send_worker_error_message(&error); | ||
| let pending = self.db_pending.borrow_mut().drain().collect::<Vec<_>>(); | ||
| for (_, origin) in pending { | ||
| self.fail_origin(origin, error.clone()); | ||
| } | ||
| if let Err(err) = self.spawn_db_worker() { | ||
| let _ = send_worker_error_message(&js_value_to_string(&err)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider limiting DB worker respawn attempts.
The handle_db_worker_failure function properly cleans up state and pending requests, then immediately attempts to respawn the DB worker (line 388). If the spawn or initialization consistently fails (e.g., due to configuration or OPFS issues), this could create an infinite respawn loop.
Consider adding:
- A respawn attempt counter
- Exponential backoff between attempts
- A maximum retry limit before giving up and keeping the worker in a failed state
This would improve resilience for transient failures while avoiding resource exhaustion on persistent errors.
Motivation
See issues:
We had a problem where during the database initialization, we had to wait a little bit. Because the leadership logic was working in the same worker, we were getting an error until the database was ready. This also means that during any ongoing queries, the page wasn't loading instantly.
This PR aims to fix that by splitting the leadership and the database logic in two different workers. So the leadership selection can be made instantly regarding the database status.
Solution
fix #21
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Refactor
New Features
Tests
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.