daemon: Mark tunable VerifyRecoveryLock as obsolete

It is pointless having a recovery lock but not sanity checking that it
is working.  Also, the logic that uses this tunable is confusing.  In
some places the recovery lock is released unnecessarily because the
tunable isn't set.

Simplify the logic by assuming that if a recovery lock is specified
then it should be verified.

Update documentation that references this tunable.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
(Imported from commit d110fe231849d76ecb83378c934627dc64b74c72)

diff --git a/doc/ctdb-tunables.7.xml b/doc/ctdb-tunables.7.xml
index 456e856355c4df143961fd72c4345abb5ab1df83..b029fdb75a89ed496b22d8f939c09833ff0ffc95 100644 (file)
--- a/doc/ctdb-tunables.7.xml
+++ b/doc/ctdb-tunables.7.xml
@@ -447,15 +447,6 @@
       </para>
     </refsect2>
 
-    <refsect2>
-      <title>VerifyRecoveryLock</title>
-      <para>Default: 1</para>
-      <para>
-       Should we take a fcntl() lock on the reclock file to verify that we are the
-       sole recovery master node on the cluster or not.
-      </para>
-    </refsect2>
-
     <refsect2>
       <title>VacuumInterval</title>
       <para>Default: 10</para>

diff --git a/doc/ctdb.1.xml b/doc/ctdb.1.xml
index f68056346fcceb94ae704b9c5b4bc272a2855ca4..ccd46d6db22c24a82a53c4dcb67d10f7d45ff208 100644 (file)
--- a/doc/ctdb.1.xml
+++ b/doc/ctdb.1.xml
@@ -682,7 +682,6 @@ RecdFailCount           = 10
 LogLatencyMs            = 0
 RecLockLatencyMs        = 1000
 RecoveryDropAllIPs      = 120
-VerifyRecoveryLock      = 1
 VacuumInterval          = 10
 VacuumMaxRunTime        = 30
 RepackLimit             = 10000
@@ -883,30 +882,51 @@ DB Statistics: locking.tdb
     <refsect2>
       <title>getreclock</title>
       <para>
-       This command is used to show the filename of the reclock file that is used.
+       Show the name of the recovery lock file, if any.
       </para>
 
       <para>
        Example output:
       </para>
       <screen>
-       Reclock file:/gpfs/.ctdb/shared
+       Reclock file:/clusterfs/.ctdb/recovery.lock
       </screen>
 
     </refsect2>
 
     <refsect2>
-      <title>setreclock [filename]</title>
+      <title>
+       setreclock <optional><parameter>FILE</parameter></optional>
+      </title>
+
       <para>
-       This command is used to modify, or clear, the file that is used as the reclock file at runtime. When this command is used, the reclock file checks are disabled. To re-enable the checks the administrator needs to activate the "VerifyRecoveryLock" tunable using "ctdb setvar".
+       FILE specifies the name of the recovery lock file.  If the
+       recovery lock file is changed at run-time then this will cause
+       a recovery, which in turn causes the recovery lock to be
+       retaken.
       </para>
 
       <para>
-       If run with no parameter this will remove the reclock file completely. If run with a parameter the parameter specifies the new filename to use for the recovery lock.
+       If no FILE is specified then a recovery lock file will no
+       longer be used.
       </para>
 
       <para>
-       This command only affects the runtime settings of a ctdb node and will be lost when ctdb is restarted. For persistent changes to the reclock file setting you must edit /etc/sysconfig/ctdb.
+       This command only affects the run-time setting of a single
+       CTDB node.  This setting <emphasis>must</emphasis> be changed
+       on all nodes simultaneously by specifying <option>-n
+       all</option> (or similar).  For information about configuring
+       the recovery lock file please see the
+       <citetitle>CTDB_RECOVERY_LOCK</citetitle> entry in
+       <citerefentry><refentrytitle>ctdbd.conf</refentrytitle>
+       <manvolnum>5</manvolnum></citerefentry> and the
+       <citetitle>--reclock</citetitle> entry in
+       <citerefentry><refentrytitle>ctdbd</refentrytitle>
+       <manvolnum>1</manvolnum></citerefentry>.  For information
+       about the recovery lock please see the <citetitle>RECOVERY
+       LOCK</citetitle> section in
+       <citerefentry><refentrytitle>ctdb</refentrytitle>
+       <manvolnum>7</manvolnum></citerefentry>.
       </para>
     </refsect2>
 

diff --git a/server/ctdb_recover.c b/server/ctdb_recover.c
index 393b72fd500672c1678b2ef8cd026e5ce9b530ee..1ac1ea0f081cdccb5119b5918351a0d325395520 100644 (file)
--- a/server/ctdb_recover.c
+++ b/server/ctdb_recover.c
@@ -639,8 +639,8 @@ int32_t ctdb_control_set_recmode(struct ctdb_context *ctdb,
                ctdb_process_deferred_attach(ctdb);
        }
 
