Page MenuHomePhabricator

[server] Don't unset TRUNCATED if threadCursor is null
ClosedPublic

Authored by ashoat on Mar 11 2022, 11:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 5, 12:22 AM
Unknown Object (File)
Fri, Feb 21, 10:24 PM
Unknown Object (File)
Jan 28 2025, 9:09 AM
Unknown Object (File)
Jan 28 2025, 7:09 AM
Unknown Object (File)
Jan 28 2025, 7:09 AM
Unknown Object (File)
Jan 21 2025, 7:42 AM
Unknown Object (File)
Jan 15 2025, 8:01 PM
Unknown Object (File)
Dec 29 2024, 3:23 PM

Details

Summary

While working on the previous diff, I noticed an issue here. Before D3351/D3352, we didn't actually use fetchMostRecentMessages anywhere in the codebase, which meant that we never set a threadCursor to null.

Now that we're actually using null threadCursors, we need to consider how they're handled here. In this particular case, we have some logic to unset TRUNCATED on the assumption that the result set is contiguous.

I don't think we can or should assume that for fetchMostRecentMessages. Instead, I think we should set TRUNCATED so that the MessageStore for that thread gets cleared, since we can't guarantee that whatever is currently in the MessageStore will be contiguous with the most recent messages.

Test Plan
  1. I tested different values of defaultMaxMessageAge such that some threads would return fewer than defaultNumberPerThread, and some threads would return 0
  2. I tested threads that needed a fetchMostRecentMessages, and threads that needed multiple fetchMessagesBeforeCursor to get to the start
  3. I made sure all threads loaded all the way to the start of a thread if I kept scrolling

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable