[lmdb facebookincubator#8] Add server_config field to select storage

* db_create_storage config to select storage backend ("rocksdb" or "lmdb")
* make DB versions Storage-specific, as they should be
* fix DatabaseJanitorTest and enable it

Author: simonmar

glean.cabal.in

@@ -2140,8 +2140,6 @@ test-suite dbjanitor
         glean:schema,
         glean:config,
         glean:util
-    -- https://github.com/facebookincubator/Glean/issues/224
-    buildable: False
 
 test-suite dbderive
     import: test

glean/config/server/server_config.thrift

@@ -236,7 +236,7 @@ struct Config {
   16: i32 db_lookup_cache_limit_mb = 1000;
 
   // What binary representation to use for newly created databases
-  // (nothing means use latest supported).
+  // (nothing means use latest supported). See also db_create_storage.
   19: optional DBVersion db_create_version;
 
   // Disable completed dependencies check in stored predicates derivation
@@ -325,6 +325,10 @@ struct Config {
   // runtime cost to decompress the DB on demand. Mounting requires
   // squashfs-tools and squashfuse to be installed on Linux.
   40: bool db_lmdb_restore_unpack = true;
+
+  // Default storage backend for newly created databases. Can be overriden
+  // by command-line options. See also db_create_version.
+  41: optional string db_create_storage;
 }
 
 // The following were automatically generated and may benefit from renaming.

glean/db/Glean/Database/Config.hs

@@ -116,7 +116,9 @@ data DataStore = DataStore
       ServerConfig.Config ->
       ((HashMap StorageName (Some Storage), Some Catalog.Store) -> IO a) ->
       IO a
-  , defaultStorage :: StorageName
+  , defaultStorage :: (StorageName, Bool)
+    -- ^ Default storage, Bool => explicitly selected on CLI
+    -- (otherwise can be overriden by ServerConfig.config_db_create_storage)
   , dataStoreTag :: String
   }
 
@@ -137,7 +139,7 @@ fileDataStore path = DataStore
           ],
         Some (Catalog.fileCatalog path)
         )
-  , defaultStorage = rocksdbName
+  , defaultStorage = (rocksdbName, False)
   , dataStoreTag = "db:" <> path
   }
 
@@ -146,7 +148,7 @@ tmpDataStore = DataStore
   { withStorage = \scfg f -> withSystemTempDirectory "glean" $ \tmp -> do
       logInfo $ "Storing temporary DBs in " <> tmp
       withStorage (fileDataStore tmp) scfg f
-  , defaultStorage = rocksdbName
+  , defaultStorage = (rocksdbName, False)
   , dataStoreTag = dataStoreTag (fileDataStore "<tmp>")
   }
 
@@ -156,7 +158,7 @@ memoryDataStore = DataStore
       cat <- Catalog.memoryCatalog
       mem <- Memory.newStorage
       f (HashMap.fromList [(memoryName, Some mem)], Some cat)
-  , defaultStorage = memoryName
+  , defaultStorage = (memoryName, False)
   , dataStoreTag = "memory"
   }
 
@@ -518,7 +520,7 @@ options = do
         help "Directory containing databases")
       lmdb <- switch (long "lmdb")
       pure $
-        (if lmdb then \s -> s { defaultStorage = lmdbName } else id) $
+        (if lmdb then \s -> s { defaultStorage = (lmdbName, True) } else id) $
         fileDataStore path
     dbTmp = tmpDataStore <$ flag' () (
       long "db-tmp" <>

glean/db/Glean/Database/Create.hs

@@ -105,10 +105,14 @@ kickOffDatabase env@Env{..} kickOff@Thrift.KickOff{..}
           , timestampRepoHash =
               posixEpochTimeToUTCTime <$> kickOff_repo_hash_time
           }
+      withDefaultStorage env $ \storageName storage -> do
+
       version <-
-        fromMaybe Storage.currentVersion . ServerConfig.config_db_create_version
+        fromMaybe (Storage.currentVersion storage) .
+          ServerConfig.config_db_create_version
         <$> Observed.get envServerConfig
-      when (not $ Storage.canOpenVersion Storage.ReadWrite version) $
+
+      when (not $ Storage.canOpenVersion storage Storage.ReadWrite version) $
         dbError kickOff_repo
           "can't create databases (unsupported binary version)"
       db <- atomically $ newDB kickOff_repo
@@ -122,7 +126,7 @@ kickOffDatabase env@Env{..} kickOff@Thrift.KickOff{..}
         -- concept of deleting DBs will change with the new metadata handling so
         -- it's not worth fixing at this point, especially since we aren't
         -- supposed to be kicking off DBs we've previously deleted.
