[PATCH] CTDB: add a new command to detach databases

[Amitay Isaacs]

On Mon, Apr 14, 2014 at 5:40 PM, Michael Adam <obnox at samba.org> wrote:

> > On Tue, Apr 8, 2014 at 6:41 PM, Michael Adam <obnox at samba.org> wrote:
>  > > >
> > > > 1) [p1] I am not certain that it is desirable to forbid
> > > >    detaching when the tunable AllowClientDBAccess == 0.
> > > >
> > > >    I thought a good workflow to have ctdb close its
> > > >    local copies might be this:
> > > >
> > > >    - ctdb setvar AllowClientDBAccess 1
> > > >    - ctdb detach_db ...
> > > >
> > > >    Otherwise closing a db and forbidding attaching again is racy!
> > > >
> > >
> > > AllowClientDBAccess is per node tunable.  So you can only set
> > > AllowClientDBAccess=1 on one of the nodes, do ctdb detach, and then
> disable
> > > access again.  Since you are changing the tunable only on one node, I
> don't
> > > see how there would be a race condition.
>
> Hmm, firstly, from reading the code, I think you need to allow
> client db access on all nodes in order for detach to work
> completety: The check for tunable.allow_client_db_attach is
> before the check if (client_id != 0) ...
>
> And it is racy in two senses:
>
> 1. Between allowing db attach (on one node maybe) and detaching
>    the db, a client may have attached to the db and started
>    working on it. What happens to the client when we just
>    close the db under the hood?...
>
> 2. When db attach is allowed and we do ctdb detatch,
>    a client can attach and thereby re-open the db,
>    before we can forbid db attach again.
>
>    But what we actually want to achieve is to have a means to
>    completely close a db without having to completely
>    shut down ctdb (which would affect other services).
>
> Therefore, I still think it would be reasonable to only
> allow the detach control if client db attach is forbidden
> on all nodes. Or we make it a per-node command and have
> it require or imply AllowClientDBAccess=0 on that node.
>
>
> The use case is this:
>
> We want to upgrade samba and change some characteristic
> of the local DBs to a new mode, e.g. use tdb mutex locking.
> Assume ctdb is already upgraded to code that supports mutexes.
> In order to use mutex locking the dbs need to be closed
> and opened again. But we want to keep ctdb running while
> doing the upgrade.
>
> So the natural flow for me would be:
>
> 1. forbid client db attach
> 2. close the local db copies (by doing ctdb detach)
> 3. upgrade samba
> 4. set config in ctdb and samba to use mutexes
> 5. allow client db attach again
> 6. start samba
>
> With the current code, at least on one node client attach
> would be allowed between steps 2 and 3, hence on that node
> clients may trigger reopening the db before step 4. So
> that node would be stuck with fcntl-locking until the db
> on that node would be detached and re-attached again.
>
>
Thanks for describing the race condition.

I am hoping that people would follow documentation and would not detach
database while the clients are still attached.  :-)

      <title>detach <parameter>DB-LIST</parameter></title>
      <para>
    Detach specified non-persistent database(s) from the cluster. This
    command will disconnect specified database(s) on all nodes in
    the cluster.  This command should only be used when none of the
    specified database(s) are in use.
      </para>

I understand your concern that clients can attach during the brief window
between enabling client access and disabling client access for ctdb detach.

I will post a patch to fix this.

Amitay.