As discussed offline, I'd suggest to add a comment to the docs for the function that the returned set only contains the newly acquired locks and locks that the owner already held aren't mentioned in the result.
On Fri, Apr 11, 2014 at 3:26 PM, Petr Pudlák <[email protected]> wrote: > 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 >> > >
