dee

Merge lp://qastaging/~mhr3/dee/shared-private-connections into lp://qastaging/dee

Proposed by Michal Hruby
Status: Merged
Approved by: Mikkel Kamstrup Erlandsen
Approved revision: 344
Merged at revision: 339
Proposed branch: lp://qastaging/~mhr3/dee/shared-private-connections
Merge into: lp://qastaging/dee
Diff against target: 680 lines (+401/-45)
5 files modified
src/dee-client.c (+8/-9)
src/dee-server.c (+209/-27)
src/dee-server.h (+11/-8)
tests/test-client-server.c (+172/-1)
vapi/dee-1.0.vapi (+1/-0)
To merge this branch: bzr merge lp://qastaging/~mhr3/dee/shared-private-connections
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen (community) Approve
Review via email: mp+86545@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2011-12-16.

Description of the change

Currently we require separate socket for each DeeSharedModel that uses a private connection. This branch will allow to use the same socket (GDBusServer) with multiple DeeSharedModels.

To post a comment you must log in.
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

I need to understand this correctly. Given this branch DeeServer will
always just create one socket as long as you use it with
dee_server_new(swarm_name) for varying swarm_names. The swarms will be
multiplxed by the DeeServer instances.

The new sockets will be created if you create DeeServers with non
standard addresses.

Correct?

Revision history for this message
Michal Hruby (mhr3) wrote : Posted in a previous version of this proposal

Not exactly, dee_server_new will keep current behaviour (separate socket for each different swarm_name). But you'll be able to do dee_server_new_for_address with varying swarm names, but the same bus_address.

The reason for this is that we need a well defined socket name for the server, so that dee_client_new can work properly.
Or to try to explain better, consider that dee_server_new would try to always use the same bus_address - this would mean that if there were two completely unrelated processes which are trying to use dee's private connections (both servers). They would try to create the same socket, so one would necessarily fail. Moreover we can't just add a pid to the socket name, otherwise DeeClient in a different process couldn't easily connect to the server.

I hope that makes it clear-ish. :)

The problem this brings though, is that DeeServers will be emitting peer-found & lost for peers from different swarms, which is sort of odd, but I don't think too much of an issue.

(the idea also is to use the same GDBusConnection for clients using the same bus_address, but that doesn't really create any problems unlike the server-side)

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

Can you add a detailed explanation of the behavior to dee_server_new() and dee_server_new_for_address()? It's crucial knowledge for consumers to know if/when they create sockets - and it'll help the review a lot.

Doubly important to know precisely when sockets are created if we ever switch to something that supports socket activation (like systemd fx.).

So to be clear, the socket creation semantics becomes a part of the public API.

Revision history for this message
Michal Hruby (mhr3) wrote : Posted in a previous version of this proposal

On it...

Revision history for this message
Michal Hruby (mhr3) wrote :

Ok, it seems I spoke too early that the client part is trivial :) We'll need to discuss some idiosyncrasies in the client part during the rally.

Anyway, I think the server part is good to go, please take a look.

Revision history for this message
Michal Hruby (mhr3) wrote :

Bump, this got lost in the void...

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Am I mising something, or when is this global initialized?

84 +static GHashTable *active_servers = NULL;

review: Needs Information
Revision history for this message
Michal Hruby (mhr3) wrote :

> Am I mising something, or when is this global initialized?
>
> 84 +static GHashTable *active_servers = NULL;

It's in class_init.

344. By Michal Hruby

Couple of gtkdoc fixes

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Great. I am happy. Excellent work!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: