Simplify database initialization (#107)

Improve error handling and Rocksdb column families setup
BlueBrain · Feb 18, 2020 · 8a51433 · 8a51433
1 parent 8583e89
commit 8a51433
Show file tree

Hide file tree

Showing 12 changed files with 103 additions and 151 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -3,37 +3,36 @@ list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake)
 
 # get version from Python package
 find_package(PythonInterp 3.5 REQUIRED)
-execute_process(COMMAND ${PYTHON_EXECUTABLE} setup.py --version
-                WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
-                RESULT_VARIABLE PY_VERSION_FAILED
-                OUTPUT_VARIABLE PY_PKG_VERSION_FULL
-                ERROR_VARIABLE PY_VERSION_ERROR
-                OUTPUT_STRIP_TRAILING_WHITESPACE)
+execute_process(
+  COMMAND ${PYTHON_EXECUTABLE} setup.py --version
+  WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
+  RESULT_VARIABLE PY_VERSION_FAILED
+  OUTPUT_VARIABLE PY_PKG_VERSION_FULL
+  ERROR_VARIABLE PY_VERSION_ERROR
+  OUTPUT_STRIP_TRAILING_WHITESPACE)
 if(PY_VERSION_FAILED)
   message(
     FATAL_ERROR
       "Could not retrieve version from command '${PYTHON_EXECUTABLE} setup.py --version'\n"
       ${PY_VERSION_ERROR})
 endif()
 # keep last line of command output
-string(REPLACE "\n"
-               ";"
-               PY_PKG_VERSION_FULL
-               "${PY_PKG_VERSION_FULL}")
+string(REPLACE "\n" ";" PY_PKG_VERSION_FULL "${PY_PKG_VERSION_FULL}")
 list(GET PY_PKG_VERSION_FULL -1 PY_PKG_VERSION_FULL)
 # keep MAJOR.MINOR.PATCH
-string(REGEX
-       REPLACE "([0-9]+\\.[0-9]+\\.[0-9]+).*"
-               "\\1"
-               PY_PKG_VERSION
-               "${PY_PKG_VERSION_FULL}")
+string(REGEX REPLACE "([0-9]+\\.[0-9]+\\.[0-9]+).*" "\\1" PY_PKG_VERSION "${PY_PKG_VERSION_FULL}")
 
-project(Basalt VERSION ${PY_PKG_VERSION} LANGUAGES CXX)
+project(
+  Basalt
+  VERSION ${PY_PKG_VERSION}
+  LANGUAGES CXX)
 
 if(NOT IS_DIRECTORY ${CMAKE_SOURCE_DIR}/cmake/hpc-coding-conventions)
-  message(FATAL_ERROR "It seems the git modules are missing.\n"
-                      "You can retrieve them by running the following command:\n"
-                      "    git submodule update --recursive --init")
+  message(
+    FATAL_ERROR
+      "It seems the git modules are missing.\n"
+      "You can retrieve them by running the following command:\n"
+      "    git submodule update --recursive --init")
 endif()
 add_subdirectory(cmake/hpc-coding-conventions/cpp)
 
@@ -45,17 +44,15 @@ bob_option(Basalt_USE_pybind11 "Build Python bindings" OFF)
 bob_option(USE_CLANG_TIDY "Perform C++ static analysis while compiling" OFF)
 
 set(Basalt_SERIALIZATION_METHODS CEREAL SSTREAM)
-bob_input(Basalt_SERIALIZATION
-          CEREAL
-          STRING
+bob_input(Basalt_SERIALIZATION CEREAL STRING
           "Serialization method: ${Basalt_SERIALIZATION_METHODS}")
 set_property(CACHE Basalt_SERIALIZATION PROPERTY STRINGS "${Basalt_SERIALIZATION_METHODS}")
 list(FIND Basalt_SERIALIZATION_METHODS ${Basalt_SERIALIZATION} index)
 if(index EQUAL -1)
   message(
     FATAL_ERROR
       "Unknown serialization '${Basalt_SERIALIZATION}'. Expected one of : ${Basalt_SERIALIZATION_METHODS}"
-    )
+  )
 endif()
 unset(Basalt_SERIALIZATION_METHODS)
 unset(index)

diff --git a/cmake/DartConfig.cmake b/cmake/DartConfig.cmake
@@ -11,10 +11,9 @@
 
 find_program(MEMORYCHECK_COMMAND valgrind DOC "Memory checking utility (valgrind by default)")
 
