ubik: always hold DB lock for urecovery_ResetState()
authorMarc Dionne <marc.c.dionne@gmail.com>
Sat, 16 Apr 2011 18:19:57 +0000 (14:19 -0400)
committerDerrick Brashear <shadow@dementia.org>
Wed, 27 Apr 2011 00:36:18 +0000 (17:36 -0700)
ubik_ResetState requires callers to hold the DB lock, since it modifies
urecovery_state.  All callers of ubeacon_AmSyncSite outside of the beacon
package hold the DB lock, but calls from the beacon thread do not, and
can't block on getting the DB lock if we're sync site.

Add a beacon internal version of ubeacon_AmSyncSite that skips the
call to ResetState, and have the callers take the DB lock and call
ResetState themselves if needed.  They can take the lock in this case
because we know we're not the sync site.  Refactor the exported
ubeacon_AmSyncSite in terms of this new function.

Change-Id: I88b231010dd52adf6e43a17802e83d12568afc6b
Reviewed-on: http://gerrit.openafs.org/4490
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

src/ubik/beacon.c

index bb78a93..2acc612 100644 (file)
@@ -97,32 +97,14 @@ ubeacon_Debug(struct ubik_debug *aparm)
     aparm->nServers = nServers;
 }
 
-/*!
- * \brief Procedure that determines whether this site has enough current votes to remain sync site.
- *
- * Called from higher-level modules (everything but the vote module).
- *
- * If we're the sync site, check that our guarantees, obtained by the ubeacon_Interact()
- * light-weight process, haven't expired.  We're sync site as long as a majority of the
- * servers in existence have promised us unexpired guarantees.  The variable #ubik_syncSiteUntil
- * contains the time at which the latest of the majority of the sync site guarantees expires
- * (if the variable #ubik_amSyncSite is true)
- * This module also calls up to the recovery module if it thinks that the recovery module
- * may have to pick up a new database (which offucr sif [sic] we lose the sync site votes).
- *
- * \return 1 if local site is the sync site
- * \return 0 if sync site is elsewhere
- */
-int
-ubeacon_AmSyncSite(void)
-{
+static int
+amSyncSite(void) {
     afs_int32 now;
     afs_int32 rcode;
 
     /* special case for fast startup */
-    if (nServers == 1 && !amIClone) {
+    if (nServers == 1 && !amIClone)
        return 1;               /* one guy is always the sync site */
-    }
 
     UBIK_BEACON_LOCK;
     if (beacon_globals.ubik_amSyncSite == 0 || amIClone)
@@ -138,14 +120,41 @@ ubeacon_AmSyncSite(void)
            rcode = 1;          /* otherwise still have the required votes */
        }
     }
-    if (rcode == 0)
-       urecovery_ResetState(); /* force recovery to re-execute */
     UBIK_BEACON_UNLOCK;
     ubik_dprint("beacon: amSyncSite is %d\n", rcode);
     return rcode;
 }
 
 /*!
+ * \brief Procedure that determines whether this site has enough current votes to remain sync site.
+ *
+ * Called from higher-level modules (everything but the vote module).
+ *
+ * If we're the sync site, check that our guarantees, obtained by the ubeacon_Interact()
+ * light-weight process, haven't expired.  We're sync site as long as a majority of the
+ * servers in existence have promised us unexpired guarantees.  The variable #ubik_syncSiteUntil
+ * contains the time at which the latest of the majority of the sync site guarantees expires
+ * (if the variable #ubik_amSyncSite is true)
+ * This module also calls up to the recovery module if it thinks that the recovery module
+ * may have to pick up a new database (which offucr sif [sic] we lose the sync site votes).
+ *
+ * \return 1 if local site is the sync site
+ * \return 0 if sync site is elsewhere
+ */
+int
+ubeacon_AmSyncSite(void)
+{
+    afs_int32 rcode;
+
+    rcode = amSyncSite();
+
+    if (!rcode)
+       urecovery_ResetState();
+
+    return rcode;
+}
+
+/*!
  * \see ubeacon_InitServerListCommon()
  */
 int
@@ -463,16 +472,22 @@ ubeacon_Interact(void *dummy)
            ttid.counter = ubik_dbase->tidCounter + 1;
        tversion.epoch = ubik_dbase->version.epoch;
        tversion.counter = ubik_dbase->version.counter;
+       UBIK_VERSION_UNLOCK;
 
        /* now analyze return codes, counting up our votes */
        yesVotes = 0;           /* count how many to ensure we have quorum */
        oldestYesVote = 0x7fffffff;     /* time quorum expires */
-       syncsite = ubeacon_AmSyncSite();
+       syncsite = amSyncSite();
+       if (!syncsite) {
+           /* Ok to use the DB lock here since we aren't sync site */
+           DBHOLD(ubik_dbase);
+           urecovery_ResetState();
+           DBRELE(ubik_dbase);
+       }
        startTime = FT_ApproxTime();
        /*
         * Don't waste time using mult Rx calls if there are no connections out there
         */
-       UBIK_VERSION_UNLOCK;
        if (i > 0) {
            char hoststr[16];
            multi_Rx(connections, i) {
@@ -557,7 +572,9 @@ ubeacon_Interact(void *dummy)
                ubik_dprint("Ubik: I am no longer the sync site\n");
            beacon_globals.ubik_amSyncSite = 0;
            UBIK_BEACON_UNLOCK;
+           DBHOLD(ubik_dbase);
            urecovery_ResetState();     /* tell recovery we're no longer the sync site */
+           DBRELE(ubik_dbase);
        }
 
     }                          /* while loop */