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/09/03 12:22:49 UTC

[GitHub] [incubator-pegasus] zhangyifan27 opened a new pull request #593: fix: Use default options to open db when latest option file has uncompatible db options

zhangyifan27 opened a new pull request #593:
URL: https://github.com/apache/incubator-pegasus/pull/593


   #What problem does this PR solve? <!--add issue link with summary if exists-->
   When we reopen an existing db, we will use latest option file to construct options. However,
   if this file is an old version file which has uncompatible options with official rocksdb, we would
   get an error and fail to open db.
   https://github.com/apache/incubator-pegasus/blob/ae7caa2599357f83cbbf79c4367a5d0a72f04288/src/server/pegasus_server_impl.cpp#L1341-L1349
   
   ### What is changed and how it works?
   If the option file has known uncompatible options `pegasus_data_version` and `pegasus_data`,
   we skip loading options from file and use default options to open db.
   A new version option file will be generated when we succeed to open db, after that we could use
   existing option file to construct options. That means the first time we open an old version db, we
   must use default options, and that may lead to unexpected flushes and compactions.
   
   ### Check List <!--REMOVE the items that are not applicable-->
   
   Tests <!-- At least one of them must be included. -->
   
   - Manual test (add detailed scripts or steps below)
   1. deploy an old version cluster.
   2. create some old version replicas.
   3. upgrade replica servers to the new version with #587 and this patch.
   4. test if new version servers could start apps sccessfully.
   
   Code changes
   
   - Has exported function/method change
   
   Side effects
   
   - Increased code complexity
   
   Related changes
   
   #587 
   - Need to cherry-pick to the release branch
   


----------------------------------------------------------------
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


[GitHub] [incubator-pegasus] neverchanje commented on a change in pull request #593: fix: Use default options to open db when latest option file has uncompatible db options

Posted by GitBox <gi...@apache.org>.
neverchanje commented on a change in pull request #593:
URL: https://github.com/apache/incubator-pegasus/pull/593#discussion_r483347124



##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1330,9 +1330,18 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
     // 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;
+    bool has_uncompatible_db_options = false;

Review comment:
       `uncompatible` is misspelled. The correct one is `incompatible`.




----------------------------------------------------------------
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


[GitHub] [incubator-pegasus] hycdong merged pull request #593: fix: Use default options to open db when latest option file has incompatible db options

Posted by GitBox <gi...@apache.org>.
hycdong merged pull request #593:
URL: https://github.com/apache/incubator-pegasus/pull/593


   


----------------------------------------------------------------
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


[GitHub] [incubator-pegasus] neverchanje commented on a change in pull request #593: fix: Use default options to open db when latest option file has incompatible db options

Posted by GitBox <gi...@apache.org>.
neverchanje commented on a change in pull request #593:
URL: https://github.com/apache/incubator-pegasus/pull/593#discussion_r483464994



##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1344,28 +1353,28 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
                                                  &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("unknown column family name.");
+            if (status.ToString().find("pegasus") == std::string::npos) {
+                derror_replica("load latest option file failed: {}.", status.ToString());
                 return ::dsn::ERR_LOCAL_APP_FAILURE;
             }

Review comment:
       ```suggestion
               if (status.code() != rocksdb::Status::kInvalidArgument || status.ToString().find(" Unrecognized option") == std::string::npos) {
                   derror_replica("load latest option file failed: {}.", status.ToString());
                   return ::dsn::ERR_LOCAL_APP_FAILURE;
               }
   ```

##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1344,28 +1353,28 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
                                                  &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("unknown column family name.");
+            if (status.ToString().find("pegasus") == std::string::npos) {
+                derror_replica("load latest option file failed: {}.", status.ToString());
                 return ::dsn::ERR_LOCAL_APP_FAILURE;
             }

Review comment:
       ```suggestion
               if (status.code() != rocksdb::Status::kInvalidArgument || status.ToString().find("Unrecognized option") == std::string::npos) {
                   derror_replica("load latest option file failed: {}.", status.ToString());
                   return ::dsn::ERR_LOCAL_APP_FAILURE;
               }
   ```




----------------------------------------------------------------
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


[GitHub] [incubator-pegasus] neverchanje commented on a change in pull request #593: fix: Use default options to open db when latest option file has incompatible db options

Posted by GitBox <gi...@apache.org>.
neverchanje commented on a change in pull request #593:
URL: https://github.com/apache/incubator-pegasus/pull/593#discussion_r483464994



##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1344,28 +1353,28 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
                                                  &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("unknown column family name.");
+            if (status.ToString().find("pegasus") == std::string::npos) {
+                derror_replica("load latest option file failed: {}.", status.ToString());
                 return ::dsn::ERR_LOCAL_APP_FAILURE;
             }

Review comment:
       ```suggestion
               if (status.code() != rocksdb::Status::kInvalidArgument || status.ToString().find(" Unrecognized DBOptions") == std::string::npos) {
                   derror_replica("load latest option file failed: {}.", status.ToString());
                   return ::dsn::ERR_LOCAL_APP_FAILURE;
               }
   ```




----------------------------------------------------------------
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