Skip to content

Commit 5780ab4

Browse files
committed
prototype: check treeInvariant without exceptions
This makes it easier to propagate the error information, for example to report it as a quickcheck property failure with relevant context.
1 parent 0bcbdbc commit 5780ab4

File tree

3 files changed

+67
-34
lines changed

3 files changed

+67
-34
lines changed

lsm-tree.cabal

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,7 @@ library prototypes
722722
, tasty
723723
, tasty-hunit
724724
, tasty-quickcheck
725+
, transformers
725726

726727
ghc-options:
727728
-Wno-incomplete-uni-patterns -Wno-partial-fields

prototypes/ScheduledMerges.hs

Lines changed: 57 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,13 @@ module ScheduledMerges (
6565
Run,
6666
supplyCreditsMergingTree,
6767
remainingDebtMergingTree,
68-
treeInvariant,
6968
mergek,
7069
mergeBatchSize,
70+
71+
-- * Invariants
72+
Invariant,
73+
evalInvariant,
74+
treeInvariant,
7175
) where
7276

7377
import Prelude hiding (lookup)
@@ -82,6 +86,7 @@ import Data.STRef
8286
import qualified Control.Exception as Exc (assert)
8387
import Control.Monad (foldM, forM, when)
8488
import Control.Monad.ST
89+
import qualified Control.Monad.Trans.Except as E
8590
import Control.Tracer (Tracer, contramap, traceWith)
8691
import GHC.Stack (HasCallStack, callStack)
8792

@@ -305,7 +310,7 @@ invariant (LSMContent _ levels ul) = do
305310
levelsInvariant 1 levels
306311
case ul of
307312
NoUnion -> return ()
308-
Union tree _ -> treeInvariant tree
313+
Union tree _ -> expectInvariant (treeInvariant tree)
309314
where
310315
levelsInvariant :: Int -> Levels s -> ST s ()
311316
levelsInvariant !_ [] = return ()
@@ -415,9 +420,9 @@ invariant (LSMContent _ levels ul) = do
415420
-- In particular, there are no invariants on the progress of the merges,
416421
-- since union merge credits are independent from the tables' regular level
417422
-- merges.
418-
treeInvariant :: MergingTree s -> ST s ()
423+
treeInvariant :: MergingTree s -> Invariant s ()
419424
treeInvariant tree@(MergingTree treeState) = do
420-
readSTRef treeState >>= \case
425+
liftI (readSTRef treeState) >>= \case
421426
CompletedTreeMerge _ ->
422427
-- We don't require the completed merges to be non-empty, since even
423428
-- a (last-level) merge of non-empty runs can end up being empty.
@@ -436,31 +441,66 @@ treeInvariant tree@(MergingTree treeState) = do
436441
-- Non-empty, but can be just one input (see 'newPendingLevelMerge').
437442
-- Note that children of a pending merge can be empty runs, as noted
438443
-- above for 'CompletedTreeMerge'.
439-
assertST $ length irs + length t > 0
444+
assertI "pending level merges have at least one input" $
445+
length irs + length t > 0
440446
for_ irs $ \case
441447
Single _ -> return ()
442448
Merging _ mr -> mergeInvariant mr
443449
for_ t treeInvariant
444450

445451
PendingTreeMerge (PendingUnionMerge ts) -> do
446-
-- Merges are non-trivial (at least two inputs).
447-
assertST $ length ts > 1
452+
assertI "pending union merges are non-trivial (at least two inputs)" $
453+
length ts > 1
448454
for_ ts treeInvariant
449455

450-
(debt, _) <- remainingDebtMergingTree tree
456+
(debt, _) <- liftI $ remainingDebtMergingTree tree
451457
when (debt <= 0) $ do
452-
_ <- expectCompletedMergingTree tree
458+
_ <- isCompletedMergingTree tree
453459
return ()
454460

