Skip to content

Commit

Permalink
load max messages on background loader, and fix handling of cached de…
Browse files Browse the repository at this point in the history
…leted messages (keybase#24520)
  • Loading branch information
mmaxim authored May 24, 2021
1 parent 2cc9c22 commit 69d3e0d
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 9 deletions.
22 changes: 22 additions & 0 deletions go/chat/convsource.go
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,28 @@ func (s *HybridConversationSource) GetMessages(ctx context.Context, convID chat1
if err := s.mergeMaybeNotify(ctx, conv, uid, rmsgsUnboxed, reason); err != nil {
return nil, err
}

// The localizer uses UnboxQuickMode for unboxing and storing messages. Because of this, if there
// is a message in the deep past used for something like a channel name, headline, or pin, then we
// will never actually cache it. Detect this case here and put a load of the messages onto the
// background loader so we can get these messages cached with the full checks on UnboxMessage.
if reason == chat1.GetThreadReason_LOCALIZE && globals.CtxUnboxMode(ctx) == types.UnboxModeQuick {
s.Debug(ctx, "GetMessages: convID: %s remoteMsgs: %d: cache miss on localizer mode with UnboxQuickMode, queuing job", convID, len(remoteMsgs))
// implement the load entirely in the post load hook since we only want to load those
// messages in remoteMsgs. We can do that by specifying a 0 length pagination object.
if err := s.G().ConvLoader.Queue(ctx, types.NewConvLoaderJob(convID, &chat1.Pagination{Num: 0},
types.ConvLoaderPriorityLowest, types.ConvLoaderUnique,
func(ctx context.Context, tv chat1.ThreadView, job types.ConvLoaderJob) {
reason := chat1.GetThreadReason_BACKGROUNDCONVLOAD
if _, err := s.G().ConvSource.GetMessages(ctx, convID, uid, remoteMsgs, &reason,
customRi, resolveSupersedes); err != nil {
s.Debug(ctx, "GetMessages: error loading UnboxQuickMode cache misses: ", err)
}

})); err != nil {
s.Debug(ctx, "GetMessages: error queuing conv loader job: %+v", err)
}
}
}

// Form final result
Expand Down
3 changes: 2 additions & 1 deletion go/chat/localizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,9 @@ func (s *localizerPipeline) localizeConversation(ctx context.Context, uid gregor
summaries = append(summaries, tlfSummary)
}
}
reason := chat1.GetThreadReason_LOCALIZE
msgs, err := s.G().ConvSource.GetMessages(ctx, conversationRemote.GetConvID(),
uid, utils.PluckMessageIDs(summaries), nil, nil, false)
uid, utils.PluckMessageIDs(summaries), &reason, nil, false)
if !s.isErrPermanent(err) {
errTyp = chat1.ConversationErrorType_TRANSIENT
}
Expand Down
14 changes: 9 additions & 5 deletions go/chat/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,16 @@ func decode(data []byte, res interface{}) error {
type SimpleResultCollector struct {
res []chat1.MessageUnboxed
target, cur, curScan int

// countAll controls whether or not deleted messages should count toward target
countAll bool
}

var _ ResultCollector = (*SimpleResultCollector)(nil)

func (s *SimpleResultCollector) Push(msg chat1.MessageUnboxed) {
s.res = append(s.res, msg)
if !msg.IsValidDeleted() {
if s.countAll || !msg.IsValidDeleted() {
s.cur++
}
s.curScan++
Expand Down Expand Up @@ -168,9 +171,10 @@ func (s *SimpleResultCollector) SetTarget(num int) {
s.target = num
}

func NewSimpleResultCollector(num int) *SimpleResultCollector {
func NewSimpleResultCollector(num int, countAll bool) *SimpleResultCollector {
return &SimpleResultCollector{
target: num,
target: num,
countAll: countAll,
}
}

Expand Down Expand Up @@ -968,7 +972,7 @@ func (s *Storage) ResultCollectorFromQuery(ctx context.Context, query *chat1.Get
s.Debug(ctx, "ResultCollectorFromQuery: types: %v", query.MessageTypes)
return NewTypedResultCollector(num, query.MessageTypes)
}
return NewSimpleResultCollector(num)
return NewSimpleResultCollector(num, false)
}

func (s *Storage) fetchUpToMsgIDLocked(ctx context.Context, rc ResultCollector,
Expand Down Expand Up @@ -1224,7 +1228,7 @@ func (s *Storage) IsTLFIdentifyBroken(ctx context.Context, tlfID chat1.TLFID) bo
}

func (s *Storage) getMessage(ctx context.Context, convID chat1.ConversationID, uid gregor1.UID, msgID chat1.MessageID) (*chat1.MessageUnboxed, Error) {
rc := NewSimpleResultCollector(1)
rc := NewSimpleResultCollector(1, true)
if err := s.engine.ReadMessages(ctx, rc, convID, uid, msgID, 0); err != nil {
// If we don't have the message, just keep going
if _, ok := err.(MissError); ok {
Expand Down
2 changes: 1 addition & 1 deletion go/chat/storage/storage_ephemeral_purge.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (s *Storage) EphemeralPurge(ctx context.Context, convID chat1.ConversationI
} else {
target = maxHoles
}
rc := NewHoleyResultCollector(maxHoles, NewSimpleResultCollector(target))
rc := NewHoleyResultCollector(maxHoles, NewSimpleResultCollector(target, false))
err = s.engine.ReadMessages(ctx, rc, convID, uid, maxMsgID, 0)
switch err.(type) {
case nil:
Expand Down
3 changes: 3 additions & 0 deletions go/protocol/chat1/common.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion protocol/avdl/chat1/common.avdl
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,8 @@
COINFLIP_9,
BOTCOMMANDS_10,
EMOJISOURCE_11,
FORWARDMSG_12
FORWARDMSG_12,
LOCALIZE_13
}

enum ReIndexingMode {
Expand Down
3 changes: 2 additions & 1 deletion protocol/json/chat1/common.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions shared/constants/types/rpc-chat-gen.tsx

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 69d3e0d

Please sign in to comment.