[CTDB] progress

[Alexander Bokovoy]

Tridge,

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.
Now I see. "Don't try to read more than written" :-)

>  > > - 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 :)
:-) Still, it confusing.

>  > 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.
I'm trying to explain that each call operation with ltdb is in fact a
triple of (fetch, execute, store): we first fetch data, pass it to a
function callback, collect results from it and change the data in the
ltdb -- either full data or EDR header only.

It means that we would have multiple fetches from tdb to process single
request if those operations would be decoupled.

>  > 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 mean that we have to do ltdb_fetch(ctdb, key) every time we make
decisions on LMASTER/DMASTER status instead of basing some of those
decisions on header of an incoming packet.

>  > This certainly streamlines logic a lot but adds a bit of load on the db.
> Load in what sense?
tdb_fetch/tdb_store multiples per one ctdb operation.

>  >   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
We have an incoming packet's header with destination/source of nodes,
reqid and other fields, in addition to the payload. Why can't we use
those? So far, logic in selection of DMASTER/LMASTER is solely based on
availability of the record in the ltdb.

>  > 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.
My understanding was that if we are DMASTER for the record then we are
also LMASTER for it. Not sure how I made to this though ;-) but you're
right that this code isn't complete.

-- 
/ Alexander