-set(
-  MEMORYCHECK_COMMAND_OPTIONS
-  "--tool=memcheck --track-origins=yes --leak-check=full --show-leak-kinds=all --verbose --gen-suppressions=all"
-  CACHE STRING "Commands passed to valgrind")
+set(MEMORYCHECK_COMMAND_OPTIONS
+    "--tool=memcheck --track-origins=yes --leak-check=full --show-leak-kinds=all --verbose --gen-suppressions=all"
+    CACHE STRING "Commands passed to valgrind")
 
 # Look for suppressions files
 file(GLOB dartconfig_supp_files ${CMAKE_SOURCE_DIR}/tests/memorycheck/*.supp)

diff --git a/cmake/FindClangTidy.cmake b/cmake/FindClangTidy.cmake
@@ -12,25 +12,27 @@
 # * ``ClangTidy_VERSION_MINOR`` The clang-tidy minor version
 # * ``ClangTidy_VERSION_PATCH`` The clang-tidy patch version
 
-find_program(ClangTidy_EXECUTABLE
-             NAMES clang-tidy
-                   clang-tidy-8
-                   clang-tidy-7
-                   clang-tidy-6.0
-                   clang-tidy-5.0
-                   clang-tidy-4.0
-                   clang-tidy-3.9
-                   clang-tidy-3.8
-                   clang-tidy-3.7
-                   clang-tidy-3.6
-                   clang-tidy-3.5
-             DOC "clang-tidy executable")
+find_program(
+  ClangTidy_EXECUTABLE
+  NAMES clang-tidy
+        clang-tidy-8
+        clang-tidy-7
+        clang-tidy-6.0
+        clang-tidy-5.0
+        clang-tidy-4.0
+        clang-tidy-3.9
+        clang-tidy-3.8
+        clang-tidy-3.7
+        clang-tidy-3.6
+        clang-tidy-3.5
+  DOC "clang-tidy executable")
 
 # Extract version from command "clang-tidy -version"
 if(ClangTidy_EXECUTABLE)
-  execute_process(COMMAND ${ClangTidy_EXECUTABLE} -version
-                  OUTPUT_VARIABLE full_version_text
-                  ERROR_QUIET OUTPUT_STRIP_TRAILING_WHITESPACE)
+  execute_process(
+    COMMAND ${ClangTidy_EXECUTABLE} -version
+    OUTPUT_VARIABLE full_version_text
+    ERROR_QUIET OUTPUT_STRIP_TRAILING_WHITESPACE)
   # full_version_text sample:
   # ~~~
   # LLVM (http://llvm.org/):
@@ -42,18 +44,12 @@ if(ClangTidy_EXECUTABLE)
   # ~~~
 
   if(full_version_text MATCHES "^LLVM .*")
-    string(REGEX
-           REPLACE ".*LLVM version ([0-9]\.[0-9]\.[0-9]).*"
-                   "\\1"
-                   ClangTidy_VERSION
-                   "${full_version_text}")
+    string(REGEX REPLACE ".*LLVM version ([0-9]\.[0-9]\.[0-9]).*" "\\1" ClangTidy_VERSION
+                         "${full_version_text}")
     # ClangTidy_VERSION sample: "7.0.1"
 
     # Extract version components
-    string(REPLACE "."
-                   ";"
-                   list_versions
-                   "${ClangTidy_VERSION}")
+    string(REPLACE "." ";" list_versions "${ClangTidy_VERSION}")
     list(GET list_versions 0 ClangTidy_VERSION_MAJOR)
     list(GET list_versions 1 ClangTidy_VERSION_MINOR)
     list(GET list_versions 2 ClangTidy_VERSION_PATCH)
@@ -63,10 +59,11 @@ if(ClangTidy_EXECUTABLE)
 endif()
 
 include(FindPackageHandleStandardArgs)
-find_package_handle_standard_args(ClangTidy
-                                  FOUND_VAR
-                                  ClangTidy_FOUND
-                                  REQUIRED_VARS
-                                  ClangTidy_EXECUTABLE
-                                  VERSION_VAR
-                                  ClangTidy_VERSION)
+find_package_handle_standard_args(
+  ClangTidy
+  FOUND_VAR
+  ClangTidy_FOUND
+  REQUIRED_VARS
+  ClangTidy_EXECUTABLE
+  VERSION_VAR
+  ClangTidy_VERSION)
diff --git a/cmake/FindGoogleBenchmark.cmake b/cmake/FindGoogleBenchmark.cmake
@@ -29,14 +29,15 @@ endif()
 # Checks 'REQUIRED', 'QUIET' and versions.
 include(FindPackageHandleStandardArgs)
 
-find_package_handle_standard_args(GoogleBenchmark
-                                  FOUND_VAR
-                                  GoogleBenchmark_FOUND
-                                  REQUIRED_VARS
-                                  GoogleBenchmark_INCLUDE_DIR
-                                  GoogleBenchmark_LIBRARY
-                                  GoogleBenchmark_LIB_DIR
-                                  VERSION_VAR
-                                  GoogleBenchmark_VERSION_STRING)
+find_package_handle_standard_args(
+  GoogleBenchmark
+  FOUND_VAR
+  GoogleBenchmark_FOUND
+  REQUIRED_VARS
+  GoogleBenchmark_INCLUDE_DIR
+  GoogleBenchmark_LIBRARY
+  GoogleBenchmark_LIB_DIR
+  VERSION_VAR
+  GoogleBenchmark_VERSION_STRING)
 
 mark_as_advanced(GoogleBenchmark_INCLUDE_DIR GoogleBenchmark_LIBRARY GoogleBenchmark_LIB_DIR)
diff --git a/cmake/FindRocksDB.cmake b/cmake/FindRocksDB.cmake
@@ -21,41 +21,33 @@ find_library(RocksDB_LIBRARIES NAMES rocksdb)
 if(RocksDB_INCLUDE_DIR AND EXISTS ${RocksDB_INCLUDE_DIR}/rocksdb/version.h)
   file(STRINGS ${RocksDB_INCLUDE_DIR}/rocksdb/version.h rocksdb_version_str
        REGEX "^#define ROCKSDB_MAJOR")
-  string(REGEX
-         REPLACE "^.*ROCKSDB_MAJOR (.*)$"
-                 "\\1"
-                 RocksDB_VERSION_MAJOR
-                 "${rocksdb_version_str}")
+  string(REGEX REPLACE "^.*ROCKSDB_MAJOR (.*)$" "\\1" RocksDB_VERSION_MAJOR
+                       "${rocksdb_version_str}")
 
   file(STRINGS ${RocksDB_INCLUDE_DIR}/rocksdb/version.h rocksdb_version_str
        REGEX "^#define ROCKSDB_MINOR")
-  string(REGEX
-         REPLACE "^.*ROCKSDB_MINOR (.*)$"
-                 "\\1"
-                 RocksDB_VERSION_MINOR
-                 "${rocksdb_version_str}")
+  string(REGEX REPLACE "^.*ROCKSDB_MINOR (.*)$" "\\1" RocksDB_VERSION_MINOR
+                       "${rocksdb_version_str}")
 
   file(STRINGS ${RocksDB_INCLUDE_DIR}/rocksdb/version.h rocksdb_version_str
        REGEX "^#define ROCKSDB_PATCH")
-  string(REGEX
-         REPLACE "^.*ROCKSDB_PATCH (.*)$"
-                 "\\1"
-                 RocksDB_VERSION_PATCH
-                 "${rocksdb_version_str}")
+  string(REGEX REPLACE "^.*ROCKSDB_PATCH (.*)$" "\\1" RocksDB_VERSION_PATCH
+                       "${rocksdb_version_str}")
 
   set(ROCKSDB_VERSION_STRING
       "${RocksDB_VERSION_MAJOR}.${RocksDB_VERSION_MINOR}.${RocksDB_VERSION_PATCH}")
   unset(rocksdb_version_str)
 endif()
 
 include(FindPackageHandleStandardArgs)
-find_package_handle_standard_args(RocksDB
-                                  FOUND_VAR
-                                  RocksDB_FOUND
-                                  REQUIRED_VARS
-                                  RocksDB_LIBRARIES
-                                  RocksDB_INCLUDE_DIR
-                                  VERSION_VAR
-                                  ROCKSDB_VERSION_STRING)
+find_package_handle_standard_args(
+  RocksDB
+  FOUND_VAR
+  RocksDB_FOUND
+  REQUIRED_VARS
+  RocksDB_LIBRARIES
+  RocksDB_INCLUDE_DIR
+  VERSION_VAR
+  ROCKSDB_VERSION_STRING)
 
 mark_as_advanced(RocksDB_INCLUDE_DIR RocksDB_LIBRARIES)
diff --git a/cmake/hpc-coding-conventions b/cmake/hpc-coding-conventions
diff --git a/dev/travis-build b/dev/travis-build
@@ -4,7 +4,7 @@ SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
 TRAVIS_BUILD_DIR="${TRAVIS_BUILD_DIR:-$SCRIPT_DIR/..}"
 
 if [ "x${CPP_CHECK}" != x ] ;then
-  pip install --user --upgrade cmake-format==0.4.5
+  pip install --user --upgrade cmake-format==0.6.0
   export PATH="$HOME/.local/bin:$PATH"
   cmake-format --version
   cd "${TRAVIS_BUILD_DIR}"

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -39,13 +39,14 @@ endif()
 configure_file(basalt/version.hpp.in basalt/version.hpp @ONLY)
 configure_file(basalt/settings.hpp.in basalt/settings.hpp @ONLY)
 
-include_directories(SYSTEM
-                    ${cereal_include_directory}
-                    ${gsl_include_directory}
-                    ${pybind11_include_directory}
-                    ${RocksDB_INCLUDE_DIR}
-                    ${spdlog_include_directory}
-                    ${nlohmann_include_directory})
+include_directories(
+  SYSTEM
+  ${cereal_include_directory}
+  ${gsl_include_directory}
+  ${pybind11_include_directory}
+  ${RocksDB_INCLUDE_DIR}
+  ${spdlog_include_directory}
+  ${nlohmann_include_directory})
 
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/basalt {CMAKE_CURRENT_SOURCE_DIR}/python_bindings
                     ${CMAKE_CURRENT_BINARY_DIR})

diff --git a/src/basalt/config.cpp b/src/basalt/config.cpp
@@ -87,13 +87,6 @@ compression_if_present(const nlohmann::json& config) {
     return nullptr;
 }
 
-/**
- * Convert rocksdb status object to a basalt one
- */
-static inline Status to_status(const rocksdb::Status& status) {
-    return {static_cast<Status::Code>(status.code()), status.ToString()};
-}
-
 /**
  * Get real column family name from one specified in the JSON config
  */
@@ -295,40 +288,6 @@ static rocksdb::ColumnFamilyOptions column_families_options(
     return result;
 }
 
-/**
- * Prepare the rocksdb column families on the filesystem if they do not already exist.
- */
-static void create_columns_families(const nlohmann::json& config,
-                                    const std::string& db_path,
-                                    rocksdb::Options& options,
-                                    std::shared_ptr<rocksdb::Cache> global_block_cache) {
-    std::unique_ptr<rocksdb::DB> db_ptr;
-    {  // open db
-        rocksdb::DB* db;
-        rocksdb::Status status = rocksdb::DB::Open(options, db_path, &db);
-        db_ptr.reset(db);
-        if (status.code() == rocksdb::Status::kInvalidArgument) {
-            return;
-        }
-        to_status(status).raise_on_error();
-    }
-
-    std::vector<std::string> column_family_names;
-    rocksdb::DB::ListColumnFamilies(options, db_ptr->GetName(), &column_family_names);
-
-    for (auto const& cf_config: config) {
-        auto const& name = column_family_name(cf_config["name"]);
-        if (std::find(column_family_names.begin(), column_family_names.end(), name) ==
-            column_family_names.end()) {
-            gsl::owner<rocksdb::ColumnFamilyHandle*> cf = nullptr;
-            auto const& cf_options = column_families_options(cf_config["config"],
-                                                             global_block_cache);
-            to_status(db_ptr->CreateColumnFamily(cf_options, name, &cf)).raise_on_error();
-            delete cf;
-        }
-    }
-}
-
 /**
  * Setup rocksdb statistics according to JSON config
  */
@@ -352,9 +311,18 @@ static void setup_max_open_files(const nlohmann::json& config, rocksdb::Options&
 }
 
 static void setup_create_if_missing(const nlohmann::json& config, rocksdb::Options& options) {
-    auto const& value = config.find("create_if_missing");
-    if (value != config.end()) {
-        options.create_if_missing = value.value().get<decltype(options.create_if_missing)>();
+    {
+        auto const& value = config.find("create_if_missing");
+        if (value != config.end()) {
+            options.create_if_missing = value.value().get<decltype(options.create_if_missing)>();
+        }
+    }
+    {
+        auto const& value = config.find("create_missing_column_families");
+        if (value != config.end()) {
+            options.create_missing_column_families =
+                value.value().get<decltype(options.create_missing_column_families)>();
+        }
     }
 }
 
@@ -375,6 +343,7 @@ static nlohmann::json default_json() {
     config["statistics"] = true;
     config["max_open_files"] = -1;
     config["create_if_missing"] = true;
+    config["create_missing_column_families"] = true;
     // clang-format off
     config["block_cache"] = {
         {"type", "lru"},
@@ -483,15 +452,14 @@ Config::Config()
 Config::Config(std::ifstream& istr)
     : config_(from_stream(istr)) {}
 
-void Config::configure(rocksdb::Options& options, const std::string& db_path) const {
+void Config::configure(rocksdb::Options& options) const {
     setup_statistics(config_, options);
     setup_max_open_files(config_, options);
     setup_create_if_missing(config_, options);
     setup_compression(config_, options);
 
     std::shared_ptr<rocksdb::Cache> empty;
     auto global_block_cache = block_cache_if_present(config_, empty);
-    create_columns_families(config_["column_families"], db_path, options, global_block_cache);
 }
 
 std::ostream& Config::write(std::ostream& ostr, std::streamsize indent) const {