Skip to content

Commit

Permalink
[IMP] mail: avoid costly queries on messages
Browse files Browse the repository at this point in the history
The combined domain for moderated messages used to trigger a complex
database query with two OR conditions on the mail_message table.
Unfortunately PostgreSQL seems to be unable to optimize this with
indexes, nor to detect that one of the conditions is void (included `AND
FALSE`). On large mail_message tables, this significantly slows down
calls to message_fetch, e.g. 6s instead of ~70ms, while returning the
same results.

As a workaround, move the logic for generating the moderation domain to
the server side, where it can be:
 - optimized out when the user is not a moderator
 - split into 2 distinct DB queries in order to avoid the pathological
   case

After this change, calls to message_fetch with and without messages
to moderate execute in respectively ~50ms and ~150ms, instead of 6s, in
a test database with 25 million messages.

The modified message_fetch signature is backwards-compatible.
  • Loading branch information
odony committed Sep 12, 2018
1 parent c62b26d commit 004fa9b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
14 changes: 12 additions & 2 deletions addons/mail/models/mail_message.py
Expand Up @@ -413,8 +413,18 @@ def message_fetch_failed(self):
return messages._format_mail_failures()

@api.model
def message_fetch(self, domain, limit=20):
return self.search(domain, limit=limit).message_format()
def message_fetch(self, domain, moderated_channels=None, limit=20):
messages = self.search(domain, limit=limit)
if moderated_channels and self.env.user.moderation_channel_ids:
# Split load moderated and regular messages, as the ORed domain can
# cause performance issues on large databases.
moderated_messages_dom = [['model', '=', 'mail.channel'],
['res_id', 'in', moderated_channels],
['need_moderation', '=', True]]
messages |= self.search(moderated_messages_dom, limit=limit)
# Truncate the results to `limit`
messages = messages.sorted(key='id', reverse=True)[:30]
return messages.message_format()

@api.multi
def message_format(self):
Expand Down
15 changes: 7 additions & 8 deletions addons/mail/static/src/js/services/chat_manager.js
Expand Up @@ -1151,13 +1151,8 @@ var ChatManager = AbstractService.extend({
(channel.id === 'channel_inbox') ? [['needaction', '=', true]] :
(channel.id === 'channel_starred') ? [['starred', '=', true]] :
(channel.id === 'channel_moderation') ? [['need_moderation', '=', true]] :
['|',
'&', '&',
['model', '=', 'mail.channel'],
['res_id', 'in', [channel.id]],
['need_moderation', '=', true],
['channel_ids', 'in', [channel.id]]
];
[['channel_ids', 'in', [channel.id]]];
var moderated_channels = typeof(channel.id) === 'number' ? [channel.id] : false;
var cache = this._getChannelCache(channel, options.domain);

if (options.domain) {
Expand All @@ -1172,7 +1167,11 @@ var ChatManager = AbstractService.extend({
model: 'mail.message',
method: 'message_fetch',
args: [domain],
kwargs: {limit: LIMIT, context: session.user_context},
kwargs: {
moderated_channels: moderated_channels,
limit: LIMIT,
context: session.user_context
},
})
.then(function (msgs) {
if (!cache.all_history_loaded) {
Expand Down

0 comments on commit 004fa9b

Please sign in to comment.