Page MenuHomePhabricator

[keyserver] Introduce index to optimize new fetchThreadInfos query
ClosedPublic

Authored by ashoat on Jul 16 2023, 4:29 PM.
Tags
None
Referenced Files
F3515314: D8512.diff
Sun, Dec 22, 8:26 AM
Unknown Object (File)
Sun, Dec 15, 4:00 AM
Unknown Object (File)
Sun, Dec 15, 4:00 AM
Unknown Object (File)
Sun, Dec 15, 4:00 AM
Unknown Object (File)
Sun, Dec 15, 4:00 AM
Unknown Object (File)
Sun, Dec 15, 3:59 AM
Unknown Object (File)
Sun, Dec 15, 3:45 AM
Unknown Object (File)
Thu, Dec 5, 11:56 AM
Subscribers
None

Details

Summary

In the following diff, I'll update fetchThreadInfos to use a query like this:

SELECT t.id, t.name, t.parent_thread_id, t.containing_thread_id,
  t.community, t.depth, t.color, t.description, t.type, t.creation_time,
  t.source_message, t.replies_count, t.avatar, t.pinned_count, m.user,
  m.role, m.permissions, m.subscription,
  m.last_read_message < m.last_message AS unread, m.sender,
  up.id AS upload_id, up.secret AS upload_secret
FROM memberships mm
LEFT JOIN threads t ON t.id = mm.thread
LEFT JOIN memberships m ON m.thread = t.id AND m.role >= 0
LEFT JOIN uploads up ON up.container = t.id
WHERE mm.user = 256 AND mm.role > -1
ORDER BY m.user ASC

The WHERE clause there filters memberships for a specific user and a range of roles. To optimize this query, I'm introducing an index on user > role > thread.

The inclusion of thread at the end may be useful for codepaths where we further filter by thread (eg. update-creator.js).

Test Plan

I ran the migration locally and confirmed that it worked

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

keyserver/src/database/migration-config.js
475–476

Note that INDEX and KEY are synonyms in MariaDB. I stuck with the phrasing matching what we had in setup-db.js for ecah, but I don't think it matters

keyserver/src/database/setup-db.js
311
  1. Merged the two index definition statements for memberships
  2. Switched to using KEY for consistency with the rest of the statement
  3. Added spaces between column names. Should do this for the whole file but didn't have time
tomek added inline comments.
keyserver/src/database/migration-config.js
471–472

In other migrations we're using slightly different syntax nad it is the reason why migrations type signature had to be changed. Using this simplified approach is better, in my opinion.

474–476

Should we update the indentation?

This revision is now accepted and ready to land.Jul 17 2023, 3:57 AM
keyserver/src/database/migration-config.js
471–472

The migrations type signature change has already been landed in D8400, and an initial use of this syntax was introduced there. Personally I find this shorthand to be cleaner / more simple

474–476

Yeah sorry, this was Prettier's fault but I think I can fix it