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

[Michael Adam]

Hi Amitay,

generally, the patches look very good, just a few comments
from a first review, without having really tested the new
functionaltiy yet:

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!

2) [p1] In ctdb_control_db_detach(), there are two
   lines with DEBUG messages slighlty exceeding 80 characters,
   also the function header is too long.

3) [p1] In ctdb_control_db_detach(), there is an error in a comment:
   "Do the actual detach only if the control comes other daemon."
   does not make complete sense. ("from" missing?)

4) [p3] header line of control_detach() is slightly too long,
   as is a debug statement in that function.

5) [p4] Maybe
   "disconnect specified database(s) on all nodes in the cluster"
   instead of                           ^^^
   "disconnect specified database(s) on the nodes in the cluster"
   for clarity?

Cheers - Michael

On 2014-04-08 at 13:04 +1000, Amitay Isaacs wrote:
> Hi,
> 
> Here is a patchset to support detaching from already connected
> (non-persistent) database.
> 
> Also includes patches for document update.
> 
> Please review and push if ok.
> 
> Amitay.


-------------- 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/20140408/1fd4b44e/attachment.pgp>