From 7380eac5a2a2c04e6e8948f74d1b71dee2ffb61e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bogus=C5=82awski?= Date: Sat, 14 Jun 2025 23:01:56 +0200 Subject: [PATCH] fix: improve dashboard loading performances (#7604) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generating dashboard page takes too long when table `action` contains many records and error log contains message like ``` 2025/04/21 21:21:07 ...activities/action.go:470:GetFeeds() [W] [Slow SQL Query] SELECT `action`.* FROM `action` INNER JOIN `repository` ON `repository`.id = `action`.repo_id WHERE user_id=? AND is_deleted=? ORDER BY `action`.`created_unix` DESC LIMIT 20 [12 false] - 2m8.393454675s ``` This mod removes unnecessary inner join like proposed in https://github.com/go-gitea/gitea/pull/32127 For complete solution index(user_id, is_deleted) for action table should be created also like in https://github.com/go-gitea/gitea/pull/32333 (not included in this mod). Related: https://github.com/go-gitea/gitea/pull/32127 Related: https://github.com/go-gitea/gitea/pull/32333 Author-Change-Id: IB#1160173 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7604 Reviewed-by: Earl Warren Co-authored-by: Paweł Bogusławski Co-committed-by: Paweł Bogusławski --- models/activities/action.go | 5 +---- models/activities/action_test.go | 18 ------------------ models/fixtures/action.yml | 8 -------- 3 files changed, 1 insertion(+), 30 deletions(-) diff --git a/models/activities/action.go b/models/activities/action.go index a309637d04..1e40546b97 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -467,11 +467,8 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err return nil, 0, err } - sess := db.GetEngine(ctx).Where(cond). - Select("`action`.*"). // this line will avoid select other joined table's columns - Join("INNER", "repository", "`repository`.id = `action`.repo_id") - opts.SetDefaultValues() + sess := db.GetEngine(ctx).Where(cond) sess = db.SetSessionPagination(sess, &opts) actions := make([]*Action, 0, opts.PageSize) diff --git a/models/activities/action_test.go b/models/activities/action_test.go index 8b9b2f6929..bcc9c98cec 100644 --- a/models/activities/action_test.go +++ b/models/activities/action_test.go @@ -226,24 +226,6 @@ func TestNotifyWatchers(t *testing.T) { }) } -func TestGetFeedsCorrupted(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - unittest.AssertExistsAndLoadBean(t, &activities_model.Action{ - ID: 8, - RepoID: 1700, - }) - - actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{ - RequestedUser: user, - Actor: user, - IncludePrivate: true, - }) - require.NoError(t, err) - assert.Empty(t, actions) - assert.Equal(t, int64(0), count) -} - func TestConsistencyUpdateAction(t *testing.T) { if !setting.Database.Type.IsSQLite3() { t.Skip("Test is only for SQLite database.") diff --git a/models/fixtures/action.yml b/models/fixtures/action.yml index f1592d4569..a97e94fbf4 100644 --- a/models/fixtures/action.yml +++ b/models/fixtures/action.yml @@ -59,14 +59,6 @@ created_unix: 1603011540 # grouped with id:7 - id: 8 - user_id: 1 - op_type: 12 # close issue - act_user_id: 1 - repo_id: 1700 # dangling intentional - is_private: false - created_unix: 1603011541 - -- id: 9 user_id: 34 op_type: 12 # close issue act_user_id: 34