Re: [PATCH master 2/2] Write lock state to disk after updates

[Petr Pudlák]

On Tue, Mar 4, 2014 at 3:42 PM, Klaus Aehlig <aeh...@google.com> wrote:

> Make use of asynchronous worker to safe to log status after every
>

s/safe to/save the/


> update. The answer to the query is only returned, once the state
> is saved on disk. Note that replication of the lock status is not
> necessary, as all jobs will be gone if the master node goes away.
>
> Signed-off-by: Klaus Aehlig <aeh...@google.com>
> ---
>  src/Ganeti/WConfd/Monad.hs  | 18 +++++++++++++-----
>  src/Ganeti/WConfd/Server.hs |  1 +
>  2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/src/Ganeti/WConfd/Monad.hs b/src/Ganeti/WConfd/Monad.hs
> index b985f64..817a239 100644
> --- a/src/Ganeti/WConfd/Monad.hs
> +++ b/src/Ganeti/WConfd/Monad.hs
> @@ -80,6 +80,7 @@ data DaemonHandle = DaemonHandle
>    -- daemon should go here;
>    -- all IDs of threads that do asynchronous work should probably also go
> here
>    , dhSaveConfigWorker :: AsyncWorker ()
> +  , dhSaveLocksWorker :: AsyncWorker ()
>    }
>
>  mkDaemonHandle :: FilePath
> @@ -88,11 +89,15 @@ mkDaemonHandle :: FilePath
>                 -> (IO ConfigState -> ResultG (AsyncWorker ()))
>                    -- ^ A function that creates a worker that
> asynchronously
>                    -- saves the configuration to the master file.
> +               -> (IO GanetiLockAllocation -> ResultG (AsyncWorker ()))
> +                  -- ^ A function that creates a worker that
> asynchronously
> +                  -- saves the lock allocation state.
>                 -> ResultG DaemonHandle
> -mkDaemonHandle cpath cstat lstat saveWorkerFn = do
> +mkDaemonHandle cpath cstat lstat saveConfigWorkerFn saveLockWorkerFn = do
>    ds <- newIORef $ DaemonState cstat lstat
> -  saveWorker <- saveWorkerFn $ dsConfigState `liftM` readIORef ds
> -  return $ DaemonHandle ds cpath saveWorker
> +  saveConfigWorker <- saveConfigWorkerFn $ dsConfigState `liftM`
> readIORef ds
> +  saveLockWorker <- saveLockWorkerFn $ dsLockAllocation `liftM` readIORef
> ds
> +  return $ DaemonHandle ds cpath saveConfigWorker saveLockWorker
>
>  -- * The monad and its instances
>
> @@ -173,8 +178,11 @@ modifyLockAllocation f = do
>    dh <- lift . WConfdMonadInt $ ask
>    let mf ds = let (la', r) = f (dsLockAllocation ds)
>                in (ds { dsLockAllocation = la' }, r)
> -  atomicModifyIORef (dhDaemonState dh) mf
> -  -- TODO: Trigger the async. lock saving worker
> +  r <- atomicModifyIORef (dhDaemonState dh) mf
> +  logDebug "Triggering lock state write"
> +  liftBase . triggerAndWait . dhSaveLocksWorker $ dh
> +  logDebug "Lock write finished"
> +  return r
>
>  -- | Atomically modifies the lock allocation state in WConfdMonad, not
>  -- producing any result
> diff --git a/src/Ganeti/WConfd/Server.hs b/src/Ganeti/WConfd/Server.hs
> index 92e3f65..0e99154 100644
> --- a/src/Ganeti/WConfd/Server.hs
> +++ b/src/Ganeti/WConfd/Server.hs
> @@ -85,6 +85,7 @@ prepMain _ _ = do
>                     (mkConfigState cdata)
>                     lock
>                     (saveConfigAsyncTask conf_file cstat)
> +                   (writeLocksAsyncTask lock_file)
>    dh <- withError (strMsg . ("Initialization of the daemon failed" ++) .
> show)
>                    dhOpt
>
> --
> 1.9.0.279.gdc9e3eb
>
>
LGTM, thanks, no need to resend.