Bug 31031

Summary: Add (optional) support for Nokia's heartbeat service
Product: Wocky Reporter: Will Thompson <will>
Component: GeneralAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium Keywords: patch
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=shortlog;h=refs/heads/iphb
Whiteboard:
i915 platform: i915 features:

Description Will Thompson 2010-10-21 11:31:42 UTC
Maemo 5's Loudmouth was patched to add support for Nokia's IP heartbeat service, which helps to reduce radio wake-ups. Processes which need to make periodic network requests, but which do not care about the *exact* period, ask a daemon to wake them up at some point during a certain time interval in the future; the daemon chooses to wake up processes slightly early if it means that they can be woken up at the same time as other processes.

This branch adds similar code to Wocky. It's much better than the code that was in Loudmouth: it supports varying the interval on the fly, and supports falling back to internal timeouts if the heartbeat service breaks somehow.

I've tested this branch in Gabble, both with and without iphb support, and empirically tested the fallback when the heartbeat service dies. There is no automated test for the iphb code: while the wire protocol is pretty simple, I didn't really fancy implementing a dummy service for the test suite, particularly since iphb hard-codes the path to the unix socket to be, ahem, /dev/shm/iphb.

Functionally, this does change the ping behaviour of Wocky somewhat. See http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=commitdiff;h=beeb0676e3708c91dca867c1620ae72b204c1dba for details of the change. I don't think it's too big a deal.
Comment 1 Will Thompson 2010-10-22 07:51:54 UTC
(In reply to comment #0)
> Functionally, this does change the ping behaviour of Wocky somewhat. See
> http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=commitdiff;h=beeb0676e3708c91dca867c1620ae72b204c1dba
> for details of the change. I don't think it's too big a deal.

(This commit contains a comment explaining how this changes Wocky's ping behaviour; it isn't the whole branch.)
Comment 2 Vivek Dasmohapatra 2010-10-22 11:11:07 UTC
Nothing major: 1 trivium and one question:

Trivium:

where DEBUG output is to tell us there's been a wakeup or a ping 
has gone out or the ping timeout has changed, it might be worth 
logging the (high-ish res)time at which said event occurred

Question:

self->next_wakeup.tv_sec += (max_interval - self->max_interval);
can go -ve, I _think_ this means we just wake up straightaway,
I assume that's Ok?
Comment 3 Will Thompson 2010-10-25 04:35:23 UTC
(In reply to comment #2)
> Nothing major: 1 trivium and one question:
> 
> Trivium:
> 
> where DEBUG output is to tell us there's been a wakeup or a ping 
> has gone out or the ping timeout has changed, it might be worth 
> logging the (high-ish res)time at which said event occurred

Added.

> Question:
> 
> self->next_wakeup.tv_sec += (max_interval - self->max_interval);
> can go -ve, I _think_ this means we just wake up straightaway,
> I assume that's Ok?

Yeah, that was the intention. I've added a clarifying comment.
Comment 4 Will Thompson 2010-10-25 04:35:50 UTC
Oh I also made the test slightly faster by reducing the number of pings it waits for.
Comment 5 Will Thompson 2010-11-10 08:05:16 UTC
Merged! Hooray.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.