[CTDB] progress

[tridge at samba.org]

Alexander,

 > It is the database for LMASTER information about DMASTERs. If LMASTER
 > isn't DMASTER for the record and the record wasn't in its tdb with any
 > info how could it know what to respond to a requester?

Easy :-)

If the LMASTER doesn't have a record for the given key in its ltdb
then LMASTER must also be DMASTER. This is certain as the LMASTER is
always involved in any transfer of the DMASTER role, so it always gets
a chance to update its ltdb record. If LMASTER != DMASTER then the
data in the ltdb record on the LMASTER may be out of date, but the
header which says who the DMASTER is will be correct.

This also makes things easier at initialisation, as it means that
with an empty ltdb all records start out with DMASTER == LMASTER.

 > > - why the additional _struct suffixes on the various structures?
 > I find it inconvenient to have same function and structure names. It is
 > error-prone from my point of view, you might accidentally to use
 > function name (as pointer) instead of struct name. Not you specifically
 > :-) but it is definitely misleading. So this ambiguity isn't good.

There is no ambiguity. The 'struct ' prefix ensures that. If you tried
to 'call' a structure as a pointer then the C compiler will tell you
that you screwed up.

Please leave off the _struct suffix :)

 > > The use of a [0] buffer is only needed for variable length data, and 
 > > this reply is not variable length.
 > I tried to follow common pattern, but you're right here.

yep, the pattern is that the 0 length arrays is used for variable
length data, not all data.

 > > - for the record header changes, I'd suggest not putting that logic
 > > in ctdb_call.c, instead it would be cleaner to do it in ctdb_ltdb.c, 
 > > and replace the tdb_fetch() call in ctdb_call() with a 
 > > ctdb_ltdb_fetch() call, which will handle removing the extra header 
 > > (and returning it as an extra parameter perhaps). Otherwise the logic
 > > in ctdb_call.c just gets too convoluted.
 > The problem there is that we then need to pass-through header as each
 > fetch is actually fetch->execute->store sequence: you fetch the record,
 > pass it to the called function, analyze the result and either update
 > header and data or only update header. So essentially, you can't do
 > fetch-only, you would need to make ctdb_ltdb_update() as well and copy a
 > portion of data.

The IO pattern isn't what I'm asking about. I'm asking that we put all
knowledge of the way that records and the additional header data into
the ctdb_ltdb.c module, rather than putting everything in one C
file. I am sure we can do that without breaking the logic.

 > So you are leaning towards 'single db, fetch to know what is there'?

We will have one tdb per ctdb database. So in the little tests we have
now, there will be one tdb per node. I don't know what you mean by
'fetch to know what is there' ?

I definately don't want a separate tdb for the headers.

See the 'Extended Data Records' section on the wiki for a description.

 > This certainly streamlines logic a lot but adds a bit of load on the db.

Load in what sense?

 >   I'm not sure that we can't really use header's info to make such
 > decisions...

I don't understand what you mean by that, sorry

 > Why? We've got a call for a record we don't own. This means we are
 > LMASTER for this record or there was network split. Thus we check for
 > LMASTER.

The logic in your code is this:

	/* If we are LMASTER */
	if (ctdb->vnn == lmaster) {
		/* Find out DMASTER for the record */
		dmaster = ctdb_ltdb_dmaster(&key);
		/* ..... etc ..... */
	} else {
		/* Otherwise, we are not LMASTER for this key, */
		real_op = CTDB_REPLY_REDIRECT;
		dest = lmaster;
	}

so, if we are the DMASTER for this record then it will do the else{}
part of the logic, and thus do a REDIRECT. That is the opposite of
what we want. We reply to a request when we own the record, which is
when we are the DMASTER.

Cheers, Tridge