docs: reflect Signal-store read routing from whatsapp-rust#1222 - #484
Conversation
read_pool_size now also governs SqliteStore's own SignalStore/ AppSyncStore/ProtocolStore/DeviceStore reads (session, identity, sender-key, pre-key lookups), not just whatsapp-rust-chat-store's queries. Notes the deliberate exceptions kept on the write queue. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016x8h4pG6KBbRULQ1cBfzen
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
📝 WalkthroughWalkthroughThe documentation clarifies SQLite read routing. It lists lookup paths that use the reader pool and reads that remain on the write queue for consistency. ChangesSQLite read routing
Estimated code review effort: 1 (Trivial) | ~2 minutes 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 |
|
| Filename | Overview |
|---|---|
| api/store.mdx | Updates SharedSqlite::read guidance and now includes all three reasons consistency-sensitive reads remain on run. |
| concepts/storage.mdx | Explains the expanded read_pool_size coverage, concurrency implications, and deliberately excluded reads. |
Reviews (2): Last reviewed commit: "docs: address review feedback on read-po..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 519: Update the permit-count wording in api/store.mdx:519 and
concepts/storage.mdx:968-970 to describe the write path's configured permits
rather than a single permit, while preserving any default-case qualification if
needed. Ensure both notes accurately reflect SqliteStoreConfig::pool_size, which
may be greater than one.
In `@concepts/storage.mdx`:
- Around line 968-970: Update the final sentence of the Note to limit the
concurrency claim to pooled reads, replacing “the whole read surface” with “most
of the read surface” or equivalent wording such as “all reads routed through the
reader pool.” Preserve the preceding examples of reads that remain on the write
queue.
🪄 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: 19fff44c-c2db-4220-a7fe-8dd31f6d9de3
📒 Files selected for processing (2)
api/store.mdxconcepts/storage.mdx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86f2b2b39f
ℹ️ 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".
| </Note> | ||
|
|
||
| <Note> | ||
| As of [whatsapp-rust#1222](https://github.com/oxidezap/whatsapp-rust/pull/1222), `read_pool_size` also covers most of `SqliteStore`'s own `SignalStore`/`AppSyncStore`/`ProtocolStore`/`DeviceStore` reads, not just [`whatsapp-rust-chat-store`](/api/chat-store)'s queries. Session, identity, sender-key, and pre-key lookups on the decrypt path — `get_session`, `load_identity`, `get_sender_key`, `load_prekey`, and similar — now run on the reader pool instead of queueing behind `pool_size`'s single write permit, raising the ceiling on read concurrency during a write-behind flush. A handful of reads stay on the write queue by design because a stale answer would go out on the wire, fail an operation outright, or get promoted into a cache unconditionally — app-state sync key lookups, `messageSecret` reads, and `get_devices` among them. Widening `read_pool_size` past `0` now buys concurrency for the whole read surface, not just chat/message queries. |
There was a problem hiding this comment.
Rewrite this note as several short, reader-focused sentences: it currently combines coverage, examples, performance effects, exceptions, and a summary in one very long sentence, while api/store.mdx:519 similarly extends an already long sentence. This makes the configuration guidance difficult to scan and violates the repository requirement to keep one idea per sentence.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
| </Note> | ||
|
|
||
| <Note> | ||
| As of [whatsapp-rust#1222](https://github.com/oxidezap/whatsapp-rust/pull/1222), `read_pool_size` also covers most of `SqliteStore`'s own `SignalStore`/`AppSyncStore`/`ProtocolStore`/`DeviceStore` reads, not just [`whatsapp-rust-chat-store`](/api/chat-store)'s queries. Session, identity, sender-key, and pre-key lookups on the decrypt path — `get_session`, `load_identity`, `get_sender_key`, `load_prekey`, and similar — now run on the reader pool instead of queueing behind `pool_size`'s single write permit, raising the ceiling on read concurrency during a write-behind flush. A handful of reads stay on the write queue by design because a stale answer would go out on the wire, fail an operation outright, or get promoted into a cache unconditionally — app-state sync key lookups, `messageSecret` reads, and `get_devices` among them. Widening `read_pool_size` past `0` now buys concurrency for the whole read surface, not just chat/message queries. |
There was a problem hiding this comment.
Replace “the whole read surface” with “most of the read surface” or similarly qualified wording. The same sentence explicitly states that app-state sync key, messageSecret, and get_devices reads remain on the write queue, so read_pool_size > 0 does not provide concurrency for the whole read surface and the conclusion can mislead readers sizing this pool.
Useful? React with 👍 / 👎.
- Note stale reads can fail an operation outright (api/store.mdx), matching the same claim already made in concepts/storage.mdx. - Say "write-path permits" instead of "single permit" since pool_size can be configured above its default of 1. - Say "most of the read surface" instead of "the whole read surface" — a handful of reads stay on the write queue by design. - Split the two long notes into shorter, one-idea sentences. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016x8h4pG6KBbRULQ1cBfzen
|
Pushed 1577f3c addressing the review feedback from CodeRabbit, Greptile, and Codex:
Generated by Claude Code |
2 participants
Summary
whatsapp-rust#1222 ("perf(sqlite): route Signal reads through the read pool") changed what
SqliteStoreConfig::read_pool_sizeactually covers. Previously, onlywhatsapp-rust-chat-store's queries went throughSharedSqlite::read— session, identity, sender-key, and pre-key lookups on the Signal decrypt path (get_session,load_identity,get_sender_key,load_prekey, and similar) stayed on the single write-path permit regardless ofread_pool_size. That PR routed most ofSqliteStore's ownSignalStore/AppSyncStore/ProtocolStore/DeviceStoreread methods through the reader pool too, deliberately holding back a handful whose stale answer would go out on the wire, fail an operation outright, or get promoted into a cache unconditionally (app-state sync key lookups,messageSecretreads,get_devices, etc.).This is a real behavior change for anyone who has turned
read_pool_sizeon: it now buys concurrency for the whole read surface, not just chat/message queries.Changes
concepts/storage.mdx— added a note under theSqliteStoreConfigtuning section explaining the widenedread_pool_sizecoverage and the reads deliberately excluded.api/store.mdx— updated theSharedSqlite::read/runnote to say thatSqliteStore's own read methods now usereadthe same waywhatsapp-rust-chat-storedoes, with the exceptions called out.No changelog entries were touched, per instructions.
Generated by Claude Code
Summary by cubic
Update docs to reflect
whatsapp-rust#1222: most ofSqliteStore’s Signal-store reads now use the read pool, soSqliteStoreConfig::read_pool_sizecovers more than justwhatsapp-rust-chat-storequeries. Clarifies that some reads still queue on write-path permits (app-state sync key lookups,messageSecret,get_devices) and that stale reads can fail an operation outright.Written for commit 1577f3c. Summary will update on new commits.