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

P20-831: Optimize user search by username #58463

Merged
merged 2 commits into from
May 8, 2024

Conversation

artem-vavilov
Copy link
Contributor

@artem-vavilov artem-vavilov commented May 7, 2024

Summary

Testing the users table locally with over 3.25 million records

Before (9483.1ms)

EXPLAIN for: SELECT `users`.* FROM `users` WHERE `users`.`username` LIKE '%art%'
+----+-------------+-------+------------+------+---------------+------+---------+------+---------+----------+-------------+
| id | select_type | table | partitions | type | possible_keys | key  | key_len | ref  | rows    | filtered | Extra       |
+----+-------------+-------+------------+------+---------------+------+---------+------+---------+----------+-------------+
|  1 | SIMPLE      | users | NULL       | ALL  | NULL          | NULL | NULL    | NULL | 3135067 |    11.11 | Using where |
+----+-------------+-------+------------+------+---------------+------+---------+------+---------+----------+-------------+

After (0.6ms)

EXPLAIN for: SELECT `users`.* FROM `users` WHERE `users`.`username` = 'artem'
+----+-------------+-------+------------+------+----------------------------------------+----------------------------------------+---------+-------+------+----------+-------+
| id | select_type | table | partitions | type | possible_keys                          | key                                    | key_len | ref   | rows | filtered | Extra |
+----+-------------+-------+------------+------+----------------------------------------+----------------------------------------+---------+-------+------+----------+-------+
|  1 | SIMPLE      | users | NULL       | ref  | index_users_on_username_and_deleted_at | index_users_on_username_and_deleted_at | 768     | const |    1 |    100.0 | NULL  |
+----+-------------+-------+------------+------+----------------------------------------+----------------------------------------+---------+-------+------+----------+-------+

Links

@artem-vavilov artem-vavilov requested a review from a team May 7, 2024 23:05
@@ -16,7 +16,7 @@ def find_students

# If requested, filter...
if params[:usernameFilter].present?
users = users.where(User.arel_table[:username].matches("%#{params[:usernameFilter]}%"))
users = users.where(User.arel_table[:username].matches("#{params[:usernameFilter]}%"))
Copy link
Contributor

Choose a reason for hiding this comment

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

What would the performance look like if we got rid of the wildcards entirely and did User.arel_table[:username].eq(params[:usernameFilter])? In your test, the improvement was pretty huge by removing the leading %. If we get another large improvement by going to .eq it might be worth switching over, considering the large size of the users table and the fact that the CX team says they normally have the exact username they want to search.

Copy link
Contributor

@carl-codeorg carl-codeorg left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for taking this on!

Copy link
Contributor

@carl-codeorg carl-codeorg left a comment

Choose a reason for hiding this comment

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

Btw after this is deployed, it's probably worth posting in #customer-support to inform them of the change.

@artem-vavilov artem-vavilov merged commit b6741a1 into staging May 8, 2024
2 checks passed
@artem-vavilov artem-vavilov deleted the P20-831/admin-users-search-speedup branch May 8, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants