Review Request 123026: do not delay run response

[Harald Sitter]

> On March 18, 2015, 1:28 p.m., Aleix Pol Gonzalez wrote:
> > +1 it makes sense to me, although I don't know why the setDelayedReply(true) was added in the first place...

git blame isn't very helpful to find out why it was introduced. it was introduced as a larger untanglement of client and worker dbus interaction so my theory right now is that the delayed reply was in fact originally added to do asyncness, alas of course it has nothing to do with asyncness eitherway because run did and does hard block on getting auth anyway


- Harald


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


On March 18, 2015, 11:58 a.m., Harald Sitter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123026/
> -----------------------------------------------------------
> 
> (Updated March 18, 2015, 11:58 a.m.)
> 
> 
> Review request for Kubuntu, LibQApt, Aleix Pol Gonzalez, and Floris-Andrei Stoica-Marcu.
> 
> 
> Repository: libqapt
> 
> 
> Description
> -------
> 
> setDelayedReply means we would have to issue a reply ourselves which from
> what I can see never happens. So after a certain timeout (30 or so seconds)
> dbus will automatically issue a NoReply error to the client. While this is
> a truthy error in that there really was no reply in the time frame its
> meaning client side is unclear as NoReply can then mean actually no reply
> or everything was fine but no reply was issued. Latter appears to be the
> success case because we never issue a reply, as already mentioned.
> This in turn makes it impossible to tell client side when run simply took
> too long or was successful.
> 
> To improve the situation simply do not mark the reply delayed. This makes
> qtdbus send a default all-good reply to the client unless we issue an auth
> error first.
> 
> Ideally the entire timeout scenario should be taken out of the picture by
> employing an entirely async API design (i.e. run always replies immediately
> but asyncronously switches state to either an error or running). The only
> other option to prevent the noreply from being issued while waiting for
> auth is to have the client timeout increased to indefinite which would seem
> misguided as most of the operations have actual potential to result in
> no-reply if something deadlocks or takes indeed way too long.
> 
> 
> Diffs
> -----
> 
>   src/worker/transaction.cpp 4333b26b211774c560ff046ab51b638b83282f1c 
> 
> Diff: https://git.reviewboard.kde.org/r/123026/diff/
> 
> 
> Testing
> -------
> 
> make && make install && make test
> 
> # Runtime Test
> 
> set apt download limit to 8kb/s http://askubuntu.com/a/50405
> 
> ## without the fix
> 
> - start muon-updater
> - do an update
> - provide authorization
> - after ~30 seconds the no-auth error comes up as per review #119793
> 
> ## with the fix
> 
> make sure qaptworker3 is not running
> 
> ### failure condition
> 
> - start muon-updater
> - do an update
> - do not provide authorization
> - after ~30 seconds the no-auth error comes up as per review #119793
> 
> ### success condition
> 
> - start muon-updater
> - do an update
> - provide authorization
> - update finishes without a no-auth error
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kubuntu-devel/attachments/20150318/475e6786/attachment.html>