Cut sequential D1 round trips on the hottest endpoints

Merged
BO
19d ago
+271-145

What does this PR aim to accomplish?:

Makes the backend faster. Production D1 stats for the last 7 days show SQL time isn't the problem: the hottest query (the phpsessid lookup, about 38k runs) averages 0.2 ms. The cost is the network round trip for each query, from the edge to the database. Probe requests from mainland China landed in SEA/SJC, and one D1 read added roughly 50–80 ms there. Several endpoints made those round trips one after another, often in loops.

How does this PR accomplish the above?:

Round trips per request, before → after:

EndpointBeforeAfter
GetMailList2 + 3 per conversation, one after another1 query (window functions)
GetBBSMentionList1 + 2 per mention1 query
GetPosts2, and the page query read all ~11k replies (window over bbs_reply)1 batch(), last reply looked up through idx_bbs_reply_post_time
NewReply / NewPostcaptcha check, then each read in turncaptcha check and reads at the same time (allSettled); a failed read can't hide a failed captcha
@user checks and mention writes (NewReply, EditReply)one at a time, often an xmoj fetch eachin parallel, at most 4 at a time; recipients de-duplicated first so two writes for the same person can't both insert
DeletePostone DELETE per replyone DELETE for all replies
Legacy session, first checkcount, then insertone INSERT OR REPLACE (Database.Insert gains an OrReplace flag)
Legacy session, already deadxmoj asked twiceasked once; only a failed fetch or non-2xx response is retried

GetPosts still reads its count and page from one snapshot. A D1 batch runs as one transaction, which replaces the shared-session trick the old comment described.

Small behavior changes:

  • A fractional Page in GetPosts now returns a page instead of an SQL error.
  • A user @-mentioned twice in one reply gets one existence check instead of two.

migrations/0007_add_existing_indexes.sql records three indexes that were created by hand in production but never added to migrations/. It uses the same names with IF NOT EXISTS, so it does nothing in production. A fourth production index, idx_phpsessid, is left out because it duplicates the primary key's own index. It makes local and fresh databases match.

Testing:

  • npm test: 155 pass, 0 fail. 7 new tests cover the retry rule (in both directions), the single upsert, mention de-duplication, the one-batch GetPosts, captcha-first error reporting and the 4-at-a-time mention bound. The 4 that test new behavior fail on the old code; the one that checks retries still happen when xmoj doesn't answer passes on both, as intended.
  • The unit tests mock D1, so I checked the rewritten SQL separately. The old and new queries for GetPosts (three filters × three pages), GetBBSMentionList and GetMailList were run side by side on a real SQLite, built from the migrations plus the production indexes and seeded with random data. That includes ties on reply_time/send_time, mentions of deleted posts and replies, and self-conversations. All 168 comparisons matched across 8 seeds. EXPLAIN QUERY PLAN confirms the new GetPosts uses the index for all three subqueries.
  • wrangler deploy --dry-run builds.

Not included: Smart Placement and moving the D1 primary out of APAC. Both depend on where real users' requests land, which should be checked in the Workers dashboard first.

Does this PR need a documentation update?:

No. CLAUDE.md gains a note on avoiding sequential round trips.

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributor's guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against. (dev for XMOJ-Script, master for everything else).
  2. I have commented on my proposed changes within the code.
  3. I have tested my changes.
  4. I am willing to help maintain this change if there are issues with it later.
  5. It is compatible with the GNU General Public License v3.0
  6. I have squashed any insignificant commits. (git rebase)
  7. I have checked that another pull request for this purpose does not exist.
  8. I have considered and confirmed that this submission will be valuable to others.
  9. I accept that this submission may not be used, and the pull request can be closed at the will of the maintainer.
  10. I give this submission freely and claim no ownership to its content.
  11. I have verified that my changes work correctly in all four UI combinations. (Backend only, so there is no UI here. The API's request and response shapes are unchanged.)
  • I have read the above and my PR is ready for review. Check this box to confirm

🤖 Generated with Claude Code

https://claude.ai/code/session_017dqWu3YgeVRGwBt5pXDq4A

Summary by Sourcery

Reduce sequential D1 round trips on high-traffic endpoints while preserving response behavior and consistency.

Bug Fixes:

  • Avoid redundant legacy session validation retries when XMOJ has already rejected a session, while retaining retries for failed or non-successful fetches.
  • Handle fractional GetPosts page values without producing an SQL error and de-duplicate repeated user mentions.

Enhancements:

  • Reduce D1 latency across messaging, mentions, post listing, session handling, and deletion by consolidating queries and running independent checks in parallel with bounded concurrency.
  • Preserve consistent GetPosts pagination snapshots while limiting reply lookups to the requested page.
  • Use a single upsert for verified legacy sessions and bulk-delete replies when removing a post.

Documentation:

  • Document guidance for avoiding sequential and N+1 database round trips in edge Workers.

Tests:

  • Add coverage for session retry behavior, session upserts, mention de-duplication and concurrency limits, captcha error precedence, and batched post retrieval.

Chores:

  • Record existing production indexes in a migration so local and fresh databases match production.

Summary by cubic

Cuts sequential D1 round trips on the hottest endpoints, folding per-item N+1 lookups into single queries and running independent reads and fetches in parallel — network round trips cost tens of milliseconds each while the SQL itself runs in well under one. Slightly different behavior: a fractional Page now returns a page instead of an SQL error, and a user @-mentioned twice gets one existence check. All 153 tests pass; the rewritten queries were compared against the old SQL on a real SQLite across eight random seeds with matching results.

Round trips per request (before → after)

  • GetMailList: 2 + 3 per conversation → 1 query using window functions.
  • GetBBSMentionList: 1 + 2 per mention → 1 query, still dropping mentions whose post is gone.
  • GetPosts: count and page in one batch() (one transaction, one snapshot); the last reply comes from idx_bbs_reply_post_time instead of a window over every reply.
  • NewPost / NewReply: captcha check overlaps the read-only lookups and its error is still reported first when a lookup beside it fails; @-mention checks and writes run in parallel at most four at a time, with recipients de-duplicated first so two writes for the same person can't both insert.
  • DeletePost: one DELETE for all replies.
  • Legacy session: one upsert (Insert gains an OrReplace flag) instead of count then insert; a session xmoj already rejected is checked once, but a failed fetch still gets one retry.

Migration

  • migrations/0007_add_existing_indexes.sql records three production indexes created by hand, using the same names with IF NOT EXISTS so it's a no-op in production and brings local and fresh databases in line.