On Fri, Jul 04, 2014 at 01:17:59PM +0200, 'Petr Pudlak' via ganeti-devel wrote:
> .. as it needs to be stored and reloaded if WConfd is restarted while
> jobs are running.
> 
> Signed-off-by: Petr Pudlak <[email protected]>
> ---
>  src/Ganeti/Path.hs                  |  5 +++++
>  src/Ganeti/WConfd/Core.hs           |  3 +--
>  src/Ganeti/WConfd/DeathDetection.hs |  3 ++-
>  src/Ganeti/WConfd/Monad.hs          | 23 ++++++++++++++++++-----
>  src/Ganeti/WConfd/Persistent.hs     | 14 +++++++++++++-
>  src/Ganeti/WConfd/Server.hs         |  3 +++
>  src/Ganeti/WConfd/TempRes.hs        |  4 ++--
>  7 files changed, 44 insertions(+), 11 deletions(-)

> +++ b/src/Ganeti/WConfd/DeathDetection.hs
> @@ -62,8 +62,9 @@ cleanupLocksTask = forever . runResultT $ do
>          let fpath = ciLockFile owner
>          died <- liftIO (isDead fpath)
>          when died $ do
> -          logInfo $ show owner ++ " died, releasing locks"
> +          logInfo $ show owner ++ " died, releasing locks and reservations"
>            persCleanup persistentLocks owner
> +          persCleanup persistentTempRes owner

Please release locks last. In the future we might get a dependency that
ownership of a lock is a prerequisite for certain reservations whereas
a dependency in the other direction cannot happen. So by switching the
order (first release temporary reseverations, then release locks) we avoid
subtle races in the future.

>            _ <- liftIO . E.try $ removeFile fpath
>                 :: WConfdMonad (Either IOError ())
>            return ()


Rest LGTM

-- 
Klaus Aehlig
Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores

Reply via email to