Review Request 125152: KBuildSycoca: replace dbus-based locking with a lock file.

[David Faure]

> On Sept. 11, 2015, 12:56 p.m., Albert Astals Cid wrote:
> > src/kbuildsycoca/kbuildsycoca.cpp, line 370
> > <https://git.reviewboard.kde.org/r/125152/diff/1/?file=402639#file402639line370>
> >
> >     .lock() calls scare me, this will block forever if say kbuildsycoca crashed, right?
> >     
> >     How do we recover from that? Will this also lock the UI when we move the sycoca creation to apps?
> 
> David Edmundson wrote:
>     QLockFile writes a timestamp and PID  into the lockfile. This is checked when the next process checks the lock.
> 
> Albert Astals Cid wrote:
>     Ah nice, then just have the question if "Will this also lock the UI when we move the sycoca creation to apps?"

Let me answer all these questions in order :-)

1) .lock() will not block *forever* if kbuildsycoca crashed, it will wait at most 30 seconds, in the worst case where the PID got reused meanwhile. The more likely case is that the PID is no longer in use, in which case QLockFile will detect immediately that the lock file is stale, and delete it.

2) Yes this code will block the GUI if called from a GUI application. But note that we already do exactly that: ksycoca currently calls (in KSycocaPrivate::buildSycoca) QProcess::waitForFinished in order to wait for kbuildsycoca to be done before reopening the DB. We've had that forever. The apps do need the stuff, in synchronous calls (e.g. KMimeTypeTrader) so they have to wait for the sycoca db to exist. OK, with one difference though, apps only used to do this when ksycoca didn't exist at all or its version number was too low (i.e. after upgrading the lib). With my changes for the last month, this would also happen when new desktop files have been installed. That is the bugfix for the 15 years old issues with sycoca missing some stuff...
I think the app blocking (for a tiny bit of time, this is an incremental rebuild) in order to see the new desktop files is actually an improvement, and there's not really another choice anyway. And merging kbuildsycoca into the lib will make the rebuild faster, compared to QProcess + relocations + initializations + loading the existing ksycoca in the kbuildsycoca process... all of that will go away.

Summary: we'll still block, more often (mostly for developers), but for a smaller amount of time.

d_ed: the lock file contains PID, appname, hostname. The timestamp used for the 30s check is the file's modification time, simply.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125152/#review85178
-----------------------------------------------------------


On Sept. 11, 2015, 7:23 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125152/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2015, 7:23 a.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> This is a more traditional way to protect concurrent access to a file,
> and it will (more easily) allow to have one lock file per sycoca file
> when we have LANG and DIRS in the filename.
> 
> It doesn't exactly remove the dependency on DBus though, since we still
> use it for the notification after the rebuild.
> 
> As an additional improvement, if a process has to wait for another
> to rebuild sycoca, when it can finally get the lock file, it does a quick
> mtime check and if there were no additional changes compared to the
> time of the last check (as stored in ksycoca's global header) then there
> is nothing to do, we can do an early return.
> 
> 
> Diffs
> -----
> 
>   src/kbuildsycoca/kbuildsycoca.cpp 2a1c6b3e2ef2add82f710e1a18cf77028f437407 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 5a0c0e8b47996750a1bf25139e1c21eb0392044b 
>   src/kbuildsycoca/kbuildsycoca_p.h 1be91bf21bb6908a63ffe6c156830838ed1ff429 
>   src/sycoca/ksycoca.h 9d8b21e3c0f08375bece923c4029b54617f04b7f 
>   src/sycoca/ksycoca.cpp 36718e3ee951df19494031f17dec29d1f4dd39c5 
>   src/sycoca/ksycoca_p.h 1a377e3586330a22cbcab507ea7d7f16d1563f13 
> 
> Diff: https://git.reviewboard.kde.org/r/125152/diff/
> 
> 
> Testing
> -------
> 
> Running "kbuildsycoca --noincremental &" 5 times in a row quickly, and watching the debug output, shows that only the first one rebuilds, all others wait (approx 1.5s) and then realize there is nothing more to do, and exit.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150911/a183e8dd/attachment.html>