docs: document SqliteStore::share_for_device - #510
Conversation
whatsapp-rust#1274 added an opt-in way for sibling devices on the same database file to share one connection pool instead of opening one per session, cutting per-session RSS from ~122.6 KiB to ~0 KiB marginal at the cost of serializing sibling writes. Document the new method under SqliteStore implementation and cross-link it from the memory/thread tuning guide. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVL1XzB9C53v9DYKRmGXqW
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
📝 WalkthroughWalkthroughThe documentation adds ChangesSQLite store sharing
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/store.mdx`:
- Line 530: Rewrite the SqliteStore::share_for_device() subsection in second
person with concise active-voice sentences, replacing third-person subjects with
“you.” Split the combined prose into separate sentences covering pool sharing,
memory usage, SQLite lookaside/cache behavior, and differing device_id values;
format restore as inline code when referring to the provisioning API, and apply
the same style to the related text at the referenced locations.
- Around line 530-548: Revise the new subsections in api/store.mdx lines 530-548
and concepts/storage.mdx lines 984-992 to use active voice and address the
reader as “you.” Split long statements into concise sentences, separating
operational, memory, sharing, and throughput claims; in api/store.mdx, format
the restore provisioning API as code.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da7a241c-eb05-41d0-aa57-b83cab2242c3
📒 Files selected for processing (2)
api/store.mdxconcepts/storage.mdx
Match the house style (AGENTS.md: active voice, second person "you", one idea per sentence) per CodeRabbit review feedback on #510. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVL1XzB9C53v9DYKRmGXqW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 242e0130e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub fn share_for_device(&self, device_id: i32) -> Self; | ||
| ``` | ||
|
|
||
| `SqliteStore::share_for_device()` returns a new `SqliteStore` for a *sibling device* in the same database file, cloning this store's pool, write-serialization semaphore, and reader pool instead of opening another connection. Every constructor builds its own r2d2 pool, so a process holding N sessions against one database file today opens N connections — and a connection costs memory before it reads a single row: a fixed ~46.9 KiB lookaside slab (`SQLITE_DEFAULT_LOOKASIDE`, not reducible via any pragma — `SQLITE_DBCONFIG_LOOKASIDE` is C-API only and unreachable through diesel) plus a page cache that grows to [`cache_size_kib`](#database-configuration). Since every query already carries a `device_id`, sibling sessions on one database only ever needed that field to differ. |
There was a problem hiding this comment.
Split the pool-cost paragraph into concise sentences
Break this paragraph into shorter sentences. It combines pool construction, connection counts, lookaside configuration, page-cache behavior, and device scoping in one dense line, which violates the project's requirement to keep sentences concise and cover one idea at a time. The similarly long performance sentence below should be split as part of the same correction.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
| pub fn share_for_device(&self, device_id: i32) -> Self; | ||
| ``` | ||
|
|
||
| `SqliteStore::share_for_device()` returns a new `SqliteStore` for a *sibling device* in the same database file, cloning this store's pool, write-serialization semaphore, and reader pool instead of opening another connection. Every constructor builds its own r2d2 pool, so a process holding N sessions against one database file today opens N connections — and a connection costs memory before it reads a single row: a fixed ~46.9 KiB lookaside slab (`SQLITE_DEFAULT_LOOKASIDE`, not reducible via any pragma — `SQLITE_DBCONFIG_LOOKASIDE` is C-API only and unreachable through diesel) plus a page cache that grows to [`cache_size_kib`](#database-configuration). Since every query already carries a `device_id`, sibling sessions on one database only ever needed that field to differ. |
There was a problem hiding this comment.
Qualify the lookaside allocation as build-dependent
Qualify the 46.9 KiB figure as applying to the tested bundled SQLite build. SQLITE_DEFAULT_LOOKASIDE is a compile-time setting, so users who follow the preceding instructions to link a system SQLite—or the later SQLCipher instructions—can have a different default size or no lookaside allocation at all; presenting this as a fixed cost for every connection makes the memory guidance inaccurate in those supported configurations.
Useful? React with 👍 / 👎.
| What it does **not** do: | ||
|
|
||
| - **Create the device row.** It only stamps queries with `device_id`; the row still comes from the usual provisioning path (`create_new_device` or restore), same as a store built with `new_for_device`. | ||
| - **Isolate writes.** Siblings share the write permits set by [`SqliteStoreConfig::pool_size`](#database-configuration) — at the default of `1`, their writes serialize against each other. Measured on a burst where every sibling writes continuously, sharing costs roughly 2.5x the aggregate write throughput of a pool per session, in exchange for FIFO-fair scheduling across siblings (a private connection per session instead leaves fairness to SQLite's busy handler, which backs off randomly — about 2x the spread between the fastest and slowest session). |
There was a problem hiding this comment.
Remove the claim that SQLite randomizes busy backoff
Correct the fairness comparison for the documented PRAGMA busy_timeout configuration. SQLite's standard timeout handler performs repeated sleeps until the configured timeout is exhausted; it does not provide the randomized backoff claimed here. The same assertion is repeated in concepts/storage.mdx, so both passages currently give readers a false explanation for the measured fairness difference.
Useful? React with 👍 / 👎.
| pub fn share_for_device(&self, device_id: i32) -> Self; | ||
| ``` | ||
|
|
||
| `SqliteStore::share_for_device()` returns a new `SqliteStore` for a *sibling device* in the same database file, cloning this store's pool, write-serialization semaphore, and reader pool instead of opening another connection. Every constructor builds its own r2d2 pool, so a process holding N sessions against one database file today opens N connections — and a connection costs memory before it reads a single row: a fixed ~46.9 KiB lookaside slab (`SQLITE_DEFAULT_LOOKASIDE`, not reducible via any pragma — `SQLITE_DBCONFIG_LOOKASIDE` is C-API only and unreachable through diesel) plus a page cache that grows to [`cache_size_kib`](#database-configuration). Since every query already carries a `device_id`, sibling sessions on one database only ever needed that field to differ. |
There was a problem hiding this comment.
Describe sharing as removing a pool rather than one connection
Scope the claim that N sessions open N connections to the default configuration. with_config_for_device can set pool_size above one and add read_pool_size, so each session can own multiple physical connections and sharing removes that entire pool. The current wording, also repeated as “removes the per-session connection” in concepts/storage.mdx, gives an incorrect connection and memory baseline for supported custom configurations.
Useful? React with 👍 / 👎.
Per Codex review feedback on #510: - Scope the "N connections" claim to the default pool_size=1/ read_pool_size=0 config; a store with either raised opens more per session, and sharing removes the whole pool, not one connection. - Note the ~46.9 KiB lookaside figure is specific to the bundled SQLite build; a system-linked or SQLCipher build can size or omit it differently. - Drop the claim that SQLite's busy-timeout handler backs off "randomly" — its retry schedule is deterministic per connection, it's just uncoordinated across connections. Keep the measured ~2x spread. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVL1XzB9C53v9DYKRmGXqW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1608020a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| What it does **not** do: | ||
|
|
||
| - **Create the device row.** It only stamps queries with `device_id`. You still need to provision the row through the usual path — `create_new_device` or a restore — the same as with a store built from `new_for_device`. |
There was a problem hiding this comment.
Remove the manual provisioning requirement
When you pass the returned store through the normal PersistenceManager initialization path, it already checks backend.exists() and calls backend.create() when the scoped row is absent (concepts/storage.mdx lines 59–64). Telling users they must provision the row themselves is therefore incorrect, and the suggested create_new_device API is not documented anywhere in the repository; clarify that share_for_device itself does not create the row, but normal client initialization does.
Useful? React with 👍 / 👎.
Per Codex review feedback on #510: PersistenceManager::new() already calls backend.exists()/backend.create() for the store's current device_id (confirmed in whatsapp-rust's DeviceStore::create impl, which delegates to create_new_device(self) using self.device_id). Manual provisioning is only needed if a share_for_device() store is used directly, outside PersistenceManager. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVL1XzB9C53v9DYKRmGXqW
Summary
Documents the new
SqliteStore::share_for_deviceAPI added in oxidezap/whatsapp-rust#1274, which lets sibling devices on the same database file share one r2d2 connection pool (and its write-serialization semaphore + reader pool) instead of each session opening its own connection.Changes
api/store.mdx— new "Sharing the pool with sibling devices" subsection under SqliteStore implementation, alongside the existing "Sharing the pool with sibling crates" (shared()) section. Documents the method signature, usage example, and the three things it deliberately does not do (create the device row, isolate writes, splitresource_report()), including the measured ~2.5x write-throughput trade-off for FIFO-fair scheduling.concepts/storage.mdx— new "Sharing one pool across sibling devices" subsection at the end of Memory and Thread Tuning, cross-linking the new method and giving the measured per-session RSS numbers (~122.6 KiB marginal for a private pool vs. ~0.0 KiB for a shared one on an idle session).Per-changelog entries were intentionally left out of this PR (changelog is human-authored only).
Test plan
/api/store#sharing-the-pool-with-sibling-devices,/concepts/storage#memory-and-thread-tuning-sqlitestoreconfig)mint broken-links(not run — Mintlify CLI unavailable in this environment)Generated by Claude Code
Summary by cubic
Documented
SqliteStore::share_for_device, which lets sibling devices on the same database file share oner2d2pool instead of opening one per session. Updated docs to cover usage and provisioning (device rows auto-created byPersistenceManager::new; manual only if using the store directly), limits (shared write permits, sharedresource_report()), trade-offs (~2.5x slower aggregate writes on continuous bursts), memory savings (~122.6 KiB → ~0 KiB per idle session), and cross-links inapi/store.mdxandconcepts/storage.mdx; plus clarifications on defaultpool_size=1/read_pool_size=0(sharing removes the whole pool), bundled lookaside (~46.9 KiB), and deterministic but uncoordinated busy-timeout (~2x spread).Written for commit 48ad1fd. Summary will update on new commits.
Summary by CodeRabbit