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

Faster multiplayer room participation listing #11263

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

notbakaneko
Copy link
Collaborator

Duplicates ends_at to multiplayer_rooms_high and uses it for sort order.
The existing issue is ordering on multiplayer_rooms while the existence check uses multiplayer_rooms_high can result in too many records being scanned to get enough matching rows. The issue doesn't exist if there's no sort order, or the required rows are all early in the sorting order.

I made the listing a separate scope so other queries that don't have the issue can skip the join mess.

  • add migration 2024_06_11_000001_add_ends_at_to_multiplayer_rooms_high
  • CREATE INDEX participated_rooms ON multiplayer_rooms_high (ends_at DESC, room_id DESC, user_id)
  • fill column UPDATE multiplayer_rooms_high h INNER JOIN multiplayer_rooms r ON h.room_id = r.id SET h.ends_at = r.ends_at

@notbakaneko
Copy link
Collaborator Author

notbakaneko commented Jun 11, 2024

SELECT * FROM `multiplayer_rooms` where `id` in (
	SELECT `room_id` FROM `multiplayer_rooms_high`
	WHERE `user_id` = 475001
	ORDER BY `ends_at` DESC, `room_id` DESC
) and `TYPE` in ('head_to_head', 'team_versus') and `deleted_at` IS NULL
LIMIT 51;

seems to perform better than join when limit increases but passing and getting the cursor for response does not look like a great time...
Doesn't work, needs to be reordered again

@nanaya
Copy link
Collaborator

nanaya commented Jun 12, 2024

fwiw adding the user,room index using table dumps from production:

before:

mysql [osu]> SELECT * FROM `multiplayer_rooms` WHERE `TYPE` in ('head_to_head', 'team_versus') and EXISTS (SELECT * FROM `multiplayer_rooms_high` WHERE `multiplayer_rooms`.`id` = `multiplayer_rooms_high`.`room_id` and `user_id` = 475001) and `multiplayer_rooms`.`deleted_at` IS NULL ORDER BY `ends_at` DESC, `id` DESC LIMIT 51;
Empty set (2.03 sec)

mysql [osu]> SELECT * FROM `multiplayer_rooms` WHERE `TYPE` in ('head_to_head', 'team_versus') and EXISTS (SELECT * FROM `multiplayer_rooms_high` WHERE `multiplayer_rooms`.`id` = `multiplayer_rooms_high`.`room_id` and `user_id` = 475001) and `multiplayer_rooms`.`deleted_at` IS NULL ORDER BY `ends_at` DESC, `id` DESC LIMIT 51;
Empty set (2.02 sec)

after:

mysql [osu]> create index user_room on multiplayer_rooms_high(user_id,room_id);
Query OK, 0 rows affected (2.14 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql [osu]> SELECT * FROM `multiplayer_rooms` WHERE `TYPE` in ('head_to_head', 'team_versus') and EXISTS (SELECT * FROM `multiplayer_rooms_high` WHERE `multiplayer_rooms`.`id` = `multiplayer_rooms_high`.`room_id` and `user_id` = 475001) and `multiplayer_rooms`.`deleted_at` IS NULL ORDER BY `ends_at` DESC, `id` DESC LIMIT 51;
Empty set (0.00 sec)

mysql [osu]> SELECT * FROM `multiplayer_rooms` WHERE `TYPE` in ('head_to_head', 'team_versus') and EXISTS (SELECT * FROM `multiplayer_rooms_high` WHERE `multiplayer_rooms`.`id` = `multiplayer_rooms_high`.`room_id` and `user_id` = 475001) and `multiplayer_rooms`.`deleted_at` IS NULL ORDER BY `ends_at` DESC, `id` DESC LIMIT 51;
Empty set (0.00 sec)

@nanaya
Copy link
Collaborator

nanaya commented Jun 12, 2024

I think we should try adding the index first and see if it helps. If it didn't then proceed with this pr. @peppy

@notbakaneko
Copy link
Collaborator Author

try it with user_id = 1040328 🤔

@nanaya
Copy link
Collaborator

nanaya commented Jun 12, 2024

|  37020 |  1040328 | smoogipoo's awesome room                                                              |   33908195 | 2020-12-11 11:51:58 | 2020-12-16 13:38:19 |         NULL |                 1 | NULL                      | head_to_head | host_only               |                   0 |         0 | 2020-12-11 11:51:58 | 2020-12-11 11:51:58 | NULL       | normal   |
+--------+----------+---------------------------------------------------------------------------------------+------------+---------------------+---------------------+--------------+-------------------+---------------------------+--------------+-------------------------+---------------------+-----------+---------------------+---------------------+------------+----------+
44 rows in set (0.01 sec)

@peppy
Copy link
Member

peppy commented Jun 12, 2024

I've added the index. I think it will work short term, but if a user ends up participating in 1,000+ multiplayer rooms it will start to falter:

> explain SELECT * FROM `multiplayer_rooms` WHERE `TYPE` in ('head_to_head', 'team_versus') and EXISTS (SELECT * FROM `multiplayer_rooms_high`
                        -> WHERE `multiplayer_rooms`.`id` = `multiplayer_rooms_high`.`room_id` and `user_id` = 1040328) and `multiplayer_rooms`.`deleted_at` IS NULL ORD
                        -> ER BY `ends_at` DESC, `id` DESC LIMIT 51\G;
***************************[ 1. row ]***************************
id            | 1
select_type   | SIMPLE
table         | multiplayer_rooms_high
partitions    | <null>
type          | ref
possible_keys | multiplayer_rooms_high_room_id_user_id_unique,user_room
key           | user_room
key_len       | 4
ref           | const
rows          | 213
filtered      | 100.0
Extra         | Using index; Using temporary; Using filesort
***************************[ 2. row ]***************************
id            | 1
select_type   | SIMPLE
table         | multiplayer_rooms
partitions    | <null>
type          | eq_ref
possible_keys | PRIMARY,multiplayer_rooms_type_category_ends_at_index
key           | PRIMARY
key_len       | 8
ref           | osu.multiplayer_rooms_high.room_id
rows          | 1
filtered      |   5.63
Extra         | Using where

The filesort is the issue here.

@@ -500,6 +525,15 @@ public function recentParticipants(): array
->all();
}

