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

[Michael Adam]

Hi Martin,

On 2014-04-14 at 11:15 +1000, Martin Schwenke wrote:
> On Wed, 9 Apr 2014 17:57:12 +1000, Amitay Isaacs <amitay at gmail.com>
> wrote:
> >
> > I have included your suggestions.  Please find attached the modified
> > patches.
> 
> Given the absence of further comments, I've marked these as reviewed by
> me and pushed them to autobuild.
> 
> Michael, I wasn't sure if I should mark any of the patches as reviewed
> by you, so I decided to be conservative and not speak for you...

That's fine.
Please apologize that I didn't manage to comment again yet.

But please see my comments below regarding racyness:

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

Cheers - Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140414/957b474e/attachment.pgp>