Skip to content

Delivering one message costs O(whole mail store): 71ms at 5,000 messages, under the global lock #1464

Description

@asim

Measured, not asserted

Benchmark added in b1a69d3a (service/mail/bench_test.go):

go test ./service/mail/ -run xxx -bench AsTheStoreGrows -benchtime 20x
messages already stored one delivery one mail_inbox
100 1.4 ms 0.46 ms
1,000 10.4 ms 4.4 ms
5,000 71.3 ms 20.6 ms

Worse than linear — 5x the messages costs 6.9x the time.

Why

SendMessageTo does two things over the whole store, not over the message:

  1. rebuildInboxes() — throws away inboxes and reconstructs every account's threads from every message, on every delivery. Called from 8 places.
  2. save() — marshals, encrypts and rewrites the entire mail.json.

Both run holding the package's write mutex, so every reader and every other writer on the instance waits behind them. At 5,000 messages that is a 71ms global stall per message delivered. At 50,000 it is closer to a second.

Reads are linear too: messages is one flat slice with no index, so ListMessages, MessageUnlocked, byMessageIDUnlocked, imapFolder and about twenty other loops each scan all of it.

Why it is invisible until it isn't

Nothing gets slower in a way anyone notices — it degrades by a millisecond a week. A test store never reaches the size where it matters, and the instance that does has no benchmark watching it. The numbers above exist so this argues from measurement.

Directions, cheapest first

  • Do not rebuild everything to add one message. addMessageToInbox already exists and takes a single message; delivery can update the affected inboxes incrementally instead of discarding the map. This is the single biggest win and does not change the storage format.
  • Do not rewrite the file per message. Coalesce saves on a short timer, as flushUIDs already does for the IMAP numbering — the pattern is in the package with a comment explaining exactly this reasoning.
  • Index by what is looked up. id → *Message, Message-ID → *Message, account → []*Message. Turns the common lookups from O(n) into O(1) and removes the ~20 hand-written scans.
  • Then consider the store. Whole-file JSON is the underlying hazard and it is also Data integrity: one bug wiped a live store, and there are no backups #1458's subject — an append-only log or SQLite would fix both, and neither is worth doing before the three above.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions