Skip to content

Commit fbda802

Browse files
committed
refuse one run instead of allowing an underfull run
1 parent c837cbc commit fbda802

File tree

1 file changed

+27
-20
lines changed

1 file changed

+27
-20
lines changed

prototypes/ScheduledMerges.hs

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,8 @@ invariant = go 1
196196
-- too large and is promoted, in that case initially there's no merge,
197197
-- but it is still represented as a 'MergingRun', using 'SingleRun'.
198198
MergePolicyLevelling -> assertST $ null rs
199-
-- Runs in tiering levels usually fit that size, but they can be one
200-
-- smaller due to compaction (if they have not been held back and
201-
-- merged again).
202-
MergePolicyTiering -> assertST $ all (\r -> tieringRunSizeToLevel r `elem` [ln-1, ln]) rs
199+
-- Runs in tiering levels fit that size.
200+
MergePolicyTiering -> assertST $ all (\r -> tieringRunSizeToLevel r == ln) rs
203201

204202
-- Incoming runs being merged also need to be of the right size, but the
205203
-- conditions are more complicated.
@@ -234,7 +232,7 @@ invariant = go 1
234232
let resident = drop 4 rs
235233
assertST $ length incoming == 4
236234
assertST $ length resident <= 1
237-
assertST $ all (\r -> tieringRunSizeToLevel r `elem` [ln-2, ln-1]) incoming
235+
assertST $ all (\r -> tieringRunSizeToLevel r == ln-1) incoming
238236
assertST $ all (\r -> levellingRunSizeToLevel r <= ln+1) resident
239237

240238
MergePolicyTiering ->
@@ -261,12 +259,12 @@ invariant = go 1
261259
assertST $ tieringRunSizeToLevel r `elem` [ln-1, ln]
262260

263261
-- An ongoing merge for tiering should have 4 incoming runs of
264-
-- the right size for the level below (or slightly smaller), and at
265-
-- most 1 run held back due to being too small (which would thus
266-
-- also be of the size of the level below).
262+
-- the right size for the level below, and at most 1 run held back
263+
-- due to being too small (which would thus also be of the size of
264+
-- the level below).
267265
(_, OngoingMerge _ rs _, _) -> do
268266
assertST $ length rs == 4 || length rs == 5
269-
assertST $ all (\r -> tieringRunSizeToLevel r `elem` [ln-2, ln-1]) rs
267+
assertST $ all (\r -> tieringRunSizeToLevel r == ln-1) rs
270268

271269
-- 'callStack' just ensures that the 'HasCallStack' constraint is not redundant
272270
-- when compiling with debug assertions disabled.
@@ -486,8 +484,11 @@ creditsForMerge SingleRun{} = 0
486484
creditsForMerge (MergingRun MergePolicyLevelling _ _) = (1 + 4) / 1
487485

488486
-- A tiering merge has 4 runs at most (once could be held back to merged again)
489-
-- and must be completed before the level is full (once 4 more runs come in).
490-
creditsForMerge (MergingRun MergePolicyTiering _ _) = 4 / 4
487+
-- and must be completed before the level is full (once 3 more runs come in,
488+
-- as it could have started out with an additional refused run).
489+
-- TODO: We could only increase the merging speed for the merges where this
490+
-- applies, which should be rare.
491+
creditsForMerge (MergingRun MergePolicyTiering _ _) = 4 / 3
491492

492493
type Event = EventAt EventDetail
493494
data EventAt e = EventAt {
@@ -531,9 +532,8 @@ increment tr sc = \r ls -> do
531532
assertST $ tieringRunSizeToLevel r `elem` [ln, ln+1] -- +1 from levelling
532533
_ -> do
533534
assertST $ length incoming == 4
534-
-- because of underfull runs
535-
assertST $ all (\r -> tieringRunSizeToLevel r `elem` [ln-2, ln-1]) incoming
536-
assertST $ tieringLevel (sum (map Map.size incoming)) `elem` [ln-1, ln]
535+
assertST $ all (\r -> tieringRunSizeToLevel r == ln-1) incoming
536+
assertST $ tieringLevel (sum (map Map.size incoming)) == ln
537537
(ls', refused) <- go' ln incoming ls
538538
for_ refused $ assertST . (== head incoming)
539539
return (ls', refused)
@@ -554,12 +554,19 @@ increment tr sc = \r ls -> do
554554
-- If r is still too small for this level then keep it and merge again
555555
-- with the incoming runs, but only if the resulting run is guaranteed
556556
-- not to be too large for this level.
557-
MergePolicyTiering
558-
| tieringRunSizeToLevel r < ln
559-
, sum (map Map.size (r : incoming)) <= tieringRunSize ln -> do
560-
let mergelast = mergeLastForLevel ls
561-
mr' <- newMerge tr' ln MergePolicyTiering mergelast (incoming ++ [r])
562-
return (Level mr' rs : ls, Nothing)
557+
-- If it might become too large, only create a 4-way merge and refuse
558+
-- the most recent of the incoming runs.
559+
MergePolicyTiering | tieringRunSizeToLevel r < ln ->
560+
if sum (map Map.size (r : incoming)) <= tieringRunSize ln
561+
then do
562+
let mergelast = mergeLastForLevel ls
563+
mr' <- newMerge tr' ln MergePolicyTiering mergelast (incoming ++ [r])
564+
return (Level mr' rs : ls, Nothing)
565+
else do
566+
-- TODO: comment
567+
let mergelast = mergeLastForLevel ls
568+
mr' <- newMerge tr' ln MergePolicyTiering mergelast (tail incoming ++ [r])
569+
return (Level mr' rs : ls, Just (head incoming))
563570

564571
-- This tiering level is now full. We take the completed merged run
565572
-- (the previous incoming runs), plus all the other runs on this level

0 commit comments

Comments
 (0)