[Kde-pim] Review Request 110766: Improve DataStore::appendItemsFlags()

[Volker Krause]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110766/#review36022
-----------------------------------------------------------

Ship it!


Looks good to me, and probably also should be backported to 1.10 branch (as the ignore thread feature of KMail actually triggers this failure).

- Volker Krause


On June 1, 2013, 1:37 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110766/
> -----------------------------------------------------------
> 
> (Updated June 1, 2013, 1:37 p.m.)
> 
> 
> Review request for Akonadi and Volker Krause.
> 
> 
> Description
> -------
> 
> This patch fixes a bug that if any of the items in the batch already had some of the flags that were passed to appendItemsFlags(), the entire operation failed. I suspect this could've been causing the bug with forgetting "read" state of emails which appeared after merging batch-notifications. As a result, instead of emitting one notification for the entire batch, we emit separate notification for each flag, always with corresponding subset of really changed items. This does not really change much, since appendItemsFlags() is called with more than one flag very very rarely. 
> 
> Secondly, the patch optimizes the operation so that instead of performing two SQL queries per each item, it does only do 2 (slightly bigger :P) queries per each flag. This makes really no difference with only several items, but pleasantly speeds up larger batches (>1k items). Second optimization is that we no longer emit FlagsChanged notification with addedFlags and removedFlags fields both empty, which could happen with the previous implementation (and was happening quite a lot during sync).
> 
> 
> Update: it does /not/ fix the bug with forgetting "read" state, it just happened to me again :-/
> 
> 
> Diffs
> -----
> 
>   server/src/storage/datastore.h bac94b7 
>   server/src/storage/datastore.cpp f5a1bc6 
> 
> Diff: http://git.reviewboard.kde.org/r/110766/diff/
> 
> 
> Testing
> -------
> 
> Not really an authoritative benchmark:
> 
> [dvratil at Odin ~]$ akonadictl start &> akonadi-nopatch.txt
> [dvratil at Odin ~]$ tail -f akonadi-nopatch.txt | grep "======"
> ======== Appended 1 flags to 1 items in 1 msecs 
> ======== Appended 1 flags to 1 items in 0 msecs 
> ======== Appended 1 flags to 4854 items in 1947 msecs 
> ======== Appended 1 flags to 5223 items in 1961 msecs 
> ======== Appended 1 flags to 1029 items in 340 msecs 
> ======== Appended 1 flags to 3571 items in 1457 msecs 
> 
> 
> [dvratil at Odin ~]$ akonadictl start &> akonadi-patch.txt
> [dvratil at Odin ~]$ tail -f akonadi-patch.txt | grep "======"
> ======== Appended 1 flags to 1 items in 0 msecs 
> ======== Appended 1 flags to 1 items in 0 msecs 
> ======== Appended 1 flags to 8960 items in 1842 msecs 
> ======== Appended 1 flags to 6098 items in 1177 msecs 
> ======== Appended 1 flags to 1960 items in 335 msecs 
> ======== Appended 1 flags to 9941 items in 1919 msecs 
> ======== Appended 1 flags to 663 items in 87 msecs 
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/