Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table union porting: change merge credit tracking strategy and return leftovers #554

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Table union porting: change merge credit tracking strategy and return leftovers #554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4dd1a73
Refactor: move spent credits var up from MergingRunState to MergingRun
dcoutts Jan 27, 2025
c59cd57
Improve documentation of credit tracking in MergingRun
dcoutts Jan 28, 2025
1ad6513
Change the MergingRun credit tracking strategy
dcoutts Jan 28, 2025
4e68cd3
Better encapsulate MergingRun internals
dcoutts Feb 3, 2025
64d0a31
Return leftover credits from supplyCredits
dcoutts Feb 4, 2025
ded9fc5
Add tests for min/max bounds on (un)spent credits
dcoutts Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lsm-tree.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ test-suite lsm-tree-test
Test.Database.LSMTree.Internal.Index.Ordinary
Test.Database.LSMTree.Internal.Lookup
Test.Database.LSMTree.Internal.Merge
Test.Database.LSMTree.Internal.MergingRun
Test.Database.LSMTree.Internal.MergingTree
Test.Database.LSMTree.Internal.Monkey
Test.Database.LSMTree.Internal.PageAcc
Expand Down
10 changes: 2 additions & 8 deletions src-extras/Database/LSMTree/Extras/NoThunks.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,8 @@ deriving anyclass instance NoThunks MergePolicyForLevel
deriving stock instance Generic NumRuns
deriving anyclass instance NoThunks NumRuns

deriving stock instance Generic (UnspentCreditsVar s)
deriving anyclass instance Typeable s => NoThunks (UnspentCreditsVar s)

deriving stock instance Generic (TotalStepsVar s)
deriving anyclass instance Typeable s => NoThunks (TotalStepsVar s)

deriving stock instance Generic (SpentCreditsVar s)
deriving anyclass instance Typeable s => NoThunks (SpentCreditsVar s)
deriving stock instance Generic (CreditsVar s)
deriving anyclass instance Typeable s => NoThunks (CreditsVar s)

deriving stock instance Generic MergeKnownCompleted
deriving anyclass instance NoThunks MergeKnownCompleted
Expand Down
11 changes: 6 additions & 5 deletions src/Database/LSMTree/Internal/MergeSchedule.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ addRunToLevels tr conf@TableConfig{..} resolve hfs hbio root uc r0 reg levels =
OneShot -> do
let !required = MR.Credits (unNumEntries (V.foldMap' Run.size rs))
let !thresh = creditThresholdForLevel conf ln
MR.supplyCredits required thresh mr
_leftoverCredits <- MR.supplyCredits mr thresh required
-- This ensures the merge is really completed. However, we don't
-- release the merge yet and only briefly inspect the resulting run.
bracket (MR.expectCompleted mr) releaseRef $ \r ->
Expand Down Expand Up @@ -802,7 +802,8 @@ supplyCredits conf c levels =
Merging mp mr -> do
let !c' = scaleCreditsForMerge mp mr c
let !thresh = creditThresholdForLevel conf ln
MR.supplyCredits c' thresh mr
_leftoverCredits <- MR.supplyCredits mr thresh c'
return ()

-- | Scale a number of credits to a number of merge steps to be performed, based
-- on the merging run.
Expand All @@ -818,7 +819,7 @@ scaleCreditsForMerge LevelTiering _ (Credits c) =
-- runs come in).
MR.Credits (c * (1 + 4))

scaleCreditsForMerge LevelLevelling (DeRef mr) (Credits c) =
scaleCreditsForMerge LevelLevelling mr (Credits c) =
-- A levelling merge has 1 input run and one resident run, which is (up
-- to) 4x bigger than the others. It needs to be completed before
-- another run comes in.
Expand All @@ -828,7 +829,7 @@ scaleCreditsForMerge LevelLevelling (DeRef mr) (Credits c) =
-- worst-case upper bound by looking at the sizes of the input runs.
-- As as result, merge work would/could be more evenly distributed over
-- time when the resident run is smaller than the worst case.
let NumRuns n = MR.mergeNumRuns mr
let NumRuns n = MR.numRuns mr
-- same as division rounding up: ceiling (c * n / 4)
in MR.Credits ((c * n + 3) `div` 4)

Expand All @@ -837,4 +838,4 @@ scaleCreditsForMerge LevelLevelling (DeRef mr) (Credits c) =
creditThresholdForLevel :: TableConfig -> LevelNo -> MR.CreditThreshold
creditThresholdForLevel conf (LevelNo _i) =
let AllocNumEntries (NumEntries x) = confWriteBufferAlloc conf
in MR.CreditThreshold x
in MR.CreditThreshold (MR.Credits x)
Loading