Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(imap): FETCH only flags for partial message updates #8453

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented May 4, 2023

Fixes #8451

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@JohannesGGE
Copy link
Contributor

before:

C: 7 UID FETCH 2726 (FLAGS BODY.PEEK[HEADER])
S: * 1 FETCH (UID 2726 FLAGS (\Seen NonJunk) BODY[HEADER] {8549}
Return-Path: <***@***>
Authentication-Results: ***
Received: ***
DKIM-Signature: ***
X-UI-Sender-Class: ***
Received: from ***
Date: ***
To: ***
From: ***
Reply-To: ***
Subject: ***
Message-ID: ***
X-MA-Provider: ***
X-Data: ***
MIME-Version: ***
Content-Type: ***
X-Provags-ID: ***
Envelope-To: ***
x-tdresult: ***
x-tdcapabilities: ***
X-Spam-Flag: ***
UI-InboundReport: ***

S: )
S: 7 OK UID FETCH completed
>> Command 7 took 0.047 seconds.

after:

C: 7 UID FETCH 2726 (FLAGS)
S: * 1 FETCH (UID 2726 FLAGS (\Seen NonJunk))
S: 7 OK UID FETCH completed
>> Command 7 took 0.0289 seconds.

@ChristophWurst
Copy link
Member Author

something is odd.

341x 55.8 ms ↴ select * from oc_mail_tags where (imap_label = ?) and (user_id = ?)
341x 47.6 ms ↴ insert into oc_mail_message_tags(...) values (?)

from blackfire

@JohannesGGE
Copy link
Contributor

Found the issue. We use the messageID to check and update the tags. Without the header, we don't fetch the messageID anymore.

It could be possible to adjust the update of the tags with an additional join of mail_message_tags and mail_messages to get the uid that is fetched. Could be worth a try.

@JohannesGGE
Copy link
Contributor

The current state fails because the entries in mail_message_tags are stored with the message_id. The message_id is not part of the header and because of that not fetched in the new partial sync. \OCA\Mail\Db\MessageMapper::updateTags tries to diff on message_id and creates a generated message_id because none is found. Because of the mismatch of the message_id in mail_message_tags and mail_messages the partial sync always adds all tags to mail_message_tags (always with auto generated message_id).

It could be an option to join additionally on mail_messages in \OCA\Mail\Db\TagMapper::getAllTagsForMessages. This requires a bit more complex change because \OCA\Mail\Db\MessageMapper::updateTags needs the uid to diff and the matching message_id to create the db entry in mail_message_tags then.

For some reason the initial sync is also broken in this branch and the message_id is generated. I don't understand why, in my understanding only the partial sync is affected by the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Partial sync only has to FETCH flags, nothing else
2 participants