[PATCH] ctdb-ipalloc: Split IP allocation into its own build subsystem

[Amitay Isaacs]

On Fri, Dec 11, 2015 at 4:25 PM, Martin Schwenke <martin at meltin.net> wrote:

> On Fri, 11 Dec 2015 02:49:37 +1100, Amitay Isaacs <amitay at gmail.com>
> wrote:
>
> > On Thu, Dec 10, 2015 at 8:58 PM, Martin Schwenke <martin at meltin.net>
> wrote:
> >
> > > Please review and push...
>
> > We still have functions that use ipalloc_state other than the top-level
> > allocation routine ipalloc().  May be it's worthwhile adding a comment on
> > the steps involved in the ctdb_takeover_run().
>
> Yeah, you'd mentioned that previously... sorry, I forgot!  :-(
>
> How about the attached patch on top of the split?  I notice that it
> really exposes some of the current ugliness...
>

The documentation of the overall ctdb_takeover_run() is good. :-)

However, I had in mind the documentation about how to implement new
IP-allocation policy.  So it would be good to have the docs on that since
there is a non-trivial step of failback calculation introduced for LCP2.
That can be an extra patch on top as discussed.

Pushed IP allocation subsystem split and documentation patches.


>
> If we can get the IP configuration and state into a database, as we've
> discussed, then a lot of that ugliness can disappear.  Some of the
> ugliness comes from needing fake some of the setup in the unit test
> code, where we can't use the controls to fetch information from nodes.
> If we're clever then we can change the unit test code to load the
> configuration and state directly from the database, skip the
> controls, and just fake the updates to the database state.  I'll be a
> thing of beauty.
>
> My latest thoughts on that are to have <ip-address>@<pnn> as the key
> and current VNN-like information in the value.  We can then add more
> information to the value to improve and clean up the algorithm(s).
> Anyway, it's one of the things I'm pondering in the background...
>
> > Also, separting of the ip allocation routines now makes some of the
> > previously static functions non-static (e.g. node_ip_coverage).  It might
> > be better to rename all those functions with "ipalloc_" prefix even
> though
> > those functions are only used in recovery daemon.  Those could be extra
> > patches on top of the split-up work.
>
> Yes, definitely.  For the split I didn't want to make any code changes.
> I just want to move things around.  However, I will admit that it isn't
> yet in my queue...  :-)
>
>
After thinking more about this, we really need to define function hooks for
IP allocation.  Every IP allocation algorithm will then implement those
hooks.  That way, you just need to initialize the correct IP allocation
algorithm hooks and everything else is automatic.  Also, it would clean up
the namespace as we would just need a single non-static function per IP
allocation algorithm to initialize all the function hooks.

These changes would also require clearly identifying the steps in the IP
allocation and the data required for each step.

Amitay.