[Kde-pim] Review Request 111885: Avoid starting a database transaction in CollectionSync when no updates are needed

[Alessandro Pignotti]

> On Aug. 7, 2013, 12:14 p.m., Dan Vrátil wrote:
> > akonadi/collectionsync.cpp, line 633
> > <http://git.reviewboard.kde.org/r/111885/diff/4/?file=176263#file176263line633>
> >
> >     Why do you need to re-fetch the collections? Fetch is read-only, so I don't think it has to happen again within the transaction.

We need to fetch again since in the time between the first fetch and starting the transaction the collections may have changed. To guarantee correctness we need to fetch them again.


> On Aug. 7, 2013, 12:14 p.m., Dan Vrátil wrote:
> > akonadi/collectionsync.cpp, line 660
> > <http://git.reviewboard.kde.org/r/111885/diff/4/?file=176263#file176263line660>
> >
> >     This is related to my question about why you need to restart the job when you create a new transaction. Why can't the code execution simply continue here?
> >     
> >     If there's no update needed then good, the job has already called emitResult(), so you could just terminate here. But if there's an update needed, you have started the transaction already and you can now just start writing the changes to Akonadi without restarting the job.
> >     
> >     So the code could be somthing like
> >     
> >     // Check whether there's anything to update
> >     if ( !checkUpdateNecessity() ) {
> >       q->emitResult();
> >       return;
> >     }
> >     
> >

See commend above. We need to start again from scratch. execute will be called again, this time with a valid currentTransaction and sync will actually happen.


- Alessandro


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


On Aug. 5, 2013, 1:27 p.m., Alessandro Pignotti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111885/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2013, 1:27 p.m.)
> 
> 
> Review request for Akonadi.
> 
> 
> Description
> -------
> 
> Rationale: Unconditionally starting a transaction on every collection
> sync has an high overhead on disk activity since transaction bookkeping
> on the database requires writing to disk. Most often collection sync are
> unnecessary since collection are rarely modified, so we end up causing a
> write to disk on an operation which would be read-only.
> 
> The proposed solution is two run the collection sync in 2 phases:
> 
> 1) Without starting a transaction, the currently known local collections
> are loaded and are compared against the received remote ones. If no
> difference is found update is stopped right away
> 
> 2) If there are differences a transaction is started, local collections
> are loaded again to guarantee consistency and the update is applied
> inside the transaction
> 
> 
> This addresses bug 322083.
>     http://bugs.kde.org/show_bug.cgi?id=322083
> 
> 
> Diffs
> -----
> 
>   akonadi/collectionsync.cpp 0b85c2be168511486b484c7fcf8de3d157e876d4 
>   akonadi/collectionsync_p.h 2ea53c5e0f2d2ce8e1a684b8f685ab6683cd1493 
> 
> Diff: http://git.reviewboard.kde.org/r/111885/diff/
> 
> 
> Testing
> -------
> 
> Code builds and collection fetching seems to be working correctly
> 
> 
> Thanks,
> 
> Alessandro Pignotti
> 
>

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