Address review feedback

TODO: split and rebase into earlier commits

Author: dcoutts

bench/macro/lsm-tree-bench-wp8.hs

@@ -180,19 +180,23 @@ mkTableConfigSetup GlobalOpts{diskCachePolicy} SetupOpts{bloomFilterAlloc} conf
     , LSM.confBloomFilterAlloc = bloomFilterAlloc
     }
 
-mkTableConfigRun :: GlobalOpts -> LSM.TableConfig -> LSM.TableConfig
-mkTableConfigRun GlobalOpts{diskCachePolicy} conf = conf {
-      LSM.confDiskCachePolicy = diskCachePolicy
+mkTableConfigRun :: GlobalOpts -> RunOpts -> LSM.TableConfig -> LSM.TableConfig
+mkTableConfigRun GlobalOpts{diskCachePolicy} RunOpts {pipelined} conf =
+    conf {
+      LSM.confDiskCachePolicy = diskCachePolicy,
+      LSM.confMergeBatchSize  = if pipelined
+                                  then LSM.MergeBatchSize 1
+                                  else LSM.confMergeBatchSize conf
     }
 
 mkTableConfigOverride :: GlobalOpts -> RunOpts -> LSM.TableConfigOverride
 mkTableConfigOverride GlobalOpts{diskCachePolicy} RunOpts {pipelined} =
-  LSM.noTableConfigOverride {
-    LSM.overrideDiskCachePolicy = Just diskCachePolicy,
-    LSM.overrideMergeBatchSize  = if pipelined
-                                    then Just (LSM.MergeBatchSize 1)
-                                    else Nothing
-  }
+    LSM.noTableConfigOverride {
+      LSM.overrideDiskCachePolicy = Just diskCachePolicy,
+      LSM.overrideMergeBatchSize  = if pipelined
+                                      then Just (LSM.MergeBatchSize 1)
+                                      else Nothing
+    }
 
 mkTracer :: GlobalOpts -> Tracer IO LSM.LSMTreeTrace
 mkTracer gopts
@@ -588,7 +592,7 @@ doRun gopts opts = do
         -- reference version starts with empty (as it's not practical or
         -- necessary for testing to load the whole snapshot).
         tbl <- if check opts
-                then let conf = mkTableConfigRun gopts benchTableConfig
+                then let conf = mkTableConfigRun gopts opts benchTableConfig
                       in LSM.newTableWith @IO @K @V @B conf session
                 else let conf = mkOverrideDiskCachePolicy gopts opts
                       in LSM.openTableFromSnapshotWith @IO @K @V @B conf session name label

lsm-tree.cabal

@@ -183,6 +183,12 @@ description:
       The /disk cache policy/ determines if lookup operations use the OS page cache.
       Caching may improve the performance of lookups and updates if database access follows certain patterns.
 
+  [@confMergeBatchSize@]
+      The merge batch size balances the maximum latency of individual update
+      operations, versus the latency of a sequence of update operations. Bigger
+      batches improves overall performance but some updates will take a lot
+      longer than others. The default is to use a large batch size.
+
   ==== Fine-tuning: Merge Policy, Size Ratio, and Write Buffer Size #fine_tuning_data_layout#
 
   The configuration parameters @confMergePolicy@, @confSizeRatio@, and @confWriteBufferAlloc@ affect how the table organises its data.
@@ -429,6 +435,31 @@ description:
   *   Use the @DiskCacheNone@ policy if the database's access pattern has does not have good spatial or temporal locality.
       For instance, if the access pattern is uniformly random.
 
+  ==== Fine-tuning: Merge Batch Size #fine_tuning_merge_batch_size#
+
+  The /merge batch size/ is a micro-tuning parameter, and in most cases you do
+  need to think about it and can leave it at its default.
+
+  When using the 'Incremental' merge schedule, merging is done in batches. This
+  is a trade-off: larger batches tends to mean better overall performance but the
+  downside is that while most updates (inserts, deletes, upserts) are fast, some
+  are slower (when a batch of merging work has to be done).
+
+  If you care most about the maximum latency of updates, then use a small batch
+  size. If you don't care about latency of individual operations, just the
+  latency of the overall sequence of operations then use a large batch size. The
+  default is to use a large batch size, the same size as the write buffer itself.
+  The minimum batch size is 1. The maximum batch size is the size of the write
+  buffer 'confWriteBufferAlloc'.
+
+  Note that the actual batch size is the minimum of this configuration
+  parameter and the size of the batch of operations performed (e.g. 'inserts').
+  So if you consistently use large batches, you can use a batch size of 1 and
+  the merge batch size will always be determined by the operation batch size.
+
+  A further reason why it may be preferable to use minimal batch sizes is to get
+  good parallel work balance, when using parallelism.
+
   == References
 
   The implementation of LSM-trees in this package draws inspiration from:

src/Database/LSMTree.hs

@@ -109,7 +109,8 @@ module Database.LSMTree (
     confBloomFilterAlloc,
     confFencePointerIndex,
     confDiskCachePolicy,
-    confMergeSchedule
+    confMergeSchedule,
+    confMergeBatchSize
   ),
   defaultTableConfig,
   MergePolicy (LazyLevelling),

src/Database/LSMTree/Internal/Config.hs

@@ -94,6 +94,12 @@ For a detailed discussion of fine-tuning the table configuration, see [Fine-tuni
 [@confDiskCachePolicy :: t'DiskCachePolicy'@]
     The /disk cache policy/ supports caching lookup operations using the OS page cache.
     Caching may improve the performance of lookups and updates if database access follows certain patterns.
+
+[@confMergeBatchSize :: t'MergeBatchSize'@]
+    The merge batch size balances the maximum latency of individual update
+    operations, versus the latency of a sequence of update operations. Bigger
+    batches improves overall performance but some updates will take a lot
+    longer than others. The default is to use a large batch size.
 -}
 data TableConfig = TableConfig {
     confMergePolicy       :: !MergePolicy
@@ -128,6 +134,8 @@ instance NFData TableConfig where
 -- OrdinaryIndex
 -- >>> confDiskCachePolicy defaultTableConfig
 -- DiskCacheAll
+-- >>> confMergeBatchSize defaultTableConfig
+-- MergeBatchSize 20000
 --
 defaultTableConfig :: TableConfig
 defaultTableConfig =
@@ -412,7 +420,8 @@ If you care most about the maximum latency of updates, then use a small batch
 size. If you don't care about latency of individual operations, just the
 latency of the overall sequence of operations then use a large batch size. The
 default is to use a large batch size, the same size as the write buffer itself.
-The minimum batch size is 1.
+The minimum batch size is 1. The maximum batch size is the size of the write
+buffer 'confWriteBufferAlloc'.
 
 Note that the actual batch size is the minimum of this configuration
 parameter and the size of the batch of operations performed (e.g. 'inserts').
@@ -429,6 +438,14 @@ newtype MergeBatchSize = MergeBatchSize Int
 -- TODO: the thresholds for doing merge work should be different for each level,
 -- and ideally all-pairs co-prime.
 creditThresholdForLevel :: TableConfig -> LevelNo -> MR.CreditThreshold
-creditThresholdForLevel TableConfig { confMergeBatchSize = MergeBatchSize n }
+creditThresholdForLevel TableConfig {
+                           confMergeBatchSize   = MergeBatchSize mergeBatchSz,
+                           confWriteBufferAlloc = AllocNumEntries writeBufferSz
+                         }
                         (LevelNo _i) =
-    MR.CreditThreshold (MR.UnspentCredits (MR.MergeCredits (max 1 n)))
+    MR.CreditThreshold
+  . MR.UnspentCredits
+  . MR.MergeCredits
+  . max 1
+  . min writeBufferSz
+  $ mergeBatchSz

test/Test/Database/LSMTree/Internal/Snapshot/Codec/Golden.hs

@@ -153,6 +153,7 @@ forallSnapshotTypes f = [
     , f (Proxy @FencePointerIndexType)
     , f (Proxy @DiskCachePolicy)
     , f (Proxy @MergeSchedule)
+    , f (Proxy @MergeBatchSize)
       -- SnapLevels
     , f (Proxy @(SnapLevels SnapshotRun))
     , f (Proxy @(SnapLevel SnapshotRun))

test/golden-file-data/snapshot-codec/MergeBatchSize.A.snapshot.golden

@@ -0,0 +1 @@
+

test/golden-file-data/snapshot-codec/MergeBatchSize.B.snapshot.golden

@@ -0,0 +1 @@
+