luckyimnotanigger/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-04-26 09:36:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3
forgejo/tests/integration/notification_test.go

33 lines
975 B
Go
Raw Normal View History

feat: improve performance of notifications page for MySQL - For the notifications page the unread and pinned notifications are gathered for doer those that and are ordered by the updated unix. MariaDB makes a bad decision (sometimes, for most users it does not make this decision) with this query, it uses the index for the `updated_unix` column to speed up this query, however this is not the correct index to be taking, if the doer does not have more than 20 (the page size) unread and pinned notifications combined MariaDB will traverse the whole notifications table before it realizes that there are no more notifications to be gathered. It instead should use the index for the `user_id` column (this is what MariaDB already does for most users), so the list that has to be traversed is limited to the doer's notifications which is significantly less than the whole notifications table. - This is a different approach than what Gitea has taken to solve this problem, which is to add a index to the (status, userid, updated_unix) tuple (Ref: https://github.com/go-gitea/gitea/pull/32395). Adding more and more indexes is not a good way if we can use existing indexes to get a query to a acceptable performance. - The code cannot use `db.Find` as it's hard to add a index hint option specifically for this query and not for the other instances that uses `activities_model.FindNotificationOptions`. - Only add a index hint for MySQL as I have not been able to test if SQLite or PostgreSQL are smart enough to use the better index (as you need a large enough dataset to test this meaningfully). - Integration test added to ensure the SQL is run by all databases. --- Performance numbers (from Codeberg's database - MariaDB 10.11.6-MariaDB-0+deb12u1): Currently: ```sql SELECT * FROM `notification` WHERE notification.user_id=26734 AND (notification.status=3 OR notification.status=1) ORDER BY notification.updated_unix DESC LIMIT 20; (5.731 sec) +------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra | +------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+ | 1 | SIMPLE | notification | index | IDX_notification_status,IDX_notification_user_id | IDX_notification_updated_unix | 8 | const | 1376836 | 1474066.00 | 50.03 | 0.00 | Using where | +------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+ ``` Using the better index: ```sql SELECT * FROM `notification` USE INDEX (IDX_notification_user_id) WHERE notification.user_id=26734 AND (notification.status=3 OR notification.status=1) ORDER BY notification.updated_unix DESC LIMIT 20; (0.834 sec) +------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra | +------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+ | 1 | PRIMARY | notification | ref | PRIMARY,IDX_notification_status,IDX_notification_user_id | IDX_notification_user_id | 8 | const | 22042 | 10756.00 | 50.03 | 0.02 | Using where; Using temporary; Using filesort | | 1 | PRIMARY | notification | eq_ref | PRIMARY | PRIMARY | 8 | gitea_production.notification.id | 1 | 1.00 | 100.00 | 100.00 | | +------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+ ```
2024-12-04 14:38:41 +01:00
// Copyright 2024 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
package integration
import (
"net/http"
"testing"
chore: branding import path (#7337) - Massive replacement of changing `code.gitea.io/gitea` to `forgejo.org`. - Resolves forgejo/discussions#258 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7337 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Reviewed-by: Michael Kriese <michael.kriese@gmx.de> Reviewed-by: Beowulf <beowulf@beocode.eu> Reviewed-by: Panagiotis "Ivory" Vasilopoulos <git@n0toose.net> Co-authored-by: Gusted <postmaster@gusted.xyz> Co-committed-by: Gusted <postmaster@gusted.xyz>
2025-03-27 19:40:14 +00:00
"forgejo.org/models/unittest"
user_model "forgejo.org/models/user"
"forgejo.org/tests"
feat: improve performance of notifications page for MySQL - For the notifications page the unread and pinned notifications are gathered for doer those that and are ordered by the updated unix. MariaDB makes a bad decision (sometimes, for most users it does not make this decision) with this query, it uses the index for the `updated_unix` column to speed up this query, however this is not the correct index to be taking, if the doer does not have more than 20 (the page size) unread and pinned notifications combined MariaDB will traverse the whole notifications table before it realizes that there are no more notifications to be gathered. It instead should use the index for the `user_id` column (this is what MariaDB already does for most users), so the list that has to be traversed is limited to the doer's notifications which is significantly less than the whole notifications table. - This is a different approach than what Gitea has taken to solve this problem, which is to add a index to the (status, userid, updated_unix) tuple (Ref: https://github.com/go-gitea/gitea/pull/32395). Adding more and more indexes is not a good way if we can use existing indexes to get a query to a acceptable performance. - The code cannot use `db.Find` as it's hard to add a index hint option specifically for this query and not for the other instances that uses `activities_model.FindNotificationOptions`. - Only add a index hint for MySQL as I have not been able to test if SQLite or PostgreSQL are smart enough to use the better index (as you need a large enough dataset to test this meaningfully). - Integration test added to ensure the SQL is run by all databases. --- Performance numbers (from Codeberg's database - MariaDB 10.11.6-MariaDB-0+deb12u1): Currently: ```sql SELECT * FROM `notification` WHERE notification.user_id=26734 AND (notification.status=3 OR notification.status=1) ORDER BY notification.updated_unix DESC LIMIT 20; (5.731 sec) +------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra | +------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+ | 1 | SIMPLE | notification | index | IDX_notification_status,IDX_notification_user_id | IDX_notification_updated_unix | 8 | const | 1376836 | 1474066.00 | 50.03 | 0.00 | Using where | +------+-------------+--------------+-------+--------------------------------------------------+-------------------------------+---------+-------+---------+------------+----------+------------+-------------+ ``` Using the better index: ```sql SELECT * FROM `notification` USE INDEX (IDX_notification_user_id) WHERE notification.user_id=26734 AND (notification.status=3 OR notification.status=1) ORDER BY notification.updated_unix DESC LIMIT 20; (0.834 sec) +------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra | +------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+ | 1 | PRIMARY | notification | ref | PRIMARY,IDX_notification_status,IDX_notification_user_id | IDX_notification_user_id | 8 | const | 22042 | 10756.00 | 50.03 | 0.02 | Using where; Using temporary; Using filesort | | 1 | PRIMARY | notification | eq_ref | PRIMARY | PRIMARY | 8 | gitea_production.notification.id | 1 | 1.00 | 100.00 | 100.00 | | +------+-------------+--------------+--------+----------------------------------------------------------+--------------------------+---------+----------------------------------+-------+----------+----------+------------+----------------------------------------------+ ```
2024-12-04 14:38:41 +01:00
)
func TestNotification(t *testing.T) {
defer tests.PrepareTestEnv(t)()
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
session := loginUser(t, user2.Name)
req := NewRequest(t, "GET", "/notifications")
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
// Unread and pinned notification.
htmlDoc.AssertElement(t, ".notifications-link[href='/user2/repo1/pulls/3']", true)
htmlDoc.AssertElement(t, ".notifications-link[href='/user2/repo1/issues/4']", true)
htmlDoc.AssertElement(t, ".notifications-link[href='/user2/repo2/issues/1']", true)
// Read notification.
htmlDoc.AssertElement(t, ".notifications-link[href='/user2/repo2/pulls/2']", false)
}
Reference in a new issue Copy permalink