-       if (ctdb->tunable.verify_recovery_lock == 0) {
-               /* dont need to verify the reclock file */
+       if (ctdb->recovery_lock_file == NULL) {
+               /* Not using recovery lock file */
                ctdb->recovery_mode = recmode;
                return 0;
        }

diff --git a/server/ctdb_recoverd.c b/server/ctdb_recoverd.c
index 39e833cf42aa92c3190a672792bdac608cab7570..6c61756c70fdd94dfda163c0c76f6f9325a9f6dc 100644 (file)
--- a/server/ctdb_recoverd.c
+++ b/server/ctdb_recoverd.c
@@ -1808,7 +1808,7 @@ static int do_recovery(struct ctdb_recoverd *rec,
                return -1;
        }
 
-        if (ctdb->tunable.verify_recovery_lock != 0) {
+        if (ctdb->recovery_lock_file != NULL) {
                DEBUG(DEBUG_ERR,("Taking out recovery lock from recovery daemon\n"));
                start_time = timeval_current();
                if (!ctdb_recovery_lock(ctdb, true)) {
@@ -2725,8 +2725,8 @@ static void election_handler(struct ctdb_context *ctdb, uint64_t srvid,
        talloc_free(rec->send_election_te);
        rec->send_election_te = NULL;
 
-        if (ctdb->tunable.verify_recovery_lock != 0) {
-               /* release the recmaster lock */
+        if (ctdb->recovery_lock_file != NULL) {
+               /* Release the recovery lock file */
                if (em->pnn != ctdb->pnn &&
                    ctdb->recovery_lock_fd != -1) {
                        DEBUG(DEBUG_NOTICE, ("Release the recovery lock\n"));
@@ -3524,7 +3524,6 @@ static int update_recovery_lock_file(struct ctdb_context *ctdb)
                                ctdb->recovery_lock_fd = -1;
                        }
                }
-               ctdb->tunable.verify_recovery_lock = 0;
                talloc_free(tmp_ctx);
                return 0;
        }
@@ -3547,7 +3546,6 @@ static int update_recovery_lock_file(struct ctdb_context *ctdb)
 
        talloc_free(ctdb->recovery_lock_file);
        ctdb->recovery_lock_file = talloc_strdup(ctdb, reclockfile);
-       ctdb->tunable.verify_recovery_lock = 0;
        if (ctdb->recovery_lock_fd != -1) {
                close(ctdb->recovery_lock_fd);
                ctdb->recovery_lock_fd = -1;
@@ -3617,7 +3615,7 @@ static void main_loop(struct ctdb_context *ctdb, struct ctdb_recoverd *rec,
        /* Make sure that if recovery lock verification becomes disabled when
           we close the file
        */
-        if (ctdb->tunable.verify_recovery_lock == 0) {
+        if (ctdb->recovery_lock_file == NULL) {
                if (ctdb->recovery_lock_fd != -1) {
                        close(ctdb->recovery_lock_fd);
                        ctdb->recovery_lock_fd = -1;
@@ -3882,7 +3880,7 @@ static void main_loop(struct ctdb_context *ctdb, struct ctdb_recoverd *rec,
        }
 
 
-        if (ctdb->tunable.verify_recovery_lock != 0) {
+        if (ctdb->recovery_lock_file != NULL) {
                /* we should have the reclock - check its not stale */
                ret = check_recovery_lock(ctdb);
                if (ret != 0) {

diff --git a/server/ctdb_server.c b/server/ctdb_server.c
index c45f4cb5ffdd3e26ea6a7dd7cd655f1e9882c074..54758957cd4157236f7687311f3d57555e544287 100644 (file)
--- a/server/ctdb_server.c
+++ b/server/ctdb_server.c
@@ -67,7 +67,6 @@ int ctdb_set_recovery_lock_file(struct ctdb_context *ctdb, const char *file)
 
        if (file == NULL) {
                DEBUG(DEBUG_ALERT,("Recovery lock file set to \"\". Disabling recovery lock checking\n"));
-               ctdb->tunable.verify_recovery_lock = 0;
                return 0;
        }
 

diff --git a/server/ctdb_tunables.c b/server/ctdb_tunables.c
index 069958653f108bdf414f9c55a7e3b8c595bbfb58..f5713f8d416925481e26e4f5333ad9936dc24522 100644 (file)
--- a/server/ctdb_tunables.c
+++ b/server/ctdb_tunables.c
@@ -57,7 +57,7 @@ static const struct {
        { "LogLatencyMs",         0,  offsetof(struct ctdb_tunable, log_latency_ms), false },
        { "RecLockLatencyMs",  1000,  offsetof(struct ctdb_tunable, reclock_latency_ms), false },
        { "RecoveryDropAllIPs", 120,  offsetof(struct ctdb_tunable, recovery_drop_all_ips), false },
-       { "VerifyRecoveryLock",   1,  offsetof(struct ctdb_tunable, verify_recovery_lock), false },
+       { "VerifyRecoveryLock",   1,  offsetof(struct ctdb_tunable, verify_recovery_lock), true },
        { "VacuumInterval",   10,  offsetof(struct ctdb_tunable, vacuum_interval), false },
        { "VacuumMaxRunTime",     120,  offsetof(struct ctdb_tunable, vacuum_max_run_time), false },
        { "RepackLimit",      10000,  offsetof(struct ctdb_tunable, repack_limit), false },
@@ -145,7 +145,7 @@ int32_t ctdb_control_get_tunable(struct ctdb_context *ctdb, TDB_DATA indata,
  */
 int32_t ctdb_control_set_tunable(struct ctdb_context *ctdb, TDB_DATA indata)
 {
-       struct ctdb_control_set_tunable *t = 
+       struct ctdb_control_set_tunable *t =
                (struct ctdb_control_set_tunable *)indata.dptr;
        char *name;
        int i;
@@ -163,15 +163,8 @@ int32_t ctdb_control_set_tunable(struct ctdb_context *ctdb, TDB_DATA indata)
                if (strcasecmp(name, tunable_map[i].name) == 0) break;
        }
 
-       if (!strcmp(name, "VerifyRecoveryLock") && t->value != 0
-       && ctdb->recovery_lock_file == NULL) {
-               DEBUG(DEBUG_ERR,("Can not activate tunable \"VerifyRecoveryLock\" since there is no recovery lock file set.\n"));
-               talloc_free(name);
-               return -1;
-       }
-
        talloc_free(name);
-       
+
        if (i == ARRAY_SIZE(tunable_map)) {
                return -1;
        }