From 4b85f9ee6a2a185674a95a7dad5dfd7ebf7d9af2 Mon Sep 17 00:00:00 2001 From: Kyriakos Z <3829551+koox00@users.noreply.github.com> Date: Wed, 13 Oct 2021 22:20:44 +0300 Subject: [PATCH] MM-36743: adds last_root_post_at in channels table (#18366) * MM-36743: adds last_root_post_at in channels table Channel recency for CRT users should not count replies, this commit solves that issue by adding a new column to the channels table: LastRootPostAt. With that new info CRT users can have recent channels to work as expected. * Adds the 'LastRootPostAt' to the SQL scripts * Fixes tests * Adds LastRootPostAt migration to v6.1 * Fixes index Co-authored-by: Mattermod --- model/channel.go | 1 + scripts/mattermost-mysql-6.0.sql | 1 + scripts/mattermost-postgresql-6.0.sql | 1 + store/sqlstore/post_store.go | 7 +++- store/sqlstore/upgrade.go | 51 +++++++++++++++++++++++---- store/storetest/channel_store.go | 35 +++++++++--------- 6 files changed, 72 insertions(+), 24 deletions(-) diff --git a/model/channel.go b/model/channel.go index 75fc2680df..d210261880 100644 --- a/model/channel.go +++ b/model/channel.go @@ -47,6 +47,7 @@ type Channel struct { Header string `json:"header"` Purpose string `json:"purpose"` LastPostAt int64 `json:"last_post_at"` + LastRootPostAt int64 `json:"last_root_post_at"` TotalMsgCount int64 `json:"total_msg_count"` ExtraUpdateAt int64 `json:"extra_update_at"` CreatorId string `json:"creator_id"` diff --git a/scripts/mattermost-mysql-6.0.sql b/scripts/mattermost-mysql-6.0.sql index 1c8142a9b2..fb0002ba19 100644 --- a/scripts/mattermost-mysql-6.0.sql +++ b/scripts/mattermost-mysql-6.0.sql @@ -106,6 +106,7 @@ CREATE TABLE `Channels` ( `Header` text, `Purpose` varchar(250) DEFAULT NULL, `LastPostAt` bigint(20) DEFAULT NULL, + `LastRootPostAt` bigint(20) DEFAULT NULL, `TotalMsgCount` bigint(20) DEFAULT NULL, `ExtraUpdateAt` bigint(20) DEFAULT NULL, `CreatorId` varchar(26) DEFAULT NULL, diff --git a/scripts/mattermost-postgresql-6.0.sql b/scripts/mattermost-postgresql-6.0.sql index b74a6c9b82..643fdde581 100644 --- a/scripts/mattermost-postgresql-6.0.sql +++ b/scripts/mattermost-postgresql-6.0.sql @@ -105,6 +105,7 @@ CREATE TABLE public.channels ( header character varying(1024), purpose character varying(250), lastpostat bigint, + lastrootpostat bigint, totalmsgcount bigint, extraupdateat bigint, creatorid character varying(26), diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 708d4c83ef..ad0a37e6ae 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -170,6 +170,7 @@ func (s *SqlPostStore) SaveMultiple(posts []*model.Post) ([]*model.Post, int, er channelNewPosts := make(map[string]int) channelNewRootPosts := make(map[string]int) maxDateNewPosts := make(map[string]int64) + maxDateNewRootPosts := make(map[string]int64) rootIds := make(map[string]int) maxDateRootIds := make(map[string]int64) for idx, post := range posts { @@ -205,10 +206,14 @@ func (s *SqlPostStore) SaveMultiple(posts []*model.Post) ([]*model.Post, int, er } else { channelNewRootPosts[post.ChannelId] = 1 } + maxDateNewRootPosts[post.ChannelId] = post.CreateAt } else { if !post.IsJoinLeaveMessage() { channelNewRootPosts[post.ChannelId] = currentChannelCount + 1 } + if post.CreateAt > maxDateNewRootPosts[post.ChannelId] { + maxDateNewRootPosts[post.ChannelId] = post.CreateAt + } } continue } @@ -256,7 +261,7 @@ func (s *SqlPostStore) SaveMultiple(posts []*model.Post) ([]*model.Post, int, er for channelId, count := range channelNewPosts { countRoot := channelNewRootPosts[channelId] - if _, err = s.GetMaster().Exec("UPDATE Channels SET LastPostAt = GREATEST(:LastPostAt, LastPostAt), TotalMsgCount = TotalMsgCount + :Count, TotalMsgCountRoot = TotalMsgCountRoot + :CountRoot WHERE Id = :ChannelId", map[string]interface{}{"LastPostAt": maxDateNewPosts[channelId], "ChannelId": channelId, "Count": count, "CountRoot": countRoot}); err != nil { + if _, err = s.GetMaster().Exec("UPDATE Channels SET LastPostAt = GREATEST(:LastPostAt, LastPostAt), LastRootPostAt = GREATEST(:LastRootPostAt, LastRootPostAt), TotalMsgCount = TotalMsgCount + :Count, TotalMsgCountRoot = TotalMsgCountRoot + :CountRoot WHERE Id = :ChannelId", map[string]interface{}{"LastPostAt": maxDateNewPosts[channelId], "LastRootPostAt": maxDateNewRootPosts[channelId], "ChannelId": channelId, "Count": count, "CountRoot": countRoot}); err != nil { mlog.Warn("Error updating Channel LastPostAt.", mlog.Err(err)) } } diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index ce7dcefa5f..fecaf20da0 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -1126,8 +1126,8 @@ func rootCountMigration(sqlStore *SqlStore) { mlog.Error("Error updating ChannelId in Threads table", mlog.Err(err)) } sqlStore.CreateColumnIfNotExists("Channels", "TotalMsgCountRoot", "bigint", "bigint", "0") - sqlStore.CreateColumnIfNotExistsNoDefault("Channels", "LastRootPostAt", "bigint", "bigint") - defer sqlStore.RemoveColumnIfExists("Channels", "LastRootPostAt") + sqlStore.CreateColumnIfNotExistsNoDefault("Channels", "LastRootAt", "bigint", "bigint") + defer sqlStore.RemoveColumnIfExists("Channels", "LastRootAt") sqlStore.CreateColumnIfNotExists("ChannelMembers", "MsgCountRoot", "bigint", "bigint", "0") sqlStore.AlterDefaultIfColumnExists("ChannelMembers", "MsgCountRoot", model.NewString("0"), model.NewString("0")) @@ -1143,28 +1143,28 @@ func rootCountMigration(sqlStore *SqlStore) { WHERE Posts.RootId = '' GROUP BY Channels.Id ` - channelsCTE := "SELECT TotalMsgCountRoot, Id, LastRootPostAt from Channels" + channelsCTE := "SELECT TotalMsgCountRoot, Id, LastRootAt from Channels" updateChannels := ` WITH q AS (` + totalMsgCountRootCTE + `) - UPDATE Channels SET TotalMsgCountRoot = q.newcount, LastRootPostAt=q.lastpost + UPDATE Channels SET TotalMsgCountRoot = q.newcount, LastRootAt=q.lastpost FROM q where q.channelid=Channels.Id; ` updateChannelMembers := ` WITH q as (` + channelsCTE + `) UPDATE ChannelMembers CM SET MsgCountRoot=TotalMsgCountRoot - FROM q WHERE q.id=CM.ChannelId AND LastViewedAt >= q.lastrootpostat; + FROM q WHERE q.id=CM.ChannelId AND LastViewedAt >= q.lastrootat; ` if sqlStore.DriverName() == model.DatabaseDriverMysql { updateChannels = ` UPDATE Channels INNER Join (` + totalMsgCountRootCTE + `) as q ON q.channelid=Channels.Id - SET TotalMsgCountRoot = q.newcount, LastRootPostAt=q.lastpost; + SET TotalMsgCountRoot = q.newcount, LastRootAt=q.lastpost; ` updateChannelMembers = ` UPDATE ChannelMembers CM INNER JOIN (` + channelsCTE + `) as q - ON q.id=CM.ChannelId and LastViewedAt >= q.lastrootpostat + ON q.id=CM.ChannelId and LastViewedAt >= q.lastrootat SET MsgCountRoot=TotalMsgCountRoot ` } @@ -1357,6 +1357,43 @@ func upgradeDatabaseToVersion610(sqlStore *SqlStore) { sqlStore.AlterColumnTypeIfExists("TeamMembers", "Roles", "text", "varchar(256)") sqlStore.CreateCompositeIndexIfNotExists("idx_jobs_status_type", "Jobs", []string{"Status", "Type"}) + forceIndex := "" + if sqlStore.DriverName() == model.DatabaseDriverMysql { + forceIndex = "FORCE INDEX(idx_posts_channel_id_update_at)" + } + + lastRootPostAtExists := sqlStore.DoesColumnExist("Channels", "LastRootPostAt") + sqlStore.CreateColumnIfNotExists("Channels", "LastRootPostAt", "bigint", "bigint", "0") + + lastRootPostAtCTE := ` + SELECT Channels.Id channelid, COALESCE(MAX(Posts.CreateAt), 0) as lastrootpost + FROM Channels + LEFT JOIN Posts ` + forceIndex + ` ON Channels.Id = Posts.ChannelId + WHERE Posts.RootId = '' + GROUP BY Channels.Id + ` + + updateChannels := ` + WITH q AS (` + lastRootPostAtCTE + `) + UPDATE Channels SET LastRootPostAt=q.lastrootpost + FROM q where q.channelid=Channels.Id; + ` + + if sqlStore.DriverName() == model.DatabaseDriverMysql { + updateChannels = ` + UPDATE Channels + INNER Join (` + lastRootPostAtCTE + `) as q + ON q.channelid=Channels.Id + SET LastRootPostAt=lastrootpost; + ` + } + + if !lastRootPostAtExists { + if _, err := sqlStore.GetMaster().ExecNoTimeout(updateChannels); err != nil { + mlog.Error("Error updating Channels table", mlog.Err(err)) + } + } + // saveSchemaVersion(sqlStore, Version610) // } } diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index d82398cd56..51a2b325ad 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -4218,6 +4218,7 @@ func testChannelStoreUpdateLastViewedAt(t *testing.T, ss store.Store) { o1.Type = model.ChannelTypeOpen o1.TotalMsgCount = 25 o1.LastPostAt = 12345 + o1.LastRootPostAt = 12345 _, nErr := ss.Channel().Save(&o1, -1) require.NoError(t, nErr) @@ -4235,6 +4236,7 @@ func testChannelStoreUpdateLastViewedAt(t *testing.T, ss store.Store) { o2.Type = model.ChannelTypeOpen o2.TotalMsgCount = 26 o2.LastPostAt = 123456 + o2.LastRootPostAt = 123456 _, nErr = ss.Channel().Save(&o2, -1) require.NoError(t, nErr) @@ -6445,24 +6447,25 @@ func testMaterializedPublicChannels(t *testing.T, ss store.Store, s SqlStore) { _, execerr = s.GetMaster().ExecNoTimeout(` INSERT INTO - Channels(Id, CreateAt, UpdateAt, DeleteAt, TeamId, Type, DisplayName, Name, Header, Purpose, LastPostAt, TotalMsgCount, ExtraUpdateAt, CreatorId, TotalMsgCountRoot) + Channels(Id, CreateAt, UpdateAt, DeleteAt, TeamId, Type, DisplayName, Name, Header, Purpose, LastPostAt, LastRootPostAt, TotalMsgCount, ExtraUpdateAt, CreatorId, TotalMsgCountRoot) VALUES - (:Id, :CreateAt, :UpdateAt, :DeleteAt, :TeamId, :Type, :DisplayName, :Name, :Header, :Purpose, :LastPostAt, :TotalMsgCount, :ExtraUpdateAt, :CreatorId, 0); + (:Id, :CreateAt, :UpdateAt, :DeleteAt, :TeamId, :Type, :DisplayName, :Name, :Header, :Purpose, :LastPostAt, :LastRootPostAt, :TotalMsgCount, :ExtraUpdateAt, :CreatorId, 0); `, map[string]interface{}{ - "Id": o3.Id, - "CreateAt": o3.CreateAt, - "UpdateAt": o3.UpdateAt, - "DeleteAt": o3.DeleteAt, - "TeamId": o3.TeamId, - "Type": o3.Type, - "DisplayName": o3.DisplayName, - "Name": o3.Name, - "Header": o3.Header, - "Purpose": o3.Purpose, - "LastPostAt": o3.LastPostAt, - "TotalMsgCount": o3.TotalMsgCount, - "ExtraUpdateAt": o3.ExtraUpdateAt, - "CreatorId": o3.CreatorId, + "Id": o3.Id, + "CreateAt": o3.CreateAt, + "UpdateAt": o3.UpdateAt, + "DeleteAt": o3.DeleteAt, + "TeamId": o3.TeamId, + "Type": o3.Type, + "DisplayName": o3.DisplayName, + "Name": o3.Name, + "Header": o3.Header, + "Purpose": o3.Purpose, + "LastPostAt": o3.LastPostAt, + "LastRootPostAt": o3.LastRootPostAt, + "TotalMsgCount": o3.TotalMsgCount, + "ExtraUpdateAt": o3.ExtraUpdateAt, + "CreatorId": o3.CreatorId, }) require.NoError(t, execerr)