diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-20 10:18:30 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-20 10:18:30 +0200 |
| commit | 694ff89b51873f4edf3328b639f421d8e0178dc9 (patch) | |
| tree | 95bae887d45d0cbd7a633c0c6091067515f159d9 /docs | |
| parent | 98914f2c184539c9be9cac7c3ae8def6820868b8 (diff) | |
| download | qtmaildir-694ff89b51873f4edf3328b639f421d8e0178dc9.tar.gz qtmaildir-694ff89b51873f4edf3328b639f421d8e0178dc9.zip | |
docs: close item 124, record the spinner defect, keep 121 open
Item 124 shipped and is proven: the index moved from a 7200rpm platter to
NVMe with the mail staying at /data/Mail. Cold start went from 38618 ms to
668 ms for a complete walk, and 2008 ms to 50 ms for the first rows.
Counts held at 49174 messages / 5594 inbox / 100 tags at every step, and
Delete then Restore round-tripped through the account's trash by hand.
Item 121 stays open, and its entry now says why. The measured platter
figures are the evidence FOR building the indicator, not against it: a
mechanical disk is the cheap configuration, not an exotic one, and a user
with a large Maildir on spinning rust has nowhere to migrate to. Fixing
one developer's hardware is not fixing the application. The constraint
that pointed at item 124 as the answer is replaced by one saying the
opposite, and prefaulting stays rejected on its own merits since it is
worst on the low-memory machines most likely to have a slow disk.
Item 125 is new, found by hand during the migration. mailsync.sh exits 75
(EX_TEMPFAIL) when another run holds the lock, and the sync indicator
never clears; because an edit made during a sync is held until the sync
ends, a Delete sat queued for a completion that could not arrive and
looked like it had done nothing. Nothing was lost, since held edits reach
the disk, but the user cannot tell that.
Also records in CLAUDE.md that notmuch_database_get_path() is not the mail
root, that database.hook_dir defaults into the index directory and
silently stops post-new under a split config, and that the ordinary
fixture layout cannot tell the two accessors apart.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 100 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 153 |
2 files changed, 185 insertions, 68 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index e7ef7db..4a4b1ff 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -5040,6 +5040,106 @@ when the write is moved above the guard. The cold-cache cost measured above was not touched and should not be. Nothing about the timing changed. +## 124. The worker reads the index directory as the mail root + +**Observed (measured, 2026-08-20):** not reported from use. Found while +measuring item 121's cold-start cost, which established that the 1.1 GB notmuch +index sits on a 7200rpm platter (`/dev/sda1`, `rotational: 1`) while an NVMe SSD +sits idle in the same machine. Moving the index to the SSD is a notmuch +configuration change and needs no code, but qtmaildir does not survive it. + +**Cause (verified against notmuch 0.39 and a throwaway database).** notmuch +supports splitting the index from the mail with two keys: + +```ini +[database] +mail_root=/data/Mail +path=/home/you/.local/share/notmuch +``` + +Under that layout `notmuch_database_get_path()` returns the **index** +directory, not the mail root. Measured on a split test database: + +| accessor | legacy (`path` only) | split | +|---|---|---| +| `notmuch_database_get_path()` | mail root | **index dir** | +| `notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT)` | mail root | mail root | + +`notmuchworker.cpp` calls `get_path()` at six sites and treats every one as the +mail root: lines 297, 755, 900, 901, 1062, and the `relativeFilePath()` calls +at 364, 380 and 918 that consume them. + +**One of those six moves mail, and it is the dangerous one.** +`moveMessages` composes its destination as +`root + "/" + destFolder + "/cur"` (`src/notmuchworker.cpp:755`). Under a split +config `root` is the index directory, so Delete would move the message into +`<index>/Trash/cur`: outside the Maildir, invisible to mbsync, and gone from +every other client. That is item 103's stranded-mail failure with a new cause, +and this repo has already shipped that class of bug once. + +The rest degrade rather than destroy. `relativeFilePath()` against the wrong +root yields `../../../data/Mail/account/cur/...` instead of `account/cur/...`, +so no path matches an account prefix and every row resolves to no account, +which is exactly what the comment at line 294 already warns about. + +**Approach.** Replace `notmuch_database_get_path()` with +`notmuch_config_get(m_db, NOTMUCH_CONFIG_MAIL_ROOT)` at the six sites. + +**No conditional is needed, and that is the point.** `MAIL_ROOT` returns the +mail root under BOTH layouts, verified above: under a legacy `path`-only config +it equals `get_path()`, so the change is a no-op against the current +configuration and correct against the split one. A fallback to `get_path()` when +`MAIL_ROOT` is NULL is the tempting belt-and-braces addition and should be +resisted unless a NULL is actually observed, since it reintroduces the wrong +answer on the path where it matters. + +**Constraints.** + +- **`moveMessages` is the site to test hardest.** A wrong root there reaches the + mail server, per the "Delete MOVES the file" note in `CLAUDE.md`. +- The test fixture must build a database whose index is NOT inside the mail + root, or it cannot tell the two accessors apart: under the ordinary fixture + layout both return the same string and a mutation stays green. +- `notmuch_config_get` returns a string owned by notmuch (`notmuch.h:2585`); + do not free it. +- Nothing about the legacy layout may change. The migration is the user's to + perform, separately, once this ships. + +**Size: S.** Six call sites and a fixture that can tell them apart. + +**Closed 2026-08-20, unreleased.** `mailRootOf()` in the anonymous namespace of +`notmuchworker.cpp` wraps `notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT)`, and +the four `notmuch_database_get_path()` call sites use it. No conditional: the +accessor is correct under both layouts, verified against the live database where +it returned the same string as `get_path()` before the migration. + +Three tests in `test_notmuchworker.cpp`, all requiring the fixture's new opt-in +`splitIndex()`. That flag is load-bearing rather than convenient: in the +ordinary layout the index sits inside the mail root and both accessors return +the same string, so a test written against it passes whichever one the code +uses. All three fail against the old accessor, confirmed by mutation, with the +move test failing outright rather than subtly. + +**Proven in production the same day.** The developer's own index moved from a +7200rpm platter to NVMe, mail staying at `/data/Mail`: + +| phase | platter, cold | NVMe, cold | +|---|---|---| +| `search_threads` | 673 ms | 12 ms | +| first 200 rows | 2008 ms | 50 ms | +| complete walk, 4634 threads | 38618 ms | 668 ms | + +Counts held at 49174 messages / 5594 inbox / 100 tags at every step, and Delete +then Restore round-tripped through `[Gmail]/Cestino` by hand, which is the +behaviour this item existed to protect. + +**Two things the migration taught that are not in the code.** `database.hook_dir` +defaults to `<database.path>/.notmuch/hooks`, so a split config silently stops +running `post-new`: `notmuch new` reports success and tags nothing. It must be +set explicitly. And the procedure's own first step, holding `/tmp/mbsync.lock`, +conflicts with hand-testing Delete, which auto-syncs; that surfaced item 125. +The procedure lives outside this repo, in the user's own documents. + ## 90. A saved-query button clears the account selection **Observed (user, notes):** "select an account and hit the 'unread' button, the diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 4039753..8129704 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -191,7 +191,9 @@ taking that too literally. | 122 | The README documents a version of the app that no longer exists | documentation | M | open, 2026-08-20, from the notes. Delete-to-trash is entirely undocumented, including a config key a user must now set | | 123 | Sending mail is not designed | v2 | ? | open, 2026-08-20, from the notes. Brainstorm only, explicitly `#plan-only`; the user places most open UX behind it | -| 124 | The worker reads the index directory as the mail root | defect | S | open, 2026-08-20. Blocks moving the index to an SSD. Under a split `mail_root`/`path` config, Delete would move mail INSIDE the index directory, where mbsync cannot see it | +| 124 | The worker reads the index directory as the mail root | defect | S | **done** 2026-08-20, unreleased. `mailRootOf()` over `NOTMUCH_CONFIG_MAIL_ROOT`, correct under both layouts. Verified by migrating the developer's own index to NVMe the same day: cold start 38.6 s to 0.67 s | + +| 125 | A skipped sync leaves the spinner running for ever | defect | S | open, 2026-08-20, found by hand. `mailsync.sh` exits 75 (EX_TEMPFAIL) when another run holds the lock; the indicator never clears, and a held edit waits for a completion that never comes | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -849,11 +851,27 @@ cold boot on this hardware: So the list is blank for **two seconds**, and keeps growing for **thirty-eight**, on 4% more mail. The user's note said "several seconds" and the note was right. -**The cause is the storage, not the code.** `/data` is `/dev/sda1`, a 7200rpm -platter (`rotational: 1`); warm, the identical walk is 154 ms, a 250x -difference. Item 124 is the prerequisite for moving the index to the NVMe SSD -already in the machine, which would make this gap ~12 ms and reduce this item to -a nicety. +**The cause is the storage, not the code**, and that is the reason to BUILD +this rather than to skip it. `/data` was `/dev/sda1`, a 7200rpm platter +(`rotational: 1`); warm, the identical walk is 154 ms, a 250x difference. + +**The developer's own index moved to NVMe on 2026-08-20** (item 124 was its +prerequisite), which took the cold figures to 12 ms / 50 ms / 668 ms and makes +this invisible *on that machine*. That is precisely why the item stays open. A +mechanical disk is not an exotic configuration, it is the cheap one, and a user +who keeps a large Maildir on spinning rust has nowhere to migrate to. The +measurements above are now the best evidence this project has for what such a +user sees on every cold start, and they were taken on real mail rather than +simulated: + +| storage | first rows | complete walk | +|---|---|---| +| 7200rpm platter, cold | 2008 ms | 38618 ms | +| NVMe, cold | 50 ms | 668 ms | + +Fixing one developer's hardware is not fixing the application. The indicator is +what makes a slow query legible on any disk, and the slower the disk the more it +matters. **Approach.** A busy state on the left pane between `runQuery` and the first `onThreadsReady`, cleared by whichever of the first batch or `queryFinished` @@ -875,10 +893,17 @@ the usual reason a spinner does not spin. ignored that guard would make every cron sync flash the list. That silence is already a test, and it should cover this too. - **Item 74's decision not to address the cold cost was taken on wrong - numbers** and is worth revisiting, though not here. It judged a 5.7 s wait not - worth prefaulting 1.1 GB; the real figure is 38.6 s. The answer is not - prefaulting either way: it is item 124 plus moving the index off the platter. - This item makes the remaining wait legible, nothing more. + numbers**, and its conclusion still holds for a different reason. It judged a + 5.7 s wait not worth prefaulting 1.1 GB; the real figure was 38.6 s. Do not + reopen prefaulting: it trades a large fixed cost at every startup against a + wait that only some users pay, and it is worse on exactly the low-memory + machines most likely to have a slow disk. + +- **Do not treat "move the index to an SSD" as this item's fix.** It is the + right advice for a user who has an SSD, and it is documented, but it is + hardware guidance rather than a change to the application. This item must + stand on its own for a user with one mechanical disk and no migration + available. - Nothing about the query timing may change. **Size: S.** @@ -969,72 +994,64 @@ queued message reaches the sending script. **Size: `?`** until the brainstorm has happened. It is the largest open item by some distance. -## 124. The worker reads the index directory as the mail root +## 125. A skipped sync leaves the spinner running for ever -**Observed (measured, 2026-08-20):** not reported from use. Found while -measuring item 121's cold-start cost, which established that the 1.1 GB notmuch -index sits on a 7200rpm platter (`/dev/sda1`, `rotational: 1`) while an NVMe SSD -sits idle in the same machine. Moving the index to the SSD is a notmuch -configuration change and needs no code, but qtmaildir does not survive it. +**Observed (user, 2026-08-20):** during the item 124 index migration, a Delete +appeared to do nothing: the view did not refresh, the message did not move, and +restarting the application showed it exactly where it had been. The user then +reported "I see a spinner in the bottom right, is going indefinitely. maybe +that's what stopped the move?" That observation is what identified the cause. -**Cause (verified against notmuch 0.39 and a throwaway database).** notmuch -supports splitting the index from the mail with two keys: +**Cause (verified from `/proc` and the sync log).** The migration procedure held +`/tmp/mbsync.lock` to keep cron out of the way. qtmaildir auto-syncs a couple of +seconds after a tag action (item 71), so Delete queued its edit and started a +sync; `mailsync.sh` found the lock held and exited **75**, by design: -```ini -[database] -mail_root=/data/Mail -path=/home/you/.local/share/notmuch +``` +2026-08-20T10:00:01+02:00 === SKIPPED: previous run still in progress === +2026-08-20T10:10:01+02:00 === SKIPPED: previous run still in progress === ``` -Under that layout `notmuch_database_get_path()` returns the **index** -directory, not the mail root. Measured on a split test database: - -| accessor | legacy (`path` only) | split | -|---|---|---| -| `notmuch_database_get_path()` | mail root | **index dir** | -| `notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT)` | mail root | mail root | - -`notmuchworker.cpp` calls `get_path()` at six sites and treats every one as the -mail root: lines 297, 755, 900, 901, 1062, and the `relativeFilePath()` calls -at 364, 380 and 918 that consume them. - -**One of those six moves mail, and it is the dangerous one.** -`moveMessages` composes its destination as -`root + "/" + destFolder + "/cur"` (`src/notmuchworker.cpp:755`). Under a split -config `root` is the index directory, so Delete would move the message into -`<index>/Trash/cur`: outside the Maildir, invisible to mbsync, and gone from -every other client. That is item 103's stranded-mail failure with a new cause, -and this repo has already shipped that class of bug once. - -The rest degrade rather than destroy. `relativeFilePath()` against the wrong -root yields `../../../data/Mail/account/cur/...` instead of `account/cur/...`, -so no path matches an account prefix and every row resolves to no account, -which is exactly what the comment at line 294 already warns about. - -**Approach.** Replace `notmuch_database_get_path()` with -`notmuch_config_get(m_db, NOTMUCH_CONFIG_MAIL_ROOT)` at the six sites. - -**No conditional is needed, and that is the point.** `MAIL_ROOT` returns the -mail root under BOTH layouts, verified above: under a legacy `path`-only config -it equals `get_path()`, so the change is a no-op against the current -configuration and correct against the split one. A fallback to `get_path()` when -`MAIL_ROOT` is NULL is the tempting belt-and-braces addition and should be -resisted unless a NULL is actually observed, since it reintroduces the wrong -answer on the path where it matters. +75 is `EX_TEMPFAIL`, chosen deliberately so a click landing during a cron run is +not reported as a failure (`assets/mailsync.sh`, the comment at the `flock -n` +guard). The application appears to treat it as neither success nor failure: the +sync indicator never cleared, and because an edit made during a sync is HELD +until the sync ends, the delete sat in that queue waiting for a completion that +could never arrive. `notmuch count tag:deleted` on the account confirmed the +write had not reached the database. + +**Nothing was lost**, and that is worth recording separately: held edits are +written to disk (item 106), so the delete survived and applied as soon as a real +sync ran. The defect is that the user cannot tell. + +**Approach.** Handle exit 75 explicitly wherever `MailSync` reports a finish. +It is a third outcome, not a variant of the other two: the work did not happen, +nothing is wrong, and it should be retried rather than reported. Clearing the +indicator is the minimum; re-arming the auto-sync timer is probably right too, +and item 89 already made a skipped auto-sync re-arm rather than give up, so +there is a precedent to follow rather than a policy to invent. **Constraints.** -- **`moveMessages` is the site to test hardest.** A wrong root there reaches the - mail server, per the "Delete MOVES the file" note in `CLAUDE.md`. -- The test fixture must build a database whose index is NOT inside the mail - root, or it cannot tell the two accessors apart: under the ordinary fixture - layout both return the same string and a mutation stays green. -- `notmuch_config_get` returns a string owned by notmuch (`notmuch.h:2585`); - do not free it. -- Nothing about the legacy layout may change. The migration is the user's to - perform, separately, once this ships. - -**Size: S.** Six call sites and a fixture that can tell them apart. +- **Do not turn 75 into an error.** The exit code exists precisely so an + overlapping click is not reported as a failed sync, and item 89 settled that a + skip is routine. Showing the log pane here would be a regression. +- **The held-edit queue must still flush.** Whatever clears the indicator has to + leave the queue in a state where the next successful sync sends it, which is + what happened by luck here rather than by design. +- **A stuck indicator is worse than a wrong one**, because it also blocks the + quit prompt's "unsynced changes" story (items 18, 19, 28, 54). This is the + same class of indicator dishonesty those four items each fixed once. + +**Reproducing it.** Hold the lock in one terminal and act in the application: + +```bash +flock /tmp/mbsync.lock -c 'sleep 300' +``` + +Then Delete a message. Verified by hand on 2026-08-20; this is how it was found. + +**Size: S.** ## Deferred, unsized, or split out |
