I'm little bit confused with the 'a' and 'b' type variables, so I'd perhaps suggest to use 'l' for locks and 'o' for owners (or something like that) so that the types are easier to read. Or perhaps add a comment to the top like "'a' always denotes a lock and 'b' always denotes an owner throughout this module".
On Sat, Feb 15, 2014 at 12:40 AM, Klaus Aehlig <[email protected]> wrote: > To allow for jobs as processes, a central daemon (wconfd) will > handle allocation and release of locks. Add an appropriate data > structure to describe the current status of the locks. > > Signed-off-by: Klaus Aehlig <[email protected]> > --- > Makefile.am | 2 ++ > src/Ganeti/Locking/Allocation.hs | 71 > ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 73 insertions(+) > create mode 100644 src/Ganeti/Locking/Allocation.hs > > diff --git a/Makefile.am b/Makefile.am > index 0466569..00b2e52 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -126,6 +126,7 @@ HS_DIRS = \ > src/Ganeti/HTools/Program \ > src/Ganeti/Hypervisor \ > src/Ganeti/Hypervisor/Xen \ > + src/Ganeti/Locking \ > src/Ganeti/Monitoring \ > src/Ganeti/Query \ > src/Ganeti/Storage \ > @@ -730,6 +731,7 @@ HS_LIB_SRCS = \ > src/Ganeti/JSON.hs \ > src/Ganeti/Jobs.hs \ > src/Ganeti/Kvmd.hs \ > + src/Ganeti/Locking/Allocation.hs \ > src/Ganeti/Logging.hs \ > src/Ganeti/Luxi.hs \ > src/Ganeti/Monitoring/Server.hs \ > diff --git a/src/Ganeti/Locking/Allocation.hs > b/src/Ganeti/Locking/Allocation.hs > new file mode 100644 > index 0000000..4305aa6 > --- /dev/null > +++ b/src/Ganeti/Locking/Allocation.hs > @@ -0,0 +1,71 @@ > +{-| Implementation of lock allocation. > + > +-} > + > +{- > + > +Copyright (C) 2014 Google Inc. > + > +This program is free software; you can redistribute it and/or modify > +it under the terms of the GNU General Public License as published by > +the Free Software Foundation; either version 2 of the License, or > +(at your option) any later version. > + > +This program is distributed in the hope that it will be useful, but > +WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +General Public License for more details. > + > +You should have received a copy of the GNU General Public License > +along with this program; if not, write to the Free Software > +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > +02110-1301, USA. > + > +-} > + > +module Ganeti.Locking.Allocation > + ( LockAllocation > + , emptyAllocation > + , OwnerState(..) > + , listLocks > + ) where > + > +import qualified Data.Map as M > +import Data.Maybe (fromMaybe) > +import qualified Data.Set as S > + > +-- | The state of a lock that is taken. > +data AllocationState a = Exclusive a | Shared (S.Set a) deriving (Eq, > Show) > + > +-- | Data type describing the way a lock can be owned. > +data OwnerState = OwnShared | OwnExclusive deriving (Ord, Eq, Show) > + > +{-| Representation of a Lock allocation > + > +To keep queries for locks efficient, we keep two > +associations, with the invariant that they fit > +together: the association from locks to their > +allocation state, and the association from an > +owner to the set of locks owned. As we do not > +export the constructor, the problem of keeping > +this invariant reduces to only exporting functions > +that keep the invariant. > + > +-} > + > +data LockAllocation a b = > + LockAllocation { laLocks :: M.Map a (AllocationState b) > + , laOwned :: M.Map b (S.Set (a, OwnerState)) > Just an idea for discussion: Would it be possible to use laOwned :: M.Map b (M.Map a OwnerState) instead? An owner can always hold just one type (excl./shared) for a given lock, and it seems to me that it could simplify the updates later in the patch series, because there'd be no need to manually delete other types of locks from the set when the type changes (or gets deleted). > + } > + deriving (Eq, Show) > + > +-- | A state with all locks being free. > +emptyAllocation :: (Ord a, Ord b) => LockAllocation a b > +emptyAllocation = > + LockAllocation { laLocks = M.fromList [] > + , laOwned = M.fromList [] > Just a minor idea: instead of 'M.fromList []' we could use just 'M.empty'. > + } > + > +-- | Obtain the set of locks held by a given owner. > +listLocks :: Ord b => b -> LockAllocation a b -> S.Set (a, OwnerState) > +listLocks owner = fromMaybe S.empty . M.lookup owner . laOwned > -- > 1.9.0.rc1.175.g0b1dcb5 > >