public function save(array $options = [])
{
if ($this->exists && $this->isDirty('ends_at')) {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it make more sense for this column to be called ended_at similar to scores table, and have it set when the score is set, rather than when the room is saved?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's duplicating the room's ends_at time and ordered to match the display of a room's end time, not when the user played
image

Copy link
Member

Choose a reason for hiding this comment

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

I get that, but I'm proposing not doing that. I think it makes more sense to list based on the user's last play rather than the time the room was closed? Will make much more sense for the profile page usage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The display will need to be adjusted a bit since the part with the room state and closing time are meant to make sense together, and it would be weird if all the times started showing other of order.

Copy link
Member

Choose a reason for hiding this comment

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

I dunno, I think it would still make sense to me 😅

@nanaya
Copy link
Collaborator

nanaya commented Jun 13, 2024

is this still needed? also is this result correct?

mysql [osu]> SELECT multiplayer_rooms.*, multiplayer_rooms_high.room_id FROM `multiplayer_rooms` inner join `multiplayer_rooms_high` on `multiplayer_rooms_high`.`room_id` = `multiplayer_rooms`.`id` WHERE `type` in ('head_to_head','team_versus') and `multiplayer_rooms_high`.`user_id` = 1040328 and `multiplayer_rooms`.`deleted_at` IS NULL ORDER BY `multiplayer_rooms_high`.`ends_at` DESC, `multiplayer_rooms_high`.`room_id` DESC LIMIT 51;
...
|  37020 |  1040328 | smoogipoo's awesome room                                                              |   33908195 | 2020-12-11 11:51:58 | 2020-12-16 13:38:19 |         NULL |                 1 | NULL                      | head_to_head | host_only               |                   0 |         0 | 2020-12-11 11:51:58 | 2020-12-11 11:51:58 | NULL       | normal   |   37020 |
+--------+----------+---------------------------------------------------------------------------------------+------------+---------------------+---------------------+--------------+-------------------+---------------------------+--------------+-------------------------+---------------------+-----------+---------------------+---------------------+------------+----------+---------+
44 rows in set (1.34 sec)

(explain)
+----+-------------+------------------------+------------+--------+-------------------------------------------------------+-----------------------------------------------+---------+--------------------------------+--------+----------+----------------------------------------------+
| id | select_type | table                  | partitions | type   | possible_keys                                         | key                                           | key_len | ref                            | rows   | filtered | Extra                                        |
+----+-------------+------------------------+------------+--------+-------------------------------------------------------+-----------------------------------------------+---------+--------------------------------+--------+----------+----------------------------------------------+
|  1 | SIMPLE      | multiplayer_rooms      | NULL       | ALL    | PRIMARY,multiplayer_rooms_type_category_ends_at_index | NULL                                          | NULL    | NULL                           | 668510 |     5.66 | Using where; Using temporary; Using filesort |
|  1 | SIMPLE      | multiplayer_rooms_high | NULL       | eq_ref | multiplayer_rooms_high_room_id_user_id_unique         | multiplayer_rooms_high_room_id_user_id_unique | 12      | osu.multiplayer_rooms.id,const |      1 |   100.00 | NULL                                         |
+----+-------------+------------------------+------------+--------+-------------------------------------------------------+-----------------------------------------------+---------+--------------------------------+--------+----------+----------------------------------------------+

@notbakaneko notbakaneko marked this pull request as draft June 17, 2024 10:14
@notbakaneko
Copy link
Collaborator Author

I'm doing a bit more testing on this at the moment, and it's really depending on how the records are spread out 😐

@peppy peppy self-assigned this Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: On hold
Development

Successfully merging this pull request may close these issues.

3 participants