PERF: Notes IMAP SELECT and STATUS scan every Note under the User writer lock #780

Open
opened 2026-10-02 13:10:35 +00:00 by kayg · 3 comments
Owner

Found in the read-mostly server architecture audit #663. Applies DESIGN §58 rules 1, 2 and 8 from queued job/instant-663. Source base origin/dev = c4a61e8cf090170f35b1bed3350d9de20c83ecd5; pending origin/job/merge-round-7a = 2f4482ded066d9c5d9c59130377907f7fd2916c9. This is structural evidence, not a measured latency or confirmed security blocker.

Context: Apple Notes opens folders and asks STATUS during normal sync. The read adapter is a separate hot path from HTTP Note reads (#702) and IMAP APPEND latency (#550).

Evidence:

  • crates/plugins/notes/src/imap.rs:738–843: snapshot takes crate::lock_user, then INSERTs note_imap_clock through writer_pool on every call, reads the clock/live revisions/flags, and synchronously reads and hashes each live Note before calculating mailbox membership from Markdown.
  • It reads up to 4,096 live revisions across all folders before filtering. The 16 MiB selected-source cap applies only after reading and filtering each file.
  • crates/calternal-imap/src/session.rs:319,353,609: STATUS, SELECT/EXAMINE and refresh all call provider.select. Refresh builds the entire next snapshot before checking whether highest_modseq is unchanged.
  • Round 7a retains this work at notes/src/imap.rs:761–866; #644/#646 repair Apple sync correctness, not this read architecture.
  • FETCH headers and RFC822.SIZE also render full MIME at session.rs:431–455; notes/src/imap.rs:846–885 reads cached wrapper data or parses Markdown and builds attachment MIME. The temporary render cache lasts only one message in one FETCH call.

Reasoned User impact: one folder STATUS costs O(all live Notes) file operations and parsing. Several folders repeat the scan. Reads queue behind Note edits/reconciliation and hold the same lock while all bodies are read. An empty folder is not cheap. No claim of a measured time or unbounded allocation.

Concrete fix: publish stable clock, mailbox membership, date and immutable source/body references together with the existing revision projection at ingest. Do not mint clock state during a healthy read. Query selected mailbox metadata through WAL without the User writer lock. Keep the protocol's finite mailbox limits, but apply them to selected metadata before loading bodies. On refresh, check the clock first. Load bodies lazily at FETCH and cache immutable MIME/header/size projections by User and revision with a byte cap. Preserve UIDVALIDITY, UID/modseq order and #644 served-base merge rules. Reuse #702 projections rather than another independent Markdown parser/cache.

Regression tests: hold the Notes User writer lock and SQLite writer connection; warm SELECT, empty-folder STATUS and unchanged refresh still complete from the committed reader projection with zero Home reads/parses/writes. Count reads for a small selected folder within 4,096 total Notes. Repeated header/size FETCH must not load all attachments again. Verify UID clocks and APPEND/EXPUNGE correctness, two-User isolation, restart, and changed membership.

Validation for the fix: preserve existing assertions and protocol status codes. Run per-crate fmt/clippy/test, plus calternal-server if the route or provider contract changes. Extend an existing bench profile with warm/cold latency, CPU, RSS and a realistic large-data burst. Measure ≥5 samples on the perf VM under /root/perf.lock with load recorded inside the lock and HDD emulation; compare only a matching baseline. The audit itself did not run a server or benchmark. No product edit is requested from the audit branch.

Found in the read-mostly server architecture audit #663. Applies DESIGN §58 rules 1, 2 and 8 from queued `job/instant-663`. Source base `origin/dev` = `c4a61e8cf090170f35b1bed3350d9de20c83ecd5`; pending `origin/job/merge-round-7a` = `2f4482ded066d9c5d9c59130377907f7fd2916c9`. This is structural evidence, not a measured latency or confirmed security blocker. Context: Apple Notes opens folders and asks STATUS during normal sync. The read adapter is a separate hot path from HTTP Note reads (#702) and IMAP APPEND latency (#550). Evidence: - `crates/plugins/notes/src/imap.rs:738–843`: snapshot takes `crate::lock_user`, then INSERTs note_imap_clock through writer_pool on every call, reads the clock/live revisions/flags, and synchronously reads and hashes each live Note before calculating mailbox membership from Markdown. - It reads up to 4,096 live revisions across all folders before filtering. The 16 MiB selected-source cap applies only after reading and filtering each file. - `crates/calternal-imap/src/session.rs:319,353,609`: STATUS, SELECT/EXAMINE and refresh all call provider.select. Refresh builds the entire next snapshot before checking whether highest_modseq is unchanged. - Round 7a retains this work at `notes/src/imap.rs:761–866`; #644/#646 repair Apple sync correctness, not this read architecture. - FETCH headers and RFC822.SIZE also render full MIME at `session.rs:431–455`; `notes/src/imap.rs:846–885` reads cached wrapper data or parses Markdown and builds attachment MIME. The temporary render cache lasts only one message in one FETCH call. Reasoned User impact: one folder STATUS costs O(all live Notes) file operations and parsing. Several folders repeat the scan. Reads queue behind Note edits/reconciliation and hold the same lock while all bodies are read. An empty folder is not cheap. No claim of a measured time or unbounded allocation. Concrete fix: publish stable clock, mailbox membership, date and immutable source/body references together with the existing revision projection at ingest. Do not mint clock state during a healthy read. Query selected mailbox metadata through WAL without the User writer lock. Keep the protocol's finite mailbox limits, but apply them to selected metadata before loading bodies. On refresh, check the clock first. Load bodies lazily at FETCH and cache immutable MIME/header/size projections by User and revision with a byte cap. Preserve UIDVALIDITY, UID/modseq order and #644 served-base merge rules. Reuse #702 projections rather than another independent Markdown parser/cache. Regression tests: hold the Notes User writer lock and SQLite writer connection; warm SELECT, empty-folder STATUS and unchanged refresh still complete from the committed reader projection with zero Home reads/parses/writes. Count reads for a small selected folder within 4,096 total Notes. Repeated header/size FETCH must not load all attachments again. Verify UID clocks and APPEND/EXPUNGE correctness, two-User isolation, restart, and changed membership. Validation for the fix: preserve existing assertions and protocol status codes. Run per-crate fmt/clippy/test, plus calternal-server if the route or provider contract changes. Extend an existing bench profile with warm/cold latency, CPU, RSS and a realistic large-data burst. Measure ≥5 samples on the perf VM under /root/perf.lock with load recorded inside the lock and HDD emulation; compare only a matching baseline. The audit itself did not run a server or benchmark. No product edit is requested from the audit branch.
Author
Owner

Starting work on job/notesperf, based on job/merge-round-7a at 2f4482ded066d9c5d9c59130377907f7fd2916c9. I will address the issue with focused changes and regression coverage, then report the final head SHA and verbatim gate output here.

Starting work on `job/notesperf`, based on `job/merge-round-7a` at `2f4482ded066d9c5d9c59130377907f7fd2916c9`. I will address the issue with focused changes and regression coverage, then report the final head SHA and verbatim gate output here.
Author
Owner

STATUS now uses indexed counters, SELECT filters Tag membership before the 4096 limit and reads only selected source files, and unchanged NOOP/IDLE refreshes check the revision clock before rebuilding. Healthy reads use WAL and do not take the Notes writer lock. Empty reads derive the initial nonzero UIDVALIDITY from the immutable User UUID; the first committed change persists the same value, so concurrent empty reads need no clock write and cannot mint different epochs. Evidence: cargo test -p calternal-plugin-notes --lib empty_mailbox_reads_do_not_need_user_or_database_writer and ... tagged_mailbox_filters_revisions_before_the_corpus_limit passed; cargo test -p calternal-imap --test session passed (8 tests). Commit: 3537938f1. A performance run is still pending.

STATUS now uses indexed counters, SELECT filters Tag membership before the 4096 limit and reads only selected source files, and unchanged NOOP/IDLE refreshes check the revision clock before rebuilding. Healthy reads use WAL and do not take the Notes writer lock. Empty reads derive the initial nonzero UIDVALIDITY from the immutable User UUID; the first committed change persists the same value, so concurrent empty reads need no clock write and cannot mint different epochs. Evidence: `cargo test -p calternal-plugin-notes --lib empty_mailbox_reads_do_not_need_user_or_database_writer` and `... tagged_mailbox_filters_revisions_before_the_corpus_limit` passed; `cargo test -p calternal-imap --test session` passed (8 tests). Commit: 3537938f1. A performance run is still pending.
Author
Owner

Implemented in 3537938f1: metadata-only STATUS, SQL filtering before the SELECT cap, and unchanged refresh via revision clock. Added select benchmark scenario; perf VM lock was busy and local server did not become ready within 30s under shared load, so no measurements are reported.

Implemented in 3537938f1: metadata-only STATUS, SQL filtering before the SELECT cap, and unchanged refresh via revision clock. Added select benchmark scenario; perf VM lock was busy and local server did not become ready within 30s under shared load, so no measurements are reported.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#780
No description provided.