-        let meta = newMeta envDefaultStorage version time
+        let meta = newMeta storageName version time
                      (Incomplete def) allProps
                      (lightDeps kickOff_dependencies')
         bracket_

glean/db/Glean/Database/Env.hs

@@ -89,7 +89,7 @@ withDatabases evb cfg cfgapi act =
 initEnv
   :: EventBaseDataplane
   -> HashMap StorageName (Some Storage.Storage)
-  -> StorageName
+  -> (StorageName, Bool)
   -> Catalog.Catalog
   -> SomeShardManager
   -> Config
@@ -222,7 +222,7 @@ spawnThreads env@Env{..} = do
 
   -- Disk usage counters
   Warden.spawn_ envWarden $ doPeriodically (seconds 600) $
-    withDefaultStorage env $ \storage -> do
+    withDefaultStorage env $ \_ storage -> do
     diskSize <- Storage.getTotalCapacity storage
     diskUsed <- Storage.getUsedCapacity storage
 

glean/db/Glean/Database/Open.hs

@@ -99,8 +99,6 @@ withOpenDatabase env@Env{..} repo action =
                     -- Note: Finalizing also needs to be ReadWrite,
                     -- because compaction modifies the DB.
                   where completeness = metaCompleteness meta
-            when (not $ canOpenVersion mode version) $ dbError dbRepo
-              $ "can't open database version " ++ show (unDBVersion version)
             writeTVar dbState Opening
             return $ Right (version, mode)
       case r of
@@ -484,10 +482,13 @@ asyncOpenDB env@Env{..} storage db@DB{..} version mode deps
     on_success on_failure =
   -- Be paranoid about 'spawnMask' itself throwing.
   handling_failures $ Warden.spawnMask envWarden $ \restore ->
-  loggingAction (runLogRepo "open" env dbRepo) (const mempty) $
+  loggingAction (runLogRepo "open" env dbRepo) (const mempty) $ do
+  when (not $ canOpenVersion storage mode version) $
+    dbError dbRepo $
+      "can't open database version " ++ show (unDBVersion version)
   bracket_
     (atomically $ acquireDB db)
-    (atomically $ releaseDB envCatalog envActive db) $
+    (atomically $ releaseDB envCatalog envActive db) $ do
   handling_failures $ do
     logInfo $ inRepo dbRepo "opening"
     bracketOnError

glean/db/Glean/Database/Storage.hs

@@ -17,7 +17,6 @@ module Glean.Database.Storage
   , WriteLock(..)
   , canOpenVersion
   , currentVersion
-  , writableVersions
   ) where
 
 import Data.ByteString (ByteString)
@@ -36,26 +35,18 @@ import qualified Glean.ServerConfig.Types as ServerConfig
 import Glean.Types (PredicateStats, Repo, SchemaId)
 import Glean.Util.Some
 
--- | List of binary representation versions we can read
-readableVersions :: [DBVersion]
-readableVersions = [DBVersion 3]
-
--- | List of binary representation versions we can write
-writableVersions :: [DBVersion]
-writableVersions = [DBVersion 3]
-
 -- | Check whether we can open a particular database version
-canOpenVersion :: Mode -> DBVersion -> Bool
-canOpenVersion mode version = version `elem` versions
+canOpenVersion :: Storage s => s -> Mode -> DBVersion -> Bool
+canOpenVersion s mode version = version `elem` versions
   where
     versions = case mode of
-      ReadOnly -> readableVersions
-      ReadWrite -> writableVersions
-      Create{} -> writableVersions
+      ReadOnly -> readableVersions s
+      ReadWrite -> writableVersions s
+      Create{} -> writableVersions s
 
 -- | Default current binary representation version
-currentVersion :: DBVersion
-currentVersion = maximum writableVersions
+currentVersion :: Storage s => s -> DBVersion
+currentVersion = maximum . writableVersions
 
 -- Choose which schema goes into a newly created DB
 data CreateSchema
@@ -87,6 +78,12 @@ class DatabaseOps (Database s) => Storage s where
   -- | A short, user-readable description of the storage
   describe :: s -> String
 
+  -- | List of binary representation versions we can read
+  readableVersions :: s -> [DBVersion]
+
+  -- | List of binary representation versions we can write
+  writableVersions :: s -> [DBVersion]
+
   -- | Open a database
   open :: s -> Repo -> Mode -> DBVersion -> IO (Database s)
 

glean/db/Glean/Database/Storage/LMDB.hs

@@ -70,6 +70,10 @@ instance CanLookup (Database LMDB) where
 instance Storage LMDB where
   describe db = "lmdb:" <> lmdbRoot db
 
+  -- we started at 3 because that's what RocksDB was on
+  readableVersions _ = [DBVersion 3]
+  writableVersions _ = [DBVersion 3]
+
   open lmdb repo mode (DBVersion version) = do
     (cmode, start, ownership) <- case mode of
       ReadOnly -> do

glean/db/Glean/Database/Storage/Memory.hs

@@ -47,6 +47,9 @@ instance Storage Memory where
 
   describe = const "memory:"
 
+  readableVersions _ = [DBVersion 0]
+  writableVersions _ = [DBVersion 0]
+
   open (Memory v) repo (Create start _unit _) _ = do
     facts <- FactSet.new start
     atomically $ do

glean/db/Glean/Database/Storage/RocksDB.hs

@@ -107,6 +107,9 @@ newtype instance Database RocksDB = Database DB
 instance Storage RocksDB where
   describe rocks = "rocksdb:" <> rocksRoot rocks
 
+  readableVersions _ = [DBVersion 3]
+  writableVersions _ = [DBVersion 3]
+
   open rocks repo mode (DBVersion version) = do
     (cmode, start, ownership) <- case mode of
       ReadOnly -> return (0, invalidFid, Nothing)