Merge lp://qastaging/~fboucault/dee/schema_passing into lp://qastaging/dee

The protocol has been amended to carry the schema as an array of strings ('as') as second argument, after the swarm name. This is used by DeeSharedModel of a swarm for which the schema was not manually set.

The unit tests have been modified so that they do not set a schema when we know it is not necessary.
The examples have been updated to take advantage of that as well.

There is one case that is currently broken and for which no unit test is written. If a DeeSharedModel is created and no schema is set, it becomes the leader and will receive clone requests that will fail. It is easy to reproduce with the master/slave example doing the following:

$ ./slave-model
$ ./master-model

Very nice work Florian! Just two nitpicks and I'll merge it:

126 + g_warning ("Received transaction before the model schema has been set \
127 + and none received from leader");
128 + g_variant_unref (transaction);
129 + return;
130 + }

I guess this will happen if you receive a Commit before the Clone call has returned from the leader, right? If you agree I think the g_warning() message should reflect that. And also could you add a FIXME - because in theory we could queue up the transaction and try to apply it once the Clone has arrived (but I don't think we need to fix it unless it proves to be a problem in practice).

141 + * The column schema will be set in one of two ways: firstly you may set it
142 + * manually with dee_model_set_schema() or secondly it will be set once
143 + * the first rows are exchanged with a peer model.
144 + * In the second case the column schema will constructed explicitly from
145 + * the Clone metadata.

I don't think we should mention that DBus interface (Clone) in the API docs. That's an implementation detail as far as I am concerned. I also think we should warn about the slave/master race here?

About the slave/master race you mention, I think we can go a long way here by adding documentation for it. A bit down the road I have been thinking about adding a mode to DeePeer where it sets the replacement flag in the DBus request for the swarm name. That would make any later swarm participants steal sram leadership form it when they join. That would be very useful for a process that basically always wants to not be swarm leader.

Thanks for the review Mikkel!

> Very nice work Florian! Just two nitpicks and I'll merge it:
>
> 126 + g_warning ("Received transaction before the model schema has been
> set \
> 127 + and none received from leader");
> 128 + g_variant_unref (transaction);
> 129 + return;
> 130 + }
>
> I guess this will happen if you receive a Commit before the Clone call has
> returned from the leader, right? If you agree I think the g_warning() message
> should reflect that. And also could you add a FIXME - because in theory we
> could queue up the transaction and try to apply it once the Clone has arrived
> (but I don't think we need to fix it unless it proves to be a problem in
> practice).

Actually both Commit and Clone now pass the schema over the wire. So I think it would only happen if the leader does not have a schema set and a Clone request is made on him. And in practice that case never happens because the Clone request is never answered. This is exactly the case highlighted in the first comment: if a DeeSharedModel is created and no schema is set, it becomes the leader and will receive clone requests that will fail.

>
> 141 + * The column schema will be set in one of two ways: firstly you may
> set it
> 142 + * manually with dee_model_set_schema() or secondly it will be set
> once
> 143 + * the first rows are exchanged with a peer model.
> 144 + * In the second case the column schema will constructed explicitly
> from
> 145 + * the Clone metadata.
>
> I don't think we should mention that DBus interface (Clone) in the API docs.
> That's an implementation detail as far as I am concerned. I also think we
> should warn about the slave/master race here?

I removed the reference to the D-Bus interface and added a little bit of explanation on creating DeeSharedModels without schema set.

>
> About the slave/master race you mention, I think we can go a long way here by
> adding documentation for it. A bit down the road I have been thinking about
> adding a mode to DeePeer where it sets the replacement flag in the DBus
> request for the swarm name. That would make any later swarm participants steal
> sram leadership form it when they join. That would be very useful for a
> process that basically always wants to not be swarm leader.

That could be a very useful flag indeed.
I think to fix the race what should automatically happen though is either:
a) a DeeSharedModel without schema should not advertise itself at all over D-Bus, thus not be the leader
or
b) if no peer has a schema set in the swarm, the first peer to get a schema becomes the leader

Note that the new revision did not appear among the comments as it usually does because the merge request was set as 'Work in progress' and not as 'Needs review'.

I merged it with a small fix. The purpose of the tests/model-helper-uninitialized.c test is to start a leader-model *without* a schema, and then start a slave that defines the schema, and then assert that the leader gets the schema in the first transaction.

Thanks a lot Mikkel.
Well spotted, I obviously had not grasped properly the purpose of tests/model-helper-uninitialized.c