[PATCH] CTDB: add a new command to detach databases
obnox at samba.org
Tue Apr 8 02:41:50 MDT 2014
generally, the patches look very good, just a few comments
from a first review, without having really tested the new
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"
Cheers - Michael
On 2014-04-08 at 13:04 +1000, Amitay Isaacs wrote:
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 215 bytes
Desc: Digital signature
More information about the samba-technical