Make lockConfig call retryable
authorKlaus Aehlig <aehlig@google.com>
Wed, 4 Nov 2015 13:52:16 +0000 (14:52 +0100)
committerKlaus Aehlig <aehlig@google.com>
Wed, 4 Nov 2015 15:57:16 +0000 (16:57 +0100)
Locking the configuration is naturally idempotent. However,
the corresponding WConfD call had a check refusing to lock
the config, if the caller has already locked it, arguing that
this should not happen. That argument misses that we have the
built-in assumption that daemons might be restarted at any time,
including the moment where a request is processed, but the caller
did not get the answer yet. So allow retries, hower logging that
they occurred (as this should only happen rarely).

Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Lisa Velden <velden@google.com>

src/Ganeti/WConfd/Core.hs

index e60108c..6fc6a6d 100644 (file)
@@ -43,7 +43,7 @@ module Ganeti.WConfd.Core where
 import Control.Arrow ((&&&))
 import Control.Concurrent (myThreadId)
 import Control.Lens.Setter (set)
-import Control.Monad (liftM, unless, when)
+import Control.Monad (liftM, unless)
 import qualified Data.Map as M
 import qualified Data.Set as S
 import Language.Haskell.TH (Name)
@@ -54,7 +54,7 @@ import Ganeti.BasicTypes
 import qualified Ganeti.Constants as C
 import qualified Ganeti.JSON as J
 import qualified Ganeti.Locking.Allocation as L
-import Ganeti.Logging (logDebug)
+import Ganeti.Logging (logDebug, logWarning)
 import Ganeti.Locking.Locks ( GanetiLocks(ConfigLock, BGL)
                             , LockLevel(LevelConfig)
                             , lockLevel, LockLevel
@@ -114,16 +114,22 @@ lockConfig
     -> Bool -- ^ set to 'True' if the lock should be shared
     -> WConfdMonad (J.MaybeForJSON ConfigData)
 lockConfig cid shared = do
-  let reqtype = if shared then ReqShared else ReqExclusive
-  -- warn if we already have the lock, this shouldn't happen
+  let (reqtype, owntype) = if shared
+                             then (ReqShared, L.OwnShared)
+                             else (ReqExclusive, L.OwnExclusive)
   la <- readLockAllocation
-  when (L.holdsLock cid ConfigLock L.OwnShared la)
-       . failError $ "Client " ++ show cid ++
-                     " already holds a config lock"
-  waiting <- tryUpdateLocks cid [(ConfigLock, reqtype)]
-  liftM J.MaybeForJSON $ case waiting of
-    []  -> liftM Just CW.readConfig
-    _   -> return Nothing
+  if L.holdsLock cid ConfigLock owntype la
+    then do
+      -- warn if we already have the lock, but continue (with no-op)
+      -- on the locks
+      logWarning $ "Client " ++ show cid ++ " asked to lock the config"
+                   ++ " while owning the lock"
+      liftM (J.MaybeForJSON . Just) CW.readConfig
+    else do
+      waiting <- tryUpdateLocks cid [(ConfigLock, reqtype)]
+      liftM J.MaybeForJSON $ case waiting of
+        []  -> liftM Just CW.readConfig
+        _   -> return Nothing
 
 -- | Release the config lock, if the client currently holds it.
 unlockConfig