You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2020/08/28 03:59:58 UTC

[GitHub] [incubator-pegasus] neverchanje commented on a change in pull request #587: fix: Load options from file when open an exist DB

neverchanje commented on a change in pull request #587:
URL: https://github.com/apache/incubator-pegasus/pull/587#discussion_r478809519



##########
File path: src/server/meta_store.cpp
##########
@@ -72,18 +84,38 @@ ::dsn::error_code meta_store::get_value_from_meta_cf(rocksdb::DB *db,
                                                      uint64_t *value)
 {
     std::string data;
+    auto ec = get_string_value_from_meta_cf(db, cf, read_flushed_data, key, &data);
+    if (ec != ::dsn::ERR_OK) {
+        return ec;
+    }
+    dassert_f(dsn::buf2uint64(data, *value),
+              "rocksdb {} get {} from meta column family got error value {}",

Review comment:
       ```suggestion
                 "rocksdb {} get \"{}\" from meta column family failed to parse into uint64",
   ```
   
   I think you cannot simply print a value out, there could be unprintable characters.

##########
File path: src/server/meta_store.cpp
##########
@@ -58,6 +58,18 @@ uint64_t meta_store::get_decree_from_readonly_db(rocksdb::DB *db,
     return last_flushed_decree;
 }
 
+std::string meta_store::get_usage_scenario() const
+{
+    // If couldn't find rocksdb usage scenario in meta column family, return normal in default.
+    std::string usage_scenario = ROCKSDB_ENV_USAGE_SCENARIO_NORMAL;
+    auto ec = get_string_value_from_meta_cf(false, ROCKSDB_ENV_USAGE_SCENARIO_KEY, &usage_scenario);
+    dassert_replica(ec == ::dsn::ERR_OK || ec == ::dsn::ERR_OBJECT_NOT_FOUND,
+                    "rocksdb {} get {} from meta column family failed",
+                    _db->GetName(),
+                    ROCKSDB_ENV_USAGE_SCENARIO_KEY);

Review comment:
       ```suggestion
       dassert_replica(ec == ::dsn::ERR_OK || ec == ::dsn::ERR_OBJECT_NOT_FOUND,
                       "rocksdb {} get {} from meta column family failed: {}",
                       _db->GetName(),
                       ROCKSDB_ENV_USAGE_SCENARIO_KEY,
                       ec.to_string());
   ```

##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1326,22 +1327,59 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
 
     ddebug("%s: start to open rocksDB's rdb(%s)", replica_name(), path.c_str());
 
+    // Here we create a `tmp_data_cf_opts` because we don't want to modify `_data_cf_opts`, which
+    // will be used elsewhere.
+    rocksdb::ColumnFamilyOptions tmp_data_cf_opts = _data_cf_opts;
     if (db_exist) {
-        // When DB exist, meta CF must be present.
         bool missing_meta_cf = true;
-        if (check_meta_cf(path, &missing_meta_cf) != ::dsn::ERR_OK) {
-            derror_replica("check meta column family failed");
+        bool missing_data_cf = true;
+        // Load latest options from option file stored in the db directory.
+        rocksdb::DBOptions loaded_db_opt;
+        std::vector<rocksdb::ColumnFamilyDescriptor> loaded_cf_descs;
+        rocksdb::ColumnFamilyOptions loaded_data_cf_opts;
+        // Set `ignore_unknown_options` true for forward compatibility.
+        auto status = rocksdb::LoadLatestOptions(path,
+                                                 rocksdb::Env::Default(),
+                                                 &loaded_db_opt,
+                                                 &loaded_cf_descs,
+                                                 /*ignore_unknown_options*/ true);
+        if (!status.ok()) {
+            derror_replica("load latest option file failed.");
             return ::dsn::ERR_LOCAL_APP_FAILURE;
         }
+        for (int i = 0; i < loaded_cf_descs.size(); ++i) {
+            if (loaded_cf_descs[i].name == META_COLUMN_FAMILY_NAME) {
+                missing_meta_cf = false;
+            } else if (loaded_cf_descs[i].name == DATA_COLUMN_FAMILY_NAME) {
+                missing_data_cf = false;
+                loaded_data_cf_opts = loaded_cf_descs[i].options;
+            } else {
+                derror_replica("unkown column family name.");

Review comment:
       ```suggestion
                   derror_replica("unknown column family name.");
   ```

##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1326,22 +1327,59 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
 
     ddebug("%s: start to open rocksDB's rdb(%s)", replica_name(), path.c_str());
 
+    // Here we create a `tmp_data_cf_opts` because we don't want to modify `_data_cf_opts`, which
+    // will be used elsewhere.
+    rocksdb::ColumnFamilyOptions tmp_data_cf_opts = _data_cf_opts;
     if (db_exist) {
-        // When DB exist, meta CF must be present.
         bool missing_meta_cf = true;
-        if (check_meta_cf(path, &missing_meta_cf) != ::dsn::ERR_OK) {
-            derror_replica("check meta column family failed");
+        bool missing_data_cf = true;
+        // Load latest options from option file stored in the db directory.
+        rocksdb::DBOptions loaded_db_opt;
+        std::vector<rocksdb::ColumnFamilyDescriptor> loaded_cf_descs;
+        rocksdb::ColumnFamilyOptions loaded_data_cf_opts;
+        // Set `ignore_unknown_options` true for forward compatibility.
+        auto status = rocksdb::LoadLatestOptions(path,
+                                                 rocksdb::Env::Default(),
+                                                 &loaded_db_opt,
+                                                 &loaded_cf_descs,
+                                                 /*ignore_unknown_options*/ true);
+        if (!status.ok()) {
+            derror_replica("load latest option file failed.");
             return ::dsn::ERR_LOCAL_APP_FAILURE;
         }
+        for (int i = 0; i < loaded_cf_descs.size(); ++i) {
+            if (loaded_cf_descs[i].name == META_COLUMN_FAMILY_NAME) {
+                missing_meta_cf = false;
+            } else if (loaded_cf_descs[i].name == DATA_COLUMN_FAMILY_NAME) {
+                missing_data_cf = false;
+                loaded_data_cf_opts = loaded_cf_descs[i].options;
+            } else {
+                derror_replica("unkown column family name.");
+                return ::dsn::ERR_LOCAL_APP_FAILURE;
+            }
+        }
+        // When DB exist, meta CF and default CF must be present.
         dassert_replica(!missing_meta_cf, "You must upgrade Pegasus server from 2.0");
+        dassert_replica(!missing_data_cf, "Missing default column family");

Review comment:
       ```suggestion
           // When DB exists, meta CF and data CF must be present.
           dassert_replica(!missing_meta_cf, "You must upgrade Pegasus server from 2.0"
           dassert_replica(!missing_data_cf, "Missing data column family");
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org