On Tue, Mar 18, 2014 at 11:13 AM, Helga Velroyen <[email protected]> wrote:
> > > > On Mon, Mar 17, 2014 at 1:46 PM, Petr Pudlak <[email protected]> wrote: > >> .. instead of using a pair. >> >> Signed-off-by: Petr Pudlak <[email protected]> >> --- >> src/Ganeti/Locking/Locks.hs | 17 +++++++++++-- >> src/Ganeti/WConfd/Core.hs | 48 >> +++++++++++++++++-------------------- >> src/Ganeti/WConfd/DeathDetection.hs | 4 +++- >> 3 files changed, 40 insertions(+), 29 deletions(-) >> >> diff --git a/src/Ganeti/Locking/Locks.hs b/src/Ganeti/Locking/Locks.hs >> index 23202c7..db81b19 100644 >> --- a/src/Ganeti/Locking/Locks.hs >> +++ b/src/Ganeti/Locking/Locks.hs >> @@ -27,6 +27,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, >> Boston, MA >> >> module Ganeti.Locking.Locks >> ( GanetiLocks(..) >> + , ClientId(..) >> , GanetiLockAllocation >> , loadLockAllocation >> , writeLocksAsyncTask >> @@ -34,7 +35,7 @@ module Ganeti.Locking.Locks >> , lockLevel >> ) where >> >> -import Control.Monad ((>=>)) >> +import Control.Monad ((>=>), liftM) >> import Control.Monad.Base (MonadBase, liftBase) >> import Control.Monad.Error (MonadError, catchError) >> import Data.List (stripPrefix) >> @@ -170,9 +171,21 @@ instance Lock GanetiLocks where >> lockImplications (Network _) = [NetworkLockSet] >> lockImplications _ = [] >> >> +-- | A client is identified as a job id, thread id and path to its >> process >> +-- identifier file >> +data ClientId = ClientId >> + { ciJobId :: JobId >> + , ciLockFile :: FilePath >> + } >> + deriving (Ord, Eq, Show) >> > > The comment mentions the thread id, but I don't see it in the data > structure. Is this intended? > I just saw that it is added in the next patch. If it is not too much trouble, maybe leave them out in the comment in this patch and add it in the next as well. > > >> + >> +instance J.JSON ClientId where >> + showJSON (ClientId jid lf) = J.showJSON (jid, lf) >> + readJSON = liftM (uncurry ClientId) . J.readJSON >> + >> -- | The type of lock Allocations in Ganeti. In Ganeti, the owner of >> -- locks are jobs. >> -type GanetiLockAllocation = LockAllocation GanetiLocks (JobId, FilePath) >> +type GanetiLockAllocation = LockAllocation GanetiLocks ClientId >> >> -- | Load a lock allocation from disk. >> https://memegen.googleplex.com/5618255569879040 >> loadLockAllocation :: FilePath -> ResultG GanetiLockAllocation >> diff --git a/src/Ganeti/WConfd/Core.hs b/src/Ganeti/WConfd/Core.hs >> index 6160bf7..916ffa5 100644 >> --- a/src/Ganeti/WConfd/Core.hs >> +++ b/src/Ganeti/WConfd/Core.hs >> @@ -38,8 +38,7 @@ import Language.Haskell.TH (Name) >> >> import Ganeti.BasicTypes (toErrorStr) >> import qualified Ganeti.Locking.Allocation as L >> -import Ganeti.Locking.Locks (GanetiLocks, lockLevel, LockLevel) >> -import Ganeti.Types (JobId) >> +import Ganeti.Locking.Locks (ClientId, GanetiLocks, lockLevel, LockLevel) >> import Ganeti.WConfd.Language >> import Ganeti.WConfd.Monad >> import Ganeti.WConfd.ConfigWriter >> @@ -55,53 +54,50 @@ echo = return >> -- ** Locking related functions >> >> -- | List the locks of a given owner (i.e., a job-id lockfile pair). >> -listLocks :: JobId -> FilePath -> WConfdMonad [(GanetiLocks, >> L.OwnerState)] >> -listLocks jid fpath = >> - liftM (M.toList . L.listLocks (jid, fpath)) readLockAllocation >> +listLocks :: ClientId -> WConfdMonad [(GanetiLocks, L.OwnerState)] >> +listLocks cid = liftM (M.toList . L.listLocks cid) readLockAllocation >> >> -- | Try to update the locks of a given owner (i.e., a job-id lockfile >> pair). >> -- This function always returns immediately. If the lock update was >> possible, >> -- the empty list is returned; otherwise, the lock status is left >> completly >> -- unchanged, and the return value is the list of jobs which need to >> release >> -- some locks before this request can succeed. >> -tryUpdateLocks :: JobId -> FilePath -> GanetiLockRequest -> WConfdMonad >> [JobId] >> -tryUpdateLocks jid fpath req = >> - liftM (S.toList . S.map fst) >> +tryUpdateLocks :: ClientId -> GanetiLockRequest -> WConfdMonad [ClientId] >> +tryUpdateLocks cid req = >> + liftM S.toList >> . (>>= toErrorStr) >> - $ modifyLockAllocation (L.updateLocks (jid, fpath) >> - (fromGanetiLockRequest req)) >> + $ modifyLockAllocation (L.updateLocks cid (fromGanetiLockRequest req)) >> >> https://memegen.googleplex.com/5618255569879040https://memegen.googleplex.com/5618255569879040 >> -- | Free all locks of a given owner (i.e., a job-id lockfile pair). >> -freeLocks :: JobId -> FilePath -> WConfdMonad () >> -freeLocks jid fpath = >> - modifyLockAllocation_ (`L.freeLocks` (jid, fpath)) >> +freeLocks :: ClientId -> WConfdMonad () >> +freeLocks cid = >> + modifyLockAllocation_ (`L.freeLocks` cid) >> >> -- | Free all locks of a given owner (i.e., a job-id lockfile pair) >> -- of a given level in the Ganeti sense (e.g., "cluster", "node"). >> -freeLocksLevel :: JobId -> FilePath -> LockLevel -> WConfdMonad () >> -freeLocksLevel jid fpath level = >> +freeLocksLevel :: ClientId -> LockLevel -> WConfdMonad () >> +freeLocksLevel cid level = >> modifyLockAllocation_ (L.freeLocksPredicate ((==) level . lockLevel) >> - `flip` (jid, fpath)) >> + `flip` cid) >> >> -- | Downgrade all locks of the given level to shared. >> -downGradeLocksLevel :: JobId -> FilePath -> LockLevel -> WConfdMonad () >> -downGradeLocksLevel jid fpath level = >> - modifyLockAllocation_ $ L.downGradePredicate ((==) level . lockLevel) >> - (jid, fpath) >> +downGradeLocksLevel :: ClientId -> LockLevel -> WConfdMonad () >> +downGradeLocksLevel cid level = >> + modifyLockAllocation_ $ L.downGradePredicate ((==) level . lockLevel) >> cid >> >> -- | Intersect the possesed locks of an owner with a given set. >> -intersectLocks :: JobId -> FilePath -> [GanetiLocks] -> WConfdMonad () >> -intersectLocks jid fpath = >> - modifyLockAllocation_ . L.intersectLocks (jid,fpath) >> +intersectLocks :: ClientId -> [GanetiLocks] -> WConfdMonad () >> +intersectLocks cid = >> + modifyLockAllocation_ . L.intersectLocks cid >> >> -- | Opportunistically allocate locks for a given owner. >> -opportunisticLockUnion :: JobId -> FilePath >> +opportunisticLockUnion :: ClientId >> -> [(GanetiLocks, L.OwnerState)] >> -> WConfdMonad [GanetiLocks] >> -opportunisticLockUnion jid fpath req = >> +opportunisticLockUnion cid req = >> liftM S.toList >> . modifyLockAllocation >> - $ L.opportunisticLockUnion (jid, fpath) req >> + $ L.opportunisticLockUnion cid reqhttps:// >> memegen.googleplex.com/5618255569879040 >> >> -- * The list of all functions exported to RPC. >> >> diff --git a/src/Ganeti/WConfd/DeathDetection.hs >> b/src/Ganeti/WConfd/DeathDetection.hs >> index 532a65f..e5d8a0b 100644 >> --- a/src/Ganeti/WConfd/DeathDetection.hs >> +++ b/src/Ganeti/WConfd/DeathDetection.hs >> @@ -45,6 +45,7 @@ import System.Posix.IO >> import Ganeti.BasicTypes >> import qualified Ganeti.Constants as C >> import qualified Ganeti.Locking.Allocation as L >> +import Ganeti.Locking.Locks (ClientId(..)) >> import Ganeti.Logging.Lifted (logDebug, logInfo) >> import Ganeti.WConfd.Monad >> >> @@ -70,7 +71,8 @@ cleanupLocksTask = forever . runResultT $ do >> logDebug "Death detection timer fired" >> owners <- liftM L.lockOwners readLockAllocation >> logDebug $ "Current lock owners: " ++ show owners >> - let cleanupIfDead owner@(_, fpath) = do >> + let cleanupIfDead owner = do >> + let fpath = ciLockFile owner >> died <- liftIO (isDead fpath) >> when died $ do >> logInfo $ show owner ++ " died, releasing locks" >> -- >> 1.9.0.279.gdc9e3eb >> >> > > Rest LGTM > > -- > -- > Helga Velroyen | Software Engineer | [email protected] | > > Google Germany GmbH > Dienerstr. 12 > 80331 München > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Christine Elizabeth Flores > -- -- Helga Velroyen | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
