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 2022/10/27 06:39:50 UTC

[GitHub] [incubator-pegasus] empiredan opened a new pull request, #1208: Fix mutation log reset from

empiredan opened a new pull request, #1208:
URL: https://github.com/apache/incubator-pegasus/pull/1208

   This PR is to fix https://github.com/apache/incubator-pegasus/issues/1207.


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

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

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] acelyc111 commented on a diff in pull request #1208: fix: fix rollback by mistake for mutation_log::reset_from

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on code in PR #1208:
URL: https://github.com/apache/incubator-pegasus/pull/1208#discussion_r1007079430


##########
src/replica/mutation_log.cpp:
##########
@@ -939,58 +939,68 @@ error_code mutation_log::reset_from(const std::string &dir,
                                     replay_callback replay_error_callback,
                                     io_failure_callback write_error_callback)
 {
-    error_code err = ERR_FILE_OPERATION_FAILED;
-
-    // close for flushing current log and be ready to open new log files after reset
+    // Close for flushing current log and get ready to open new log files after reset.
     close();
 
-    // make sure logs in `dir` (such as /learn) are valid.
+    // Ensure that log files in `dir` (such as "/learn") are valid.
     error_s es = log_utils::check_log_files_continuity(dir);
     if (!es.is_ok()) {
-        LOG_ERROR_F("the log of source dir {} is invalid:{}, will remove it.", dir, es);
+        LOG_ERROR_F("the log files of source dir {} are invalid: {}, will remove it", dir, es);
         if (!utils::filesystem::remove_path(dir)) {

Review Comment:
   If remove file failed, there maybe some terrible error of the disk, would it better to mark this disk as error?
   We can leave an issue for it and deal with it in the future.



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

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

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] empiredan commented on a diff in pull request #1208: fix: fix rollback by mistake for mutation_log::reset_from

Posted by GitBox <gi...@apache.org>.
empiredan commented on code in PR #1208:
URL: https://github.com/apache/incubator-pegasus/pull/1208#discussion_r1007603598


##########
src/replica/mutation_log.cpp:
##########
@@ -939,58 +939,68 @@ error_code mutation_log::reset_from(const std::string &dir,
                                     replay_callback replay_error_callback,
                                     io_failure_callback write_error_callback)
 {
-    error_code err = ERR_FILE_OPERATION_FAILED;
-
-    // close for flushing current log and be ready to open new log files after reset
+    // Close for flushing current log and get ready to open new log files after reset.
     close();
 
-    // make sure logs in `dir` (such as /learn) are valid.
+    // Ensure that log files in `dir` (such as "/learn") are valid.
     error_s es = log_utils::check_log_files_continuity(dir);
     if (!es.is_ok()) {
-        LOG_ERROR_F("the log of source dir {} is invalid:{}, will remove it.", dir, es);
+        LOG_ERROR_F("the log files of source dir {} are invalid: {}, will remove it", dir, es);
         if (!utils::filesystem::remove_path(dir)) {

Review Comment:
   OK, I'll launch another issue to track this problem.



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

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

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] empiredan merged pull request #1208: fix: fix rollback by mistake for mutation_log::reset_from

Posted by GitBox <gi...@apache.org>.
empiredan merged PR #1208:
URL: https://github.com/apache/incubator-pegasus/pull/1208


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

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

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