[Kde-pim] Akonadi batch notifications review

[laurent Montel]

Hi,
On two computers, maildir is blocked after sometime.
I need to delete my agent_config_akonadi_maildir_resource_0_changes.dat to 
unlock it :(
.dat file must be broken.

(I just selected some mails that’s all. I didn’t try to move mails or delete 
mails)

Regards.

Le samedi 18 mai 2013 19:05:24 Daniel Vrátil a écrit :
> On Saturday 18 of May 2013 13:12:26 Volker Krause wrote:
> > Hi,
> > 
> > thanks a lot for implementing this! And sorry for taking so long with the
> > review.
> 
> Hi,
> 
> thanks for reviewing the code. I've fixed the issues you mentioned and
> merged the branches in akonadi, kdepimlibs and kdepim-runtime to master.
> 
> Guys, if you see any regressions, please let me know asap :-)
> 
> > On Monday 29 April 2013 21:28:41 Daniel Vrátil wrote:
> <snip>
> 
> > > A bit of elaboration on the changes I've done.
> > > 
> > > Akonadi Server
> > > Akonadi Server now uses and emits NotificationMessageV2. Unlike the
> > > "V1",
> > > it can represent a single operation on multiple entities. Akonadi server
> > > emits the NotificationMessageV2 via notifyV2() signal, and when there's
> > > at least one subscriber that does not support the new version (support
> > > for V2 is announced via client capabilities), it splits the V2
> > > notification into multiple V1 notifications and emits them "the old
> > > way",
> > > via notify(). A completely new operation called "ModifyFlags" has been
> > > added. For this operation, the NotificationMessageV2 has two additional
> > > attributes - addedFlags and removedFlags. When
> > > Akonadi::AgentBase::ObserverV3 is used itemModified() will no longer
> > > notify about flag changes (see below).
> > 
> > Just some details, the overall approach looks good:
> > DataStore::setItemFlags(): if the transaction is really needed there (and
> > not already done by the caller), use the Transaction RAII class to ensure
> > automatic rollback (there might be exceptions in there).
> 
> Dropped the transaction completely - it's called from a transaction in
> Storage::parseStream()
> 
> > NotificationMessageV2::List should probably be a QVector, we couldn't
> > change that in V1 anymore.
> 
> Done. Had to add NotificationMessageV2::appendAndCompress() for QList,
> because in Monitor we work with QQueue, which cannot be converted to
> non-const reference to QVector.
> 
> > NotificationMessageV2::Item should probably be called "Entity" if it also
> > can represent Collections.
> 
> Done
> 
> > > Akonadi::Monitor
> 
> <snip>
> 
> > > Akonadi::AgentBase::ObserverV3
> > > A new ObserverV3 has been added, with virtual method for every new batch
> > > operation. Very important thing here is that if you use ObserverV3, you
> > > will never be delivered the non-batch versions of notification from V1
> > > or
> > > V2. So even when ObserverV3::itemsRemoved() is not reimplemented in an
> > > agent/resource, ObserverV2::itemRemoved() will simply not be called.
> > > This
> > > is because of design limitation, that would require the Observer telling
> > > back to Monitor to re-queue, split and re-emit the batch notification as
> > > a set of per- item notifications.
> > 
> > Yep, keeping backward compatibility there is tricky, but I like your
> > approach, the ObserverV3 constraints are fine IMHO.
> > 
> > We can look into how to simplify all this for KDE5 then, is there a
> > use-case for keeping the single object signals?
> 
> I agree we could merge all Observers to one and drop single-item singals. If
> users don't want to support batches in their resources, they can just use
> Q_FOREACH :-)
> 
> > One issue in the implementation:
> > ResourceBase::itemsX(): these skip all changes if any of the items is
> > invalid (empty RID), that seems a bit extreme, maybe just drop them from
> > the list instead?
> 
> Fixed.
> 
> 
> Cheers,
> Dan
> 
> > > IMAP resource
> > > IMAP resource has been updated to support itemsMoved and itemsRemoved
> > > and
> > > itemsFlagsChanged.
> > 
> > Looks good.
> > 
> > 
> > Feel free to merge it, and thanks again for all the work :) If anyone
> > happens to look for things to work on, we can now optimize many resources
> > by making use of the new notifications :)
> > 
> > regards,
> > Volker
-- 
Laurent Montel | laurent.montel at kdab.com | KDE/Qt Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, Sweden (HQ) +46-563-540090


_______________________________________________
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/