Skip to content
This repository was archived by the owner on Oct 11, 2022. It is now read-only.

Conversation

@mxstbr
Copy link
Contributor

@mxstbr mxstbr commented Nov 22, 2018

This is one of the most high-trafficed tables, and this is the only query in the API that's expensive since it's batched and does a .filter.

Fixed by adding a new index userIdAnIsMember and made even faster by using .count instead of .reduce! 🎉

Status

  • WIP
  • Ready for review
  • Needs testing

Deploy after merge (delete what needn't be deployed)

  • api

Run database migrations (delete if no migration was added)
YES

Related issues (delete if you don't know of any)
I accidentally based this on #4308, so until that's merged you'll also see those changes here. If you don't want #4308 but you do want this then I'll split this out!

This is one of the most high-trafficed tables, and this is the only
query in the API that's expensive since it's batched and does a `.filter`.

Fixed by adding a new index `userIdAnIsMember` and made even faster by
using `.count` instead of `.reduce`! 🎉
Copy link
Contributor

@brianlovin brianlovin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Should definitely have an impact on speed, well done man.

@brianlovin
Copy link
Contributor

Cursory approval, but the fact that e2e failed and also #4308 e2e failed tells me that something is actually failing :P

@brianlovin
Copy link
Contributor

Hopefully that merge fixes the tests! Then can merge and will run migrations tomorrow

@brianlovin brianlovin merged commit 2d9b165 into alpha Nov 23, 2018
@brianlovin brianlovin deleted the add-users-communities-index branch November 23, 2018 01:28
@brianlovin
Copy link
Contributor

.count() counts the total records returned, not the total reputation ;) Fixing in a followup

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants