[refs #3210] remove unnecessary reads from Realm for messages #3265
Conversation
| (get-stored-message message-id)))))))) | ||
| (not (or (get deleted-chats chat-identifier) | ||
| (get messages message-id) | ||
| (get not-loaded-message-ids message-id)))))) |
There was a problem hiding this comment.
I'm afraid we can't get rid of the realm message read in this case, the following scenario is problematic:
You have some deleted chat and message for that chat get re-delivered to you (because you didn't ack before, or any other reason, I generally found the re-delivery quite unreliable and you always how to defend against message with same id delivered to you again).
Now we now that the message is for deleted chat (as deleted chat ids are loaded on startup) but we don't know if it's message not received before (in which case, we have to restore the chat and add message, otherwise once you delete 1-1 chat, you will never be able to have conversation with given contact anymore) or already received message (so nothing is restored and we ignore the message).
The reason for it is that for deleted chats, we don't load their message ids into db at startup.
I did it that way originally because:
- It's a quite rare case (re-delivery of message for deleted chat) so the perf impact of reading from realm (only in that specific case) is probably negligible overall
- Having
:not-loaded-message-idsfor deleted chats always in db seemed like a waste
Maybe point 2) is open to discussion and it's nothing horrible.
There was a problem hiding this comment.
@janherich good call. Returned that logic back.
src/status_im/protocol/handlers.cljs
Outdated
| message (or (get-in db message-db-path) | ||
| (and (get (:not-loaded-message-ids db) message-identifier) | ||
| (get-stored-message message-identifier)))] | ||
| message (get-in db message-db-path)] |
There was a problem hiding this comment.
Similar point like before, we only have 20 messages loaded for each chat when it's opened (more when one scrolls up), so whenever we receive update for some message which is not yet loaded (second and condition in the original expression), we should update it as well, otherwise we ignore status updates for just little bit older messages.
|
@dmitryn now it's good, but please return cofx registration |
|
@janherich it was returned. please take a look |
|
@janherich sorry, haven't push that yet. |
dmitryn
force-pushed
the
3146-realm-reads-part-3
branch
from
059a5c9
to
365fe9a
Compare
Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>
flexsurfer
force-pushed
the
3146-realm-reads-part-3
branch
from
365fe9a
to
d799396
Compare
Addresses #3210
Summary:
Replace realm read for messages data with app-db read as we don't need to touch realm since data is already in app-db.
Testing notes:
App behavior should not change. Code changed related to messages:
status: ready