Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve user status revert performance #31106

Merged

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Feb 10, 2022

Currently if a meeting in Talk is ended for everyone the database basically dies. One of the problems is resetting the user status:

Queries before "5 per user"

  1. Get user backup status revertUserStatus
  2. Get user status revertUserStatus
  3. Get user status removeUserStatus
  4. Delete user status removeUserStatus
  5. Update user backup status revertUserStatus

Plan level 1: Improve current function: "3 per user"

  • Get user backup status revertUserStatus: ☑️
  • Get user status revertUserStatus: 💥 Remove by adding a delete which has the matching parameters
  • Get user status removeUserStatus: 💥 Removed in this PR
  • Delete user status removeUserStatus: ☑️
  • Update user backup status revertUserStatus: ☑️

Plan level 2: Add special handling for "End call for everyone": "3 in total"

  • Get the user + backup status of all users in single query
  • Delete statuses by id in single query
  • Update statuses in single query

Plan level 3

  • There is currently no index on is_backup so all those queries might do a using WHERE => removed from query as the leading underscore ensures it's value already

Fix #31190

@nickvergessen
Copy link
Member Author

at least the first/first-two commits should be backported to 23

@nickvergessen nickvergessen force-pushed the techdebt/noid/improve-user-status-update-handling branch 2 times, most recently from 99f51e9 to cd31725 Compare February 11, 2022 13:41
@nickvergessen nickvergessen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 11, 2022
@nickvergessen nickvergessen requested review from CarlSchwan, a team, juliusknorr, skjnldsv and PVince81 and removed request for a team February 11, 2022 13:41
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
…d of 3*n

Signed-off-by: Joas Schilling <coding@schilljs.com>
…ser_id leading underscore already

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the techdebt/noid/improve-user-status-update-handling branch from e1aa54b to 058d1de Compare February 15, 2022 15:06
@nickvergessen
Copy link
Member Author

Also reduced inserting the status from 7 to 2-3 queries as described in #31190

@nickvergessen
Copy link
Member Author

3 way comparison in talks integration test of the query log makes it obvious:
Sample: ./run.sh features/callapi/group.feature

left before middle with this PR right with user status disabled
2884 2594 2505

Bildschirmfoto von 2022-02-16 15-46-16

@vitormattos
Copy link
Contributor

Drone tests isn't necessary?

if (!$userStatus->getIsBackup()
&& $userStatus->getMessageId() === $messageId
&& $userStatus->getStatus() === $status) {
$statuesToDelete[$userStatus->getUserId()] = $userStatus->getId();
Copy link
Member

Choose a reason for hiding this comment

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

typo deleting statues 🗽

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@nickvergessen nickvergessen merged commit bf4acd5 into master Feb 23, 2022
@nickvergessen nickvergessen deleted the techdebt/noid/improve-user-status-update-handling branch February 23, 2022 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Call to setUserStatus does too many database queries
4 participants