fix(search): stop the chat index corrupting itself, and cut the service-side floods

go/chat/emojisource.go

@@ -494,16 +494,28 @@ func (s *DevConvEmojiSource) ToggleAnimations(ctx context.Context, uid gregor1.U
 
 func (s *DevConvEmojiSource) RemoteToLocalSource(ctx context.Context, uid gregor1.UID,
 	remote chat1.EmojiRemoteSource,
+) (source chat1.EmojiLoadSource, noAnimSource chat1.EmojiLoadSource, err error) {
+	return s.remoteToLocalSource(ctx, remote, s.animationsDisabled(ctx, uid))
+}
+
+// callers converting a whole set of emojis pass noAnim in so the gregor state is
+// read once for the set instead of once per emoji
+func (s *DevConvEmojiSource) remoteToLocalSource(ctx context.Context, remote chat1.EmojiRemoteSource,
+	noAnim bool,
 ) (source chat1.EmojiLoadSource, noAnimSource chat1.EmojiLoadSource, err error) {
 	typ, err := remote.Typ()
 	if err != nil {
 		return source, noAnimSource, err
 	}
-	noAnim := s.animationsDisabled(ctx, uid)
 	switch typ {
 	case chat1.EmojiRemoteSourceTyp_MESSAGE:
 		msg := remote.Message()
 		sourceURL := s.G().AttachmentURLSrv.GetURL(ctx, msg.ConvID, msg.MsgID, false, noAnim, true)
+		if noAnim {
+			// same arguments, so the second lookup would return the same URL
+			ret := chat1.NewEmojiLoadSourceWithHttpsrv(sourceURL)
+			return ret, ret, nil
+		}
 		noAnimSourceURL := s.G().AttachmentURLSrv.GetURL(ctx, msg.ConvID, msg.MsgID, false, true, true)
 		return chat1.NewEmojiLoadSourceWithHttpsrv(sourceURL),
 			chat1.NewEmojiLoadSourceWithHttpsrv(noAnimSourceURL), nil
@@ -573,6 +585,9 @@ func (s *DevConvEmojiSource) getNoSet(ctx context.Context, uid gregor1.UID, conv
 	}
 	convs := ibox.Convs
 	seenAliases := make(map[string]int)
+	// read once for the whole set: this runs for every emoji the user has, and each
+	// read is a full gregor state fetch
+	noAnim := s.animationsDisabled(ctx, uid)
 	addEmojis := func(convs []chat1.ConversationLocal, isCrossTeam bool) {
 		if opts.OnlyInTeam && isCrossTeam {
 			return
@@ -596,7 +611,7 @@ func (s *DevConvEmojiSource) getNoSet(ctx context.Context, uid gregor1.UID, conv
 					continue
 				}
 				var creationInfo *chat1.EmojiCreationInfo
-				source, noAnimSource, err := s.RemoteToLocalSource(ctx, uid, storedEmoji)
+				source, noAnimSource, err := s.remoteToLocalSource(ctx, storedEmoji, noAnim)
 				if err != nil {
 					s.Debug(ctx, "Get: skipping emoji on remote-to-local error: %s", err)
 					continue
@@ -954,9 +969,11 @@ func (s *DevConvEmojiSource) Decorate(ctx context.Context, body string, uid greg
 	offset := 0
 	added := 0
 	isReacji := messageType == chat1.MessageType_REACTION
+	// read once for the whole message rather than once per matched emoji
+	noAnim := s.animationsDisabled(ctx, uid)
 	for _, match := range matches {
 		if remoteSource, ok := emojiMap[match.name]; ok {
-			source, noAnimSource, err := s.RemoteToLocalSource(ctx, uid, remoteSource)
+			source, noAnimSource, err := s.remoteToLocalSource(ctx, remoteSource, noAnim)
 			if err != nil {
 				s.Debug(ctx, "Decorate: failed to get local source: %s", err)
 				continue

go/chat/participantsource.go

@@ -18,10 +18,24 @@ import (
 )
 
 type partDiskStorage struct {
-	Uids []gregor1.UID
-	Hash string
+	Uids     []gregor1.UID
+	Hash     string
+	CachedAt time.Time
 }
 
+// How long a cached participant list is served without asking the server. Every
+// localization of a team conversation asks for participants, so a team screen
+// with N channels used to mean N remote round trips every time it loaded, even
+// when nothing had changed and the server only answered HashMatch. Membership
+// changes arrive by server push for anything that re-localizes the conversation.
+//
+// Known gap: nothing deletes the cached entry, and every production caller
+// passes DataSourceAll (the RemoteOnly path exists but is unused), so a
+// membership change that does not re-localize can take up to this long to show
+// up in a participant list or @-mention completion. Before the cache that was
+// one cheap HashMatch round trip per localization instead.
+const participantsCacheFreshness = 5 * time.Minute
+
 type CachingParticipantSource struct {
 	globals.Contextified
 	utils.DebugLabeler
@@ -130,6 +144,10 @@ func (s *CachingParticipantSource) GetNonblock(ctx context.Context, uid gregor1.
 			if found {
 				resCh <- types.ParticipantResult{Uids: local.Uids}
 				localHash = local.Hash
+				if !local.CachedAt.IsZero() &&
+					s.G().GetClock().Since(local.CachedAt) < participantsCacheFreshness {
+					return
+				}
 			}
 		default:
 		}
@@ -147,12 +165,23 @@ func (s *CachingParticipantSource) GetNonblock(ctx context.Context, uid gregor1.
 			}
 			if partRes.HashMatch {
 				s.Debug(ctx, "GetNonblock: hash match on remote, all done")
+				// nothing changed, but record that we just checked so the next
+				// localization of this conv can be served from disk
+				var local partDiskStorage
+				found, err := s.encryptedDB.Get(ctx, s.dbKey(uid, conv.GetConvID()), &local)
+				if err == nil && found {
+					local.CachedAt = s.G().GetClock().Now()
+					if err := s.encryptedDB.Put(ctx, s.dbKey(uid, conv.GetConvID()), local); err != nil {
+						s.Debug(ctx, "GetNonblock: failed to record refresh time: %s", err)
+					}
+				}
 				return
 			}
 
 			if err := s.encryptedDB.Put(ctx, s.dbKey(uid, conv.GetConvID()), partDiskStorage{
-				Uids: partRes.Uids,
-				Hash: partRes.Hash,
+				Uids:     partRes.Uids,
+				Hash:     partRes.Hash,
+				CachedAt: s.G().GetClock().Now(),
 			}); err != nil {
 				s.Debug(ctx, "GetNonblock: failed to store participants: %s", err)
 			}