Yes, this was something I was thinking about. But since computing unions of empty sets is very cheap anyway, LGTM in any case (with or without the interdiff).
On Fri, Apr 11, 2014 at 3:23 PM, Klaus Aehlig <[email protected]> wrote: > On Fri, Apr 11, 2014 at 03:01:11PM +0200, Petr Pudlák wrote: > > Can it ever happen that the set of notifications is non-empty? Since we > are > > only adding locks, I'd say no. > > The set of notifications can never be non-empty, as we're only asking for > more and not giving anything back. Does your question implicitly suggest > adding the following interdiff? > > diff --git a/src/Ganeti/Locking/Waiting.hs b/src/Ganeti/Locking/Waiting.hs > index 9a3fa51..1f403c5 100644 > --- a/src/Ganeti/Locking/Waiting.hs > +++ b/src/Ganeti/Locking/Waiting.hs > @@ -335,13 +335,12 @@ opportunisticLockUnion :: (Lock a, Ord b, Ord c) > opportunisticLockUnion owner reqs state = > let locks = L.listLocks owner $ getAllocation state > reqs' = sort $ filter (uncurry (<) . (flip M.lookup locks *** > Just)) reqs > - maybeAllocate (s, (success, notify)) (lock, ownstate) = > - let (s', (result, notify')) = > + maybeAllocate (s, success) (lock, ownstate) = > + let (s', (result, _)) = > updateLocks owner > [(if ownstate == L.OwnShared > then L.requestShared > else L.requestExclusive) lock] > s > - in (s', ( if result == Ok S.empty then lock:success else success > - , notify `S.union` notify')) > - in foldl maybeAllocate (state, ([], S.empty)) reqs' > + in (s', if result == Ok S.empty then lock:success else success) > + in second (flip (,) S.empty) $ foldl maybeAllocate (state, []) reqs' > > > -- > 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 >
