From ca3f4d00bc6c3edb6a961c1cf68a460e2b68eaf8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 12 Aug 2024 11:12:38 -0700 Subject: [PATCH 1/8] improve get feed with pagination --- models/activities/action.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/models/activities/action.go b/models/activities/action.go index d23f2bd986f3f..a1a06dfe001f3 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -451,16 +451,28 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err } sess := db.GetEngine(ctx).Where(cond). - Select("`action`.*"). // this line will avoid select other joined table's columns + Select("`action`.id"). Join("INNER", "repository", "`repository`.id = `action`.repo_id") opts.SetDefaultValues() sess = db.SetSessionPagination(sess, &opts) - actions := make([]*Action, 0, opts.PageSize) - count, err := sess.Desc("`action`.created_unix").FindAndCount(&actions) + actionIDs := make([]int64, 0, opts.PageSize) + if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil { + return nil, 0, fmt.Errorf("Find: %w", err) + } + + count, err := db.GetEngine(ctx).Where(cond). + Table("action"). + Cols("`action`.id"). + Join("INNER", "repository", "`repository`.id = `action`.repo_id").Count() if err != nil { - return nil, 0, fmt.Errorf("FindAndCount: %w", err) + return nil, 0, fmt.Errorf("Count: %w", err) + } + + actions := make([]*Action, 0, opts.PageSize) + if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Find(&actions); err != nil { + return nil, 0, fmt.Errorf("Find: %w", err) } if err := ActionList(actions).LoadAttributes(ctx); err != nil { From 5082ae853deb495738c68d88b4e65ab9d20740a6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 14 Aug 2024 10:39:06 +0800 Subject: [PATCH 2/8] Update models/activities/action.go Co-authored-by: delvh --- models/activities/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/activities/action.go b/models/activities/action.go index a1a06dfe001f3..089b6c557dacf 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -449,7 +449,7 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err if err != nil { return nil, 0, err } - + // First, only query which IDs are necessary, and only then query all actions to speed up the overall query sess := db.GetEngine(ctx).Where(cond). Select("`action`.id"). Join("INNER", "repository", "`repository`.id = `action`.repo_id") From 9b538171cf4e831a14eb2e595177fa6ae25d5994 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 14 Aug 2024 14:42:08 -0700 Subject: [PATCH 3/8] Fix sequence bug of action --- models/activities/action.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/models/activities/action.go b/models/activities/action.go index a1a06dfe001f3..8cd7aee34004a 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -9,6 +9,7 @@ import ( "fmt" "net/url" "path" + "sort" "strconv" "strings" "time" @@ -475,6 +476,10 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err return nil, 0, fmt.Errorf("Find: %w", err) } + sort.SliceStable(actions, func(i, j int) bool { + return actions[i].ID > actions[j].ID // id's sequence should be equal to created_unix's sequence + }) + if err := ActionList(actions).LoadAttributes(ctx); err != nil { return nil, 0, fmt.Errorf("LoadAttributes: %w", err) } From 0f69b0183792948530c9cf2fbbba3e8610b54a7d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 14 Aug 2024 14:50:48 -0700 Subject: [PATCH 4/8] Use the old logic for the first 10 page --- models/activities/action.go | 63 +++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/models/activities/action.go b/models/activities/action.go index 8cd7aee34004a..2775c1460e63d 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -451,34 +451,51 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err return nil, 0, err } - sess := db.GetEngine(ctx).Where(cond). - Select("`action`.id"). - Join("INNER", "repository", "`repository`.id = `action`.repo_id") + actions := make([]*Action, 0, opts.PageSize) + var count int64 - opts.SetDefaultValues() - sess = db.SetSessionPagination(sess, &opts) + if opts.Page > 10 { // TODO: why it's 10 but other values? It's an experience value. + 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") - actionIDs := make([]int64, 0, opts.PageSize) - if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil { - return nil, 0, fmt.Errorf("Find: %w", err) - } + opts.SetDefaultValues() + sess = db.SetSessionPagination(sess, &opts) - count, err := db.GetEngine(ctx).Where(cond). - Table("action"). - Cols("`action`.id"). - Join("INNER", "repository", "`repository`.id = `action`.repo_id").Count() - if err != nil { - return nil, 0, fmt.Errorf("Count: %w", err) - } + count, err = sess.Desc("`action`.created_unix").FindAndCount(&actions) + if err != nil { + return nil, 0, fmt.Errorf("FindAndCount: %w", err) + } + } else { + // First, only query which IDs are necessary, and only then query all actions to speed up the overall query + sess := db.GetEngine(ctx).Where(cond). + Select("`action`.id"). + Join("INNER", "repository", "`repository`.id = `action`.repo_id") - actions := make([]*Action, 0, opts.PageSize) - if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Find(&actions); err != nil { - return nil, 0, fmt.Errorf("Find: %w", err) - } + opts.SetDefaultValues() + sess = db.SetSessionPagination(sess, &opts) - sort.SliceStable(actions, func(i, j int) bool { - return actions[i].ID > actions[j].ID // id's sequence should be equal to created_unix's sequence - }) + actionIDs := make([]int64, 0, opts.PageSize) + if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil { + return nil, 0, fmt.Errorf("Find: %w", err) + } + + count, err = db.GetEngine(ctx).Where(cond). + Table("action"). + Cols("`action`.id"). + Join("INNER", "repository", "`repository`.id = `action`.repo_id").Count() + if err != nil { + return nil, 0, fmt.Errorf("Count: %w", err) + } + + if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Find(&actions); err != nil { + return nil, 0, fmt.Errorf("Find: %w", err) + } + + sort.SliceStable(actions, func(i, j int) bool { + return actions[i].ID > actions[j].ID // id's sequence should be equal to created_unix's sequence + }) + } if err := ActionList(actions).LoadAttributes(ctx); err != nil { return nil, 0, fmt.Errorf("LoadAttributes: %w", err) From e1cf097fe145ad2ebd5960cded48659ea856daaa Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 15 Aug 2024 18:11:06 -0700 Subject: [PATCH 5/8] Fix bug --- models/activities/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/activities/action.go b/models/activities/action.go index 2775c1460e63d..5c555076faf32 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -454,7 +454,7 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err actions := make([]*Action, 0, opts.PageSize) var count int64 - if opts.Page > 10 { // TODO: why it's 10 but other values? It's an experience value. + if opts.Page < 10 { // TODO: why it's 10 but other values? It's an experience value. 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") From 92d87844a7dc98c83125664cebf49eae79426ef8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 25 Aug 2024 02:54:27 +0800 Subject: [PATCH 6/8] Update models/activities/action.go Co-authored-by: delvh --- models/activities/action.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/models/activities/action.go b/models/activities/action.go index 5c555076faf32..2e0ddfab3ebd1 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -488,13 +488,9 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err return nil, 0, fmt.Errorf("Count: %w", err) } - if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Find(&actions); err != nil { + if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Desc("`action`.created_unix").Find(&actions); err != nil { return nil, 0, fmt.Errorf("Find: %w", err) } - - sort.SliceStable(actions, func(i, j int) bool { - return actions[i].ID > actions[j].ID // id's sequence should be equal to created_unix's sequence - }) } if err := ActionList(actions).LoadAttributes(ctx); err != nil { From b2137c08b38edd81ff55c8d43f97562a068ede66 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 24 Aug 2024 13:06:52 -0700 Subject: [PATCH 7/8] Fix lint --- models/activities/action.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/activities/action.go b/models/activities/action.go index 2e0ddfab3ebd1..065aee08e3ba8 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -9,7 +9,6 @@ import ( "fmt" "net/url" "path" - "sort" "strconv" "strings" "time" From 49cbb2fc3ff63877e4924d94f0cf26c6285c0985 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 26 Aug 2024 11:27:34 -0700 Subject: [PATCH 8/8] more detail for error --- models/activities/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/activities/action.go b/models/activities/action.go index 065aee08e3ba8..532667d495798 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -476,7 +476,7 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err actionIDs := make([]int64, 0, opts.PageSize) if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil { - return nil, 0, fmt.Errorf("Find: %w", err) + return nil, 0, fmt.Errorf("Find(actionsIDs): %w", err) } count, err = db.GetEngine(ctx).Where(cond).