Skip to content

Commit 18d2e5d

Browse files
committed
Shrink TableConfig in StateMachine tests
The `TableConfig` arbitrary instance is still quite suboptimal. I've added a TODO to improve it.
1 parent b6935e1 commit 18d2e5d

File tree

1 file changed

+44
-3
lines changed

1 file changed

+44
-3
lines changed

test/Test/Database/LSMTree/Normal/StateMachine.hs

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,17 +192,58 @@ propLockstep_ModelIOImpl =
192192
= Nothing
193193

194194
instance Arbitrary R.TableConfig where
195-
arbitrary :: Gen R.TableConfig
196-
arbitrary = pure $ R.TableConfig {
195+
arbitrary = do
196+
confWriteBufferAlloc <- QC.arbitrary
197+
pure $ R.TableConfig {
197198
R.confMergePolicy = R.MergePolicyLazyLevelling
198199
, R.confSizeRatio = R.Four
199-
, R.confWriteBufferAlloc = R.AllocNumEntries (R.NumEntries 30)
200+
, confWriteBufferAlloc
200201
, R.confBloomFilterAlloc = R.AllocFixed 10
201202
, R.confFencePointerIndex = R.CompactIndex
202203
, R.confDiskCachePolicy = R.DiskCacheNone
203204
, R.confMergeSchedule = R.OneShot
204205
}
205206

207+
shrink R.TableConfig{..} =
208+
[ R.TableConfig {
209+
confWriteBufferAlloc = confWriteBufferAlloc'
210+
, ..
211+
}
212+
| confWriteBufferAlloc' <- QC.shrink confWriteBufferAlloc
213+
]
214+
215+
-- TODO: the current generator is suboptimal, and should be improved. There are
216+
-- some aspects to consider, and since they are at tension with each other, we
217+
-- should try find a good balance.
218+
--
219+
-- Say the write buffer allocation is @n@.
220+
--
221+
-- * If @n@ is too large, then the tests degrade to only testing the write
222+
-- buffer, because there are no flushes/merges.
223+
--
224+
-- * If @n@ is too small, then the resulting runs are also small. They might
225+
-- consist of only a few disk pages, or they might consist of only 1 underfull
226+
-- disk page. This means there are some parts of the code that we would rarely
227+
-- cover in the state machine tests.
228+
--
229+
-- * If @n@ is too small, then we flush/merge very frequently, which may exhaust
230+
-- the number of open file handles when using the real file system. Specially
231+
-- if the test fails and the shrinker kicks in, then @n@ can currently become
232+
-- very small. This is good for finding a minimal counterexample, but it might
233+
-- also make the real filesystem run out of file descriptors. Arguably, you
234+
-- could overcome this specific issue by only generating or shrinking to small
235+
-- @n@ when we use the mocked file system. This would require some boilerplate
236+
-- to add type level tags to distinguish between the two cases.
237+
instance Arbitrary R.WriteBufferAlloc where
238+
arbitrary = QC.scale (max 30) $ do
239+
QC.Positive x <- QC.arbitrary
240+
pure (R.AllocNumEntries (R.NumEntries x))
241+
242+
shrink (R.AllocNumEntries (R.NumEntries x)) =
243+
[ R.AllocNumEntries (R.NumEntries x')
244+
| QC.Positive x' <- QC.shrink (QC.Positive x)
245+
]
246+
206247
propLockstep_RealImpl_RealFS_IO ::
207248
Actions (Lockstep (ModelState R.TableHandle))
208249
-> QC.Property

0 commit comments

Comments
 (0)