ubik: avoid early DISK_Begin calls we know will fail

Currently, we can start a write transaction on a site immediately after
it is elected as the sync site. However, after commit d47beca1,
SDISK_Begin on remote sites will fail right after an election occurs
(since lastYesState is not set, and so urecovery_AllBetter will fail).
And after commit fac0b742, this error is always noticed and propagated
back to the application.

As a result, when we try to write immediately after a sync site is
elected, the transaction will fail with UNOQUORUM, the remote sites will
be marked as down, and we may lose quorum and require another election
to be performed. This can easily happen repeatedly for a site that
frequently tries to make changes to a ubik database.

To avoid marking other sites down and going through another election
process, do not allow write transactions until we know that lastYesState
is set on the remote sites. We do this by waiting until the next wave of
beacons are sent, which tell the remote sites that we are the sync site.
In other words, only allow write transactions after the sync site knows
that the remote sites also know that the sync site has been elected.

With this commit, a write transaction immediately after an election
will still fail with UNOQUORUM, but we avoid triggering an error on the
remote sites, and avoid losing quorum in this situation.

Change-Id: I9e1a76b4022e6d734af1165d94c12e90af04974d
Reviewed-on: https://gerrit.openafs.org/12592
Reviewed-by: Andrew Deason <adeason@dson.org>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/ubik/beacon.c b/src/ubik/beacon.c
index a3a48d6..bf32432 100644 (file)
--- a/src/ubik/beacon.c
+++ b/src/ubik/beacon.c
@@ -119,6 +119,7 @@ amSyncSite(void)
            if (beacon_globals.ubik_amSyncSite)
                ubik_dprint("Ubik: I am no longer the sync site\n");
            beacon_globals.ubik_amSyncSite = 0;
+           beacon_globals.ubik_syncSiteAdvertised = 0;
            rcode = 0;
        } else {
            rcode = 1;          /* otherwise still have the required votes */
@@ -159,6 +160,32 @@ ubeacon_AmSyncSite(void)
 }
 
 /*!
+ * \brief Determine whether at least quorum are aware we have a sync-site.
+ *
+ * Called from higher-level modules.
+ *
+ * There is a gap between the time when a new sync-site is elected and the time
+ * when the remotes are aware of that. Therefore, any write transaction between
+ * this gap will fail. This will force a new re-election which might be time
+ * consuming. This procedure determines whether the remotes (quorum) are aware
+ * we have a sync-site.
+ *
+ * \return 1 if remotes are aware we have a sync-site
+ * \return 0 if remotes are not aware we have a sync-site
+ */
+int
+ubeacon_SyncSiteAdvertised(void)
+{
+    afs_int32 rcode;
+
+    UBIK_BEACON_LOCK;
+    rcode = beacon_globals.ubik_syncSiteAdvertised;
+    UBIK_BEACON_UNLOCK;
+
+    return rcode;
+}
+
+/*!
  * \see ubeacon_InitServerListCommon()
  */
 int
@@ -371,6 +398,7 @@ ubeacon_InitServerListCommon(afs_uint32 ame, struct afsconf_cell *info,
        if (nServers == 1 && !amIClone) {
            beacon_globals.ubik_amSyncSite = 1; /* let's start as sync site */
            beacon_globals.syncSiteUntil = 0x7fffffff;  /* and be it quite a while */
+           beacon_globals.ubik_syncSiteAdvertised = 1;
        }
     } else {
        if (nServers == 1)      /* special case 1 server */
@@ -382,6 +410,7 @@ ubeacon_InitServerListCommon(afs_uint32 ame, struct afsconf_cell *info,
            ubik_dprint("Ubik: I am the sync site - 1 server\n");
        beacon_globals.ubik_amSyncSite = 1;
        beacon_globals.syncSiteUntil = 0x7fffffff;      /* quite a while */
+       beacon_globals.ubik_syncSiteAdvertised = 1;
     }
     return 0;
 }
@@ -579,6 +608,11 @@ ubeacon_Interact(void *dummy)
            UBIK_BEACON_LOCK;
            if (!beacon_globals.ubik_amSyncSite)
                ubik_dprint("Ubik: I am the sync site\n");
+           else {
+               /* at this point, we have the guarantee that at least quorum
+                * received a beacon packet informing we have a sync-site. */
+               beacon_globals.ubik_syncSiteAdvertised = 1;
+           }
            beacon_globals.ubik_amSyncSite = 1;
            beacon_globals.syncSiteUntil = oldestYesVote + SMALLTIME;
 #ifndef AFS_PTHREAD_ENV
@@ -592,6 +626,7 @@ ubeacon_Interact(void *dummy)
            if (beacon_globals.ubik_amSyncSite)
                ubik_dprint("Ubik: I am no longer the sync site\n");
            beacon_globals.ubik_amSyncSite = 0;
+           beacon_globals.ubik_syncSiteAdvertised = 0;
            UBIK_BEACON_UNLOCK;
            DBHOLD(ubik_dbase);
            urecovery_ResetState();     /* tell recovery we're no longer the sync site */

diff --git a/src/ubik/ubik.c b/src/ubik/ubik.c
index 229c596..7eda47a 100644 (file)
--- a/src/ubik/ubik.c
+++ b/src/ubik/ubik.c
@@ -643,6 +643,11 @@ BeginTrans(struct ubik_dbase *dbase, afs_int32 transMode,
            DBRELE(dbase);
            return UNOTSYNC;
        }
+       if (!ubeacon_SyncSiteAdvertised()) {
+           /* i am the sync-site but the remotes are not aware yet */
+           DBRELE(dbase);
+           return UNOQUORUM;
+       }
     }
 
     /* create the transaction */

diff --git a/src/ubik/ubik.p.h b/src/ubik/ubik.p.h
index 8e40264..2c1a318 100644 (file)
--- a/src/ubik/ubik.p.h
+++ b/src/ubik/ubik.p.h
@@ -362,6 +362,7 @@ struct beacon_data {
 #endif
     int ubik_amSyncSite;               /*!< flag telling if I'm sync site */
     afs_int32 syncSiteUntil;           /*!< valid only if amSyncSite */
+    int ubik_syncSiteAdvertised;       /*!< flag telling if remotes are aware we have quorum */
 };
 
 #define UBIK_BEACON_LOCK opr_mutex_enter(&beacon_globals.beacon_lock)
@@ -502,6 +503,7 @@ extern void ubeacon_InitSecurityClass(void);
 extern void ubeacon_ReinitServer(struct ubik_server *ts);
 extern void ubeacon_Debug(struct ubik_debug *aparm);
 extern int ubeacon_AmSyncSite(void);
+extern int ubeacon_SyncSiteAdvertised(void);
 extern int ubeacon_InitServerListByInfo(afs_uint32 ame,
                                        struct afsconf_cell *info,
                                        char clones[]);