[CTDB] progress

[tridge at samba.org]

Alexander,

 > we are close to implement CTDB protocol and EDRs in
 > http://samba.org/~ab/ctdb. Please see revision 26 of common/ctdb_call.c
 > include/ctdb_private.h, there are number of helpers which aren't
 > implemented yet but other than that we are almost done with basic logic
 > of DMASTER/LMASTER.

sorry for the delay in reviewing this.

Some questions :-)

- What is the lmasterdb tdb for? (in ctdb_private.h)

- why the additional _struct suffixes on the various structures?

- In struct ctdb_reply_redirect_struct, you have this:

    struct ctdb_reply_redirect_struct {
        struct ctdb_req_header hdr;
        uint32_t datalen;
        uint8_t  data[0]; /* uint32_t for dmaster id */
    };

  why the fancy data buffer? It would be much simpler to have this:

    struct ctdb_reply_redirect {
        struct ctdb_req_header hdr;
        uint32_t dmaster;
    };

  The use of a [0] buffer is only needed for variable length data, and
  this reply is not variable length.

- 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.

- This comment is a bit misleading:
        /* 
	           if c->hdr.destnode == ctdb->vnn then we are possible DMASTER
	           (if nothing changed between issuing the call and receiving it here)
           */

  A test for (c->hdr.destnode == ctdb->vnn) is really only useful for
  checking if the vnn logic has screwed up. When these are not equal
  it means the mapping of vnn to physical node is not correct, which
  probably happened because either source or destination nodes didn't
  participate in a recovery.

  The way to check if we are the DMASTER is to try to load the record
  from the ltdb. If we are the DMASTER then either the record header
  will list our vnn as the DMASTER, or the record is not there and we
  are the LMASTER.

- similarly for this comment:
        /* TODO: 
          If we are not the recepient (hdr->dstnode != ctdb->vnn):
          1. redirect it to the recepient
          2. Note the requester to forward response later
         */
  if we get a packet for which we are not the recipient then either
  sender or receiver didn't participate properly in a
  recovery. We need to start a new recovery and vnn assignment.

- The logic in ctdb_request_call() is not right. It must check if we
  are the DMASTER, not if we are the LMASTER. In this function, being
  the LMASTER is only important if the ltdb record is empty (which
  means we are by default the DMASTER too)

Cheers, Tridge