From b59f1d384908f58f48968183a507419a37b20f0e Mon Sep 17 00:00:00 2001 From: frank Date: Fri, 18 Oct 2024 10:25:34 +0800 Subject: [PATCH] fix_: chats and message history loading after login takes too much time (#5932) * fix_: chats and message history loading after login takes too much time * chore_: split to small functions to writing unit test easily * test_: add test * chore_: improve OldestMessageWhisperTimestampByChatIDs function - Use 'any' type instead of 'interface{}' for args slice - Add error check after rows iteration * chore_: optimize OldestMessageWhisperTimestampByChatIDs query This commit simplifies and optimizes the SQL query in the OldestMessageWhisperTimestampByChatIDs function. The changes include: 1. Removing the subquery and ROW_NUMBER() function 2. Using MIN() and GROUP BY instead of the previous approach 3. Directly selecting the required columns in a single query These changes should improve the performance of the function, especially for large datasets, while maintaining the same functionality. --- protocol/communities/manager.go | 10 +++ protocol/message_persistence.go | 57 +++++++++++----- protocol/messenger.go | 102 +++++++++++++++++++++++----- protocol/messenger_test.go | 113 ++++++++++++++++++++++++++++++++ protocol/persistence_test.go | 12 ++-- 5 files changed, 254 insertions(+), 40 deletions(-) diff --git a/protocol/communities/manager.go b/protocol/communities/manager.go index 23f3abb4b..56b179f0a 100644 --- a/protocol/communities/manager.go +++ b/protocol/communities/manager.go @@ -2001,6 +2001,12 @@ func (m *Manager) EditChatFirstMessageTimestamp(communityID types.HexBytes, chat return community, changes, nil } +func (m *Manager) UpdateChatFirstMessageTimestamp(community *Community, chatID string, timestamp uint32) (*CommunityChanges, error) { + communityID := community.ID().String() + chatID = strings.TrimPrefix(chatID, communityID) + return community.UpdateChatFirstMessageTimestamp(chatID, timestamp) +} + func (m *Manager) ReorderCategories(request *requests.ReorderCommunityCategories) (*Community, *CommunityChanges, error) { m.communityLock.Lock(request.CommunityID) defer m.communityLock.Unlock(request.CommunityID) @@ -4430,6 +4436,10 @@ func (m *Manager) accountsHasPrivilegedPermission(preParsedCommunityPermissionDa return false } +func (m *Manager) SaveAndPublish(community *Community) error { + return m.saveAndPublish(community) +} + func (m *Manager) saveAndPublish(community *Community) error { err := m.persistence.SaveCommunity(community) if err != nil { diff --git a/protocol/message_persistence.go b/protocol/message_persistence.go index 5186f83ca..a7eed93cd 100644 --- a/protocol/message_persistence.go +++ b/protocol/message_persistence.go @@ -1305,26 +1305,49 @@ func (db sqlitePersistence) MessageByChatIDs(chatIDs []string, currCursor string return result, newCursor, nil } -func (db sqlitePersistence) OldestMessageWhisperTimestampByChatID(chatID string) (timestamp uint64, hasAnyMessage bool, err error) { - var whisperTimestamp uint64 - err = db.db.QueryRow( - ` - SELECT - whisper_timestamp - FROM - user_messages m1 - WHERE - m1.local_chat_id = ? - ORDER BY substr('0000000000000000000000000000000000000000000000000000000000000000' || m1.clock_value, -64, 64) || m1.id ASC - LIMIT 1 - `, chatID).Scan(&whisperTimestamp) - if err == sql.ErrNoRows { - return 0, false, nil +func (db sqlitePersistence) OldestMessageWhisperTimestampByChatIDs(chatIDs []string) (map[string]uint64, error) { + if len(chatIDs) == 0 { + return nil, nil } + + args := make([]any, len(chatIDs)) + for i, id := range chatIDs { + args[i] = id + } + + inVector := strings.Repeat("?, ", len(chatIDs)-1) + "?" + //nolint:gosec + query := fmt.Sprintf(` + SELECT + m1.local_chat_id, + m1.whisper_timestamp, + MIN(substr('0000000000000000000000000000000000000000000000000000000000000000' || m1.clock_value, -64, 64) || m1.id) + FROM user_messages m1 + WHERE m1.local_chat_id IN (%s) + GROUP BY m1.local_chat_id +`, inVector) + + rows, err := db.db.Query(query, args...) if err != nil { - return 0, false, err + return nil, err } - return whisperTimestamp, true, nil + defer rows.Close() + + result := make(map[string]uint64) + for rows.Next() { + var chatID string + var whisperTimestamp uint64 + var cursor string + if err := rows.Scan(&chatID, &whisperTimestamp, &cursor); err != nil { + return nil, err + } + result[chatID] = whisperTimestamp + } + if err := rows.Err(); err != nil { + return nil, err + } + + return result, nil } // EmojiReactionsByChatID returns the emoji reactions for the queried messages, up to a maximum of 100, as it's a potentially unbound number. diff --git a/protocol/messenger.go b/protocol/messenger.go index 7a5aad832..d48dca998 100644 --- a/protocol/messenger.go +++ b/protocol/messenger.go @@ -1859,17 +1859,18 @@ func (m *Messenger) InitFilters() error { } communityInfo := make(map[string]*communities.Community) + var validChats []*Chat for _, chat := range chats { if err := chat.Validate(); err != nil { logger.Warn("failed to validate chat", zap.Error(err)) continue } + validChats = append(validChats, chat) + } - if err = m.initChatFirstMessageTimestamp(chat); err != nil { - logger.Warn("failed to init first message timestamp", zap.Error(err)) - continue - } + m.initChatsFirstMessageTimestamp(communityInfo, validChats) + for _, chat := range validChats { if !chat.Active || chat.Timeline() { m.allChats.Store(chat.ID, chat) continue @@ -2043,24 +2044,84 @@ func (m *Messenger) Mailservers() ([]string, error) { return nil, ErrNotImplemented } -func (m *Messenger) initChatFirstMessageTimestamp(chat *Chat) error { - if !chat.CommunityChat() || chat.FirstMessageTimestamp != FirstMessageTimestampUndefined { +func (m *Messenger) initChatsFirstMessageTimestamp(communityCache map[string]*communities.Community, chats []*Chat) { + communityChats, communityChatIDs := m.filterCommunityChats(chats) + if len(communityChatIDs) == 0 { + return + } + + oldestMessageTimestamps, err := m.persistence.OldestMessageWhisperTimestampByChatIDs(communityChatIDs) + if err != nil { + m.logger.Warn("failed to get oldest message timestamps", zap.Error(err)) + return + } + + changedCommunities := m.processCommunityChats(communityChats, communityCache, oldestMessageTimestamps) + m.saveAndPublishCommunities(changedCommunities) +} + +func (m *Messenger) filterCommunityChats(chats []*Chat) ([]*Chat, []string) { + var communityChats []*Chat + var communityChatIDs []string + for _, chat := range chats { + if chat.CommunityChat() && chat.FirstMessageTimestamp == FirstMessageTimestampUndefined { + communityChats = append(communityChats, chat) + communityChatIDs = append(communityChatIDs, chat.ID) + } + } + return communityChats, communityChatIDs +} + +func (m *Messenger) processCommunityChats(communityChats []*Chat, communityCache map[string]*communities.Community, oldestMessageTimestamps map[string]uint64) []*communities.Community { + var changedCommunities []*communities.Community + for _, chat := range communityChats { + community := m.getCommunity(chat.CommunityID, communityCache) + if community == nil { + continue + } + + oldestMessageTimestamp, ok := oldestMessageTimestamps[chat.ID] + timestamp := uint32(FirstMessageTimestampNoMessage) + if ok { + if oldestMessageTimestamp == FirstMessageTimestampUndefined { + continue + } + timestamp = whisperToUnixTimestamp(oldestMessageTimestamp) + } + + changes, err := m.updateChatFirstMessageTimestampForCommunity(chat, timestamp, community) + if err != nil { + m.logger.Warn("failed to init first message timestamp", zap.Error(err), zap.String("chatID", chat.ID)) + continue + } + if changes != nil { + changedCommunities = append(changedCommunities, community) + } + } + return changedCommunities +} + +func (m *Messenger) getCommunity(communityID string, communityCache map[string]*communities.Community) *communities.Community { + community, ok := communityCache[communityID] + if ok { + return community + } + community, err := m.communitiesManager.GetByIDString(communityID) + if err != nil { + m.logger.Warn("failed to get community", zap.Error(err), zap.String("communityID", communityID)) return nil } + communityCache[communityID] = community + return community +} - oldestMessageTimestamp, hasAnyMessage, err := m.persistence.OldestMessageWhisperTimestampByChatID(chat.ID) - if err != nil { - return err - } - - if hasAnyMessage { - if oldestMessageTimestamp == FirstMessageTimestampUndefined { - return nil +func (m *Messenger) saveAndPublishCommunities(communities []*communities.Community) { + for _, community := range communities { + err := m.communitiesManager.SaveAndPublish(community) + if err != nil { + m.logger.Warn("failed to save and publish community", zap.Error(err), zap.String("communityID", community.IDString())) } - return m.updateChatFirstMessageTimestamp(chat, whisperToUnixTimestamp(oldestMessageTimestamp), &MessengerResponse{}) } - - return m.updateChatFirstMessageTimestamp(chat, FirstMessageTimestampNoMessage, &MessengerResponse{}) } func (m *Messenger) addMessagesAndChat(chat *Chat, messages []*common.Message, response *MessengerResponse) (*MessengerResponse, error) { @@ -2557,6 +2618,13 @@ func (m *Messenger) updateChatFirstMessageTimestamp(chat *Chat, timestamp uint32 return nil } +func (m *Messenger) updateChatFirstMessageTimestampForCommunity(chat *Chat, timestamp uint32, community *communities.Community) (*communities.CommunityChanges, error) { + if community.IsControlNode() && chat.UpdateFirstMessageTimestamp(timestamp) { + return m.communitiesManager.UpdateChatFirstMessageTimestamp(community, chat.ID, chat.FirstMessageTimestamp) + } + return nil, nil +} + func (m *Messenger) ShareImageMessage(request *requests.ShareImageMessage) (*MessengerResponse, error) { if err := request.Validate(); err != nil { return nil, err diff --git a/protocol/messenger_test.go b/protocol/messenger_test.go index 2258ca715..32e93b449 100644 --- a/protocol/messenger_test.go +++ b/protocol/messenger_test.go @@ -25,6 +25,7 @@ import ( "github.com/status-im/status-go/images" "github.com/status-im/status-go/multiaccounts/settings" "github.com/status-im/status-go/protocol/common" + "github.com/status-im/status-go/protocol/communities" "github.com/status-im/status-go/protocol/protobuf" "github.com/status-im/status-go/protocol/requests" "github.com/status-im/status-go/protocol/tt" @@ -2515,3 +2516,115 @@ func (s *MessengerSuite) TestSendMessageMention() { s.Require().NoError(err) s.Require().Equal("Alice talk to bobby", response.Notifications()[0].Message) } + +func (s *MessengerSuite) TestInitChatsFirstMessageTimestamp() { + createRequest := &requests.CreateCommunity{ + Name: "status", + Description: "status community description", + Membership: protobuf.CommunityPermissions_AUTO_ACCEPT, + } + communityManager := s.m.communitiesManager + c, err := communityManager.CreateCommunity(createRequest, true) + s.Require().NoError(err) + s.Require().NotNil(c) + + chat := &protobuf.CommunityChat{ + Identity: &protobuf.ChatIdentity{ + DisplayName: "chat1", + Description: "description", + }, + Permissions: &protobuf.CommunityPermissions{ + Access: protobuf.CommunityPermissions_AUTO_ACCEPT, + }, + Members: make(map[string]*protobuf.CommunityMember), + } + _, err = s.m.CreateCommunityChat(c.ID(), chat) + s.Require().NoError(err) + + community, err := communityManager.GetByID(c.ID()) + s.Require().NoError(err) + s.Require().Len(community.Chats(), 1) + + communityDefaultChat, ok := s.m.allChats.Load(community.ChatIDs()[0]) + s.Require().True(ok) + s.Require().Equal(FirstMessageTimestampNoMessage, int(communityDefaultChat.FirstMessageTimestamp)) + + // prepared community and chat, now start test each case + // case 1: FirstMessageTimestamp is FirstMessageTimestampNoMessage, so no changes in communityCache + communityCache := make(map[string]*communities.Community) + s.m.initChatsFirstMessageTimestamp(communityCache, []*Chat{communityDefaultChat}) + // chat FirstMessageTimestamp is FirstMessageTimestampNoMessage, so no changes in communityCache + s.Require().Len(communityCache, 0) + + // case 2: FirstMessageTimestamp is FirstMessageTimestampUndefined, + // for oldestMessageTimestamp, ok := oldestMessageTimestamps[chat.ID] within initChatsFirstMessageTimestamp + // now `ok` will be false but 1 change expected in communityCache + forceFirstMessageTimestampUndefined := func() { + // force FirstMessageTimestamp to FirstMessageTimestampUndefined so we can still get chats after filterCommunityChats + communityDefaultChat.FirstMessageTimestamp = FirstMessageTimestampUndefined + err = s.m.SaveChat(communityDefaultChat) + s.Require().NoError(err) + } + forceFirstMessageTimestampUndefined() + communityCache = make(map[string]*communities.Community) + s.m.initChatsFirstMessageTimestamp(communityCache, []*Chat{communityDefaultChat}) + s.Require().Len(communityCache, 1) + + // case 3: FirstMessageTimestamp is FirstMessageTimestampUndefined and send a message, + // for oldestMessageTimestamp, ok := oldestMessageTimestamps[chat.ID] within initChatsFirstMessageTimestamp + // now `ok` will be true, `oldestMessageTimestamp`(e.g. 1728886305475) will be greater than 1 + msg := &common.Message{CommunityID: community.IDString(), ChatMessage: &protobuf.ChatMessage{ + Text: "text", + ChatId: communityDefaultChat.ID, + MessageType: protobuf.MessageType_COMMUNITY_CHAT, + ContentType: protobuf.ChatMessage_TEXT_PLAIN, + }} + _, err = s.m.sendChatMessage(context.Background(), msg) + s.Require().NoError(err) + forceFirstMessageTimestampUndefined() + communityCache = make(map[string]*communities.Community) + s.m.initChatsFirstMessageTimestamp(communityCache, []*Chat{communityDefaultChat}) + s.Require().Len(communityCache, 1) + s.Require().Greater(communityDefaultChat.FirstMessageTimestamp, uint32(1)) +} + +func (s *MessengerSuite) TestFilterCommunityChats() { + communityChat1 := &Chat{ + ID: "community-chat-1", + ChatType: ChatTypeCommunityChat, + FirstMessageTimestamp: FirstMessageTimestampUndefined, + } + communityChat2 := &Chat{ + ID: "community-chat-2", + ChatType: ChatTypeCommunityChat, + FirstMessageTimestamp: FirstMessageTimestampUndefined, + } + nonCommunityChat := &Chat{ + ID: "non-community-chat", + ChatType: ChatTypeOneToOne, + } + communityWithTimestamp := &Chat{ + ID: "community-with-timestamp", + ChatType: ChatTypeCommunityChat, + FirstMessageTimestamp: 12345, + } + + chats := []*Chat{communityChat1, nonCommunityChat, communityChat2, communityWithTimestamp} + + filteredChats, filteredIDs := s.m.filterCommunityChats(chats) + + s.Require().Len(filteredChats, 2, "Should have filtered 2 community chats") + s.Require().Len(filteredIDs, 2, "Should have 2 community chat IDs") + + s.Require().Contains(filteredChats, communityChat1, "Should contain communityChat1") + s.Require().Contains(filteredChats, communityChat2, "Should contain communityChat2") + + s.Require().Contains(filteredIDs, communityChat1.ID, "Should contain ID of communityChat1") + s.Require().Contains(filteredIDs, communityChat2.ID, "Should contain ID of communityChat2") + + s.Require().NotContains(filteredChats, nonCommunityChat, "Should not contain nonCommunityChat") + s.Require().NotContains(filteredChats, communityWithTimestamp, "Should not contain communityWithTimestamp") + + s.Require().NotContains(filteredIDs, nonCommunityChat.ID, "Should not contain ID of nonCommunityChat") + s.Require().NotContains(filteredIDs, communityWithTimestamp.ID, "Should not contain ID of communityWithTimestamp") +} diff --git a/protocol/persistence_test.go b/protocol/persistence_test.go index ee7784f09..6ec300b4d 100644 --- a/protocol/persistence_test.go +++ b/protocol/persistence_test.go @@ -322,15 +322,15 @@ func TestLatestMessageByChatID(t *testing.T) { require.Equal(t, m[0].ID, ids[9]) } -func TestOldestMessageWhisperTimestampByChatID(t *testing.T) { +func TestOldestMessageWhisperTimestampByChatIDs(t *testing.T) { db, err := openTestDB() require.NoError(t, err) p := newSQLitePersistence(db) chatID := testPublicChatID - _, hasMessage, err := p.OldestMessageWhisperTimestampByChatID(chatID) + timestamps, err := p.OldestMessageWhisperTimestampByChatIDs([]string{chatID}) require.NoError(t, err) - require.False(t, hasMessage) + require.Equal(t, 0, len(timestamps)) var messages []*common.Message for i := 0; i < 10; i++ { @@ -348,10 +348,10 @@ func TestOldestMessageWhisperTimestampByChatID(t *testing.T) { err = p.SaveMessages(messages) require.NoError(t, err) - timestamp, hasMessage, err := p.OldestMessageWhisperTimestampByChatID(chatID) + timestamps, err = p.OldestMessageWhisperTimestampByChatIDs([]string{chatID}) require.NoError(t, err) - require.True(t, hasMessage) - require.Equal(t, uint64(10), timestamp) + require.Equal(t, 1, len(timestamps)) + require.Equal(t, uint64(10), timestamps[chatID]) } func TestPinMessageByChatID(t *testing.T) {