Review Request 124304: Reduce the KDBusService timeout from 5 minutes to 25 seconds

[Martin Klapetek]

> On July 10, 2015, 11:03 a.m., David Faure wrote:
> > Guys, shortening this value won't make anything faster. Instead, in case an app takes 26 seconds to start (slow machine, busy system, lots of initialization code...) the caller will get an error message erroneously.
> > 
> > Did you never see that in kdelibs4? Type kmail twice in a terminal, the first one goes through some slow path for some reason, and the second one tells you "DBus communication error, couldn't communicate with running instance blah blah". There's no error though.
> > 
> > Yes it's a blocking call, but it's done by the just-starting second-instance of the app, which has shown no GUI yet, so this isn't going to block some GUI for the user. Waiting is better than a wrong error IMHO.
> 
> Martin Klapetek wrote:
>     I would say that error is correct though, because it does fail to communicate with the running instance within a reasonable limit (and same error will come if app startup takes 301 seconds...old machines/mobile devices etc). I'd say such application should be fixed to not block itself during startup; receiving activation request after 2 minutes and suddenly popping to the front seems...kinda broken anyway. Also application that is unusable for minutes after launching is a sign of bad design and this feels like providing a workaround for it.
>     
>     Nevertheless, this was just something I stumbled upon while reading the code and I wasn't sure if that was intended or not. If you, the maintainer, believe this should stay, I'll just discard this.
> 
> David Faure wrote:
>     Yes it's intended, the comment in the source code even made it quite explicit :-)
>     
>     26 seconds, I have seen in the past. 301 never.
>     
>     It's not necessarily bad design for an app to take 26 seconds to initialize, if it does provide some feedback (e.g. a popup "please wait, I'm reindexing your mail folders"). There is actually a good argument for not being available on DBus before doing that, since the app isn't ready to handle incoming calls, in this "being repaired" state. This is a made up example though, I don't remember which exact cases we had where this happened.
>     
>     I maintain that the error is incorrect, because it creates two problems instead of one (the first instance is slow ok, but now on top of that we have a failing second instance).

I meant if 5 minutes was intended, not the timeout itself ;) I do think that you should be able to use your email client even when it is reindexing things, that sounds like a bg/threaded job. But that's a different discussion :)

Ok, I'll close this then.


- Martin


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


On July 9, 2015, 1:48 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124304/
> -----------------------------------------------------------
> 
> (Updated July 9, 2015, 1:48 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kdbusaddons
> 
> 
> Description
> -------
> 
> Now I don't know if that was perhaps intended, but 5 minute timeout on dbus call to activate an app seems a bit too much?
> 
> So I've reduced it to standard 25 seconds.
> 
> 
> Diffs
> -----
> 
>   src/kdbusservice.cpp ea7727d 
> 
> Diff: https://git.reviewboard.kde.org/r/124304/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150710/5f269e66/attachment-0001.html>