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
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
Unknown Object (File)
Wed, Dec 4, 4:21 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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

keyserver/src/database/migration-config.js
475–476 ↗(On Diff #28707)

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 ↗(On Diff #28707)
  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 ↗(On Diff #28707)

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 ↗(On Diff #28707)

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 ↗(On Diff #28707)

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 ↗(On Diff #28707)

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