Skip to content

Commit bbb4daf

Browse files
committed
Do not require input tables for unions to have matching config
The config of the resulting table is actually independent from the config of the input tables, so we do not need them to match. So we take the config from the first input table.
1 parent 64ef233 commit bbb4daf

File tree

2 files changed

+14
-32
lines changed

2 files changed

+14
-32
lines changed

src/Database/LSMTree/Internal.hs

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ import Control.Concurrent.Class.MonadSTM (MonadSTM (..))
7878
import Control.Concurrent.Class.MonadSTM.RWVar (RWVar)
7979
import qualified Control.Concurrent.Class.MonadSTM.RWVar as RW
8080
import Control.DeepSeq
81-
import Control.Monad (forM, unless, void, zipWithM_)
81+
import Control.Monad (forM, unless, void)
8282
import Control.Monad.Class.MonadST (MonadST (..))
8383
import Control.Monad.Class.MonadThrow
8484
import Control.Monad.Primitive
@@ -232,9 +232,6 @@ data LSMTreeError =
232232
Int -- ^ Vector index of table @t1@ involved in the mismatch
233233
Int -- ^ Vector index of table @t2@ involved in the mismatch
234234
-- | 'unions' was called on tables that do not have the same configuration.
235-
| ErrUnionsTableConfigMismatch
236-
Int -- ^ Vector index of table @t1@ involved in the mismatch
237-
Int -- ^ Vector index of table @t2@ involved in the mismatch
238235
deriving stock (Show, Eq)
239236
deriving anyclass (Exception)
240237

@@ -1359,16 +1356,12 @@ unions ts = do
13591356

13601357
traceWith (sessionTracer sesh) $ TraceUnions (NE.map tableId ts)
13611358

1362-
-- TODO: Do we really need the configs to match exactly? It's okay as a
1363-
-- requirement for now, but we might want to revisit it. Some settings don't
1364-
-- really need to match for things to work, but of course we'd still need to
1365-
-- answer the question of which config to use for the new table, possibly
1366-
-- requiring to supply it manually? Or, we could generalise the exact match
1367-
-- to have a config compatibility test and config merge?
1368-
conf <-
1369-
case match (fmap tableConfig ts) of
1370-
Left (i, j) -> throwIO $ ErrUnionsTableConfigMismatch i j
1371-
Right conf -> pure conf
1359+
-- The TableConfig for the new table is taken from the first / left
1360+
-- table in the union. This works because the new table is almost
1361+
-- completely fresh. It will have an empty write buffer and no runs
1362+
-- in the normal levels. All the existing runs get squashed down into
1363+
-- a single run before rejoining as a last level.
1364+
let conf = tableConfig (NE.head ts)
13721365

13731366
-- We acquire a read-lock on the session open-state to prevent races, see
13741367
-- 'sessionOpenTables'.
@@ -1399,24 +1392,6 @@ unions ts = do
13991392

14001393
pure (seshState, t)
14011394

1402-
-- | Like 'matchBy', but the match function is @(==)@.
1403-
match :: Eq a => NonEmpty a -> Either (Int, Int) a
1404-
match = matchBy (==)
1405-
1406-
-- | Check that all values in the list match. If so, return the matched value.
1407-
-- If there is a mismatch, return the list indices of the mismatching values.
1408-
matchBy :: forall a. (a -> a -> Bool) -> NonEmpty a -> Either (Int, Int) a
1409-
matchBy eq (x0 :| xs) =
1410-
case zipWithM_ (matchOne x0) [1..] xs of
1411-
Left i -> Left (0, i)
1412-
Right () -> Right x0
1413-
where
1414-
matchOne :: a -> Int -> a -> Either Int ()
1415-
matchOne x i y =
1416-
if (x `eq` y)
1417-
then Right ()
1418-
else Left i
1419-
14201395
-- | Check that all tables in the session match. If so, return the matched
14211396
-- session. If there is a mismatch, return the list indices of the mismatching
14221397
-- tables.

test/Database/LSMTree/Model/Session.hs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,13 @@ data Table k v b = Table {
305305
}
306306
deriving stock Show
307307

308+
-- | The model does not distinguish multiple values of the config type. This is
309+
-- the right thing because the table config is never observed directly, only
310+
-- via the influence it might have on other operation results. And we /do not
311+
-- want/ table configs to have observable value-level effects. The table config
312+
-- should only have performance effects. Thus having only one config value is
313+
-- the right abstraction for the model.
314+
--
308315
data TableConfig = TableConfig
309316
deriving stock (Show, Eq)
310317

0 commit comments

Comments
 (0)