455-
mergeInvariant :: MergingRun t s -> ST s ()
461+
mergeInvariant :: MergingRun t s -> Invariant s ()
456462
mergeInvariant (MergingRun _ ref) =
457-
readSTRef ref >>= \case
463+
liftI (readSTRef ref) >>= \case
458464
CompletedMerge _ -> return ()
459465
OngoingMerge _ rs _ -> do
460-
-- Inputs to ongoing merges aren't empty.
461-
assertST $ all (\r -> runSize r > 0) rs
462-
-- Merges are non-trivial (at least two inputs).
463-
assertST $ length rs > 1
466+
assertI "inputs to ongoing merges aren't empty" $
467+
all (\r -> runSize r > 0) rs
468+
assertI "ongoing merges are non-trivial (at least two inputs)" $
469+
length rs > 1
470+
471+
isCompletedMergingRun :: MergingRun t s -> Invariant s Run
472+
isCompletedMergingRun (MergingRun _ ref) = do
473+
mrs <- liftI $ readSTRef ref
474+
case mrs of
475+
CompletedMerge r -> return r
476+
OngoingMerge d _ _ -> failI $ "not completed: OngoingMerge with"
477+
++ " remaining debt " ++ show d
478+
479+
isCompletedMergingTree :: MergingTree s -> Invariant s Run
480+
isCompletedMergingTree (MergingTree ref) = do
481+
mts <- liftI $ readSTRef ref
482+
case mts of
483+
CompletedTreeMerge r -> return r
484+
OngoingTreeMerge mr -> isCompletedMergingRun mr
485+
PendingTreeMerge _ -> failI $ "not completed: PendingTreeMerge"
486+
487+
type Invariant s = E.ExceptT String (ST s)
488+
489+
assertI :: String -> Bool -> Invariant s ()
490+
assertI _ True = return ()
491+
assertI e False = failI e
492+
493+
failI :: String -> Invariant s a
494+
failI = E.throwE
495+
496+
liftI :: ST s a -> Invariant s a
497+
liftI = E.ExceptT . fmap Right
498+
499+
expectInvariant :: HasCallStack => Invariant s a -> ST s a
500+
expectInvariant act = E.runExceptT act >>= either error return
501+
502+
evalInvariant :: Invariant s a -> ST s (Either String a)
503+
evalInvariant = E.runExceptT
464504

465505
-- 'callStack' just ensures that the 'HasCallStack' constraint is not redundant
466506
-- when compiling with debug assertions disabled.
@@ -521,12 +561,7 @@ combineUnion (Insert v' b') (Insert v b) = Insert (resolveValue v' v)
521561
(resolveBlob b' b)
522562

523563
expectCompletedMergingRun :: HasCallStack => MergingRun t s -> ST s Run
524-
expectCompletedMergingRun (MergingRun _ ref) = do
525-
mrs <- readSTRef ref
526-
case mrs of
527-
CompletedMerge r -> return r
528-
OngoingMerge d _ _ -> error $ "expectCompletedMergingRun:"
529-
++ " remaining debt of " ++ show d
564+
expectCompletedMergingRun = expectInvariant . isCompletedMergingRun
530565

531566
supplyCreditsMergingRun :: Credit -> MergingRun t s -> ST s Credit
532567
supplyCreditsMergingRun = checked remainingDebtMergingRun $ \credits (MergingRun _ ref) -> do
@@ -1221,11 +1256,7 @@ expectCompletedChildren (PendingMerge mt irs trees) = do
12211256
Merging _ mr -> expectCompletedMergingRun mr
12221257

12231258
expectCompletedMergingTree :: HasCallStack => MergingTree s -> ST s Run
1224-
expectCompletedMergingTree (MergingTree ref) = do
1225-
readSTRef ref >>= \case
1226-
CompletedTreeMerge r -> return r
1227-
OngoingTreeMerge mr -> expectCompletedMergingRun mr
1228-
PendingTreeMerge _ -> error $ "expectCompletedMergingTree: PendingTreeMerge"
1259+
expectCompletedMergingTree = expectInvariant . isCompletedMergingTree
12291260

12301261
-------------------------------------------------------------------------------
12311262
-- Measurements

prototypes/ScheduledMergesTest.hs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -208,18 +208,19 @@ prop_MergingTree t credits =
208208
QC.ioProperty $ runWithTracer $ \_tr ->
209209
stToIO $ do
210210
tree <- fromT t
211-
go tree (QC.getInfiniteList credits)
212-
(d', _) <- LSM.remainingDebtMergingTree tree
211+
res <- go tree (QC.getInfiniteList credits)
213212
return $
214-
QC.classify (d' <= 0) "got completed" $
215-
True
213+
res === Right ()
216214
where
215+
-- keep supplying until there is an error or the tree merge is completed
216+
go :: MergingTree s -> [SmallCredit] -> ST s (Either String ())
217217
go tree (SmallCredit c : cs) = do
218218
c' <- LSM.supplyCreditsMergingTree c tree
219-
treeInvariant tree
220-
if c' > 0 then return ()
221-
else go tree cs
222-
go _ _ = error "infinite list is finite"
219+
evalInvariant (treeInvariant tree) >>= \case
220+
Left e -> return (Left e)
221+
Right () -> if c' > 0 then return (Right ())
222+
else go tree cs
223+
go _ [] = error "infinite list is finite"
223224

224225
newtype SmallCredit = SmallCredit Credit
225226
deriving stock Show

0 commit comments

Comments
 (0)