[Bug 28791] pendingproc_cleanup_pending_messages_db has been disabled

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28791

--- Comment #1 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-03-07 11:29:44 PST ---
(In reply to comment #0)
> In order to fix bug #28787 I commented out
> pendingproc_cleanup_pending_messages_db.
> According to the doc of this fucntion, that's just an optionnal optimisation so
> that's not the end of the world but we should fix it properly.
> 
> Few things seem wrong to me in it:
> 
> - SQL procedures can take a while. Is there any reason to block returning from
> ObserveChannels on this? If not, this function should be removed from the
> channel preparing chain.

I think the original goal was to be able to catch messages in-between
receptions and message being acknowledged (which would generally occurs when
the logger crash). But as we don't want to log them twice, we store the message
ID associated with the channel pending list, and use a hash of different
information including this ID to detect that message has been acknowledge.

As soon as the message is acknowledged, there is no use of keeping the message
in the pending message list (in database).

> 
> - tpl_channel_text_clean_up_stale_tokens() iterates on the tokens and call
> _tpl_log_store_sqlite_set_acknowledgment() for each token. So a new SQL command
> is executed for each token! I'm not a SQL guru but I'm sure we could create a
> single command operating on all the tokens.
> 
> - According to pendingproc_cleanup_pending_messages_db's DEBUG message, this
> function is supposed to remove entries from the DB. But
> _tpl_log_store_sqlite_set_acknowledgment() does a SQL UPDATE so nothing is
> actually removed. Is that expected?

As explained, the implementation is off from the original goal. Instead of
removing the messages from the pending message cache (stored to disc in case of
crash) it simply clears the pending ID to mark the message as acknowledge. This
extra step takes time, is indeed implemented the slow way and does not reduce
the memory usage as pretended by the comment. To save the day, the SQLite log
store clears older logs after a certain period of time, keeping it small.

The pending message cache is important to avoid logging twice, or not logging
at all messages when recovering from crash. I think this concept of pending
message cache should be better abstracted, especially without coming call logs,
where the message cache could be used to rebuild call event that where lost due
to crash during call while recovery take place after the call has ended.

If/when SQLite will become the main writable log-store, the need of separate
message cache might disappear with the ability to store incomplete messages
early and update them while more information are being retreived.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.