From 44801d157e6fd0933f37f0fc6451e37bdca285f3 Mon Sep 17 00:00:00 2001 From: Marc Dionne Date: Sat, 16 Apr 2011 14:19:57 -0400 Subject: [PATCH] ubik: always hold DB lock for urecovery_ResetState() 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 Tested-by: BuildBot Reviewed-by: Derrick Brashear --- src/ubik/beacon.c | 67 +++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/src/ubik/beacon.c b/src/ubik/beacon.c index bb78a9355..2acc6129e 100644 --- a/src/ubik/beacon.c +++ b/src/ubik/beacon.c @@ -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,13 +120,40 @@ 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() */ @@ -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 */ -- 2.39.5