acelyc111 commented on code in PR #2068:
URL: 
https://github.com/apache/incubator-pegasus/pull/2068#discussion_r1773077588


##########
src/replica/duplication/load_from_private_log.cpp:
##########
@@ -156,14 +157,32 @@ void load_from_private_log::find_log_file_to_start()
     // Reopen the files. Because the internal file handle of `file_map`
     // is cleared once WAL replay finished. They are unable to read.
     mutation_log::log_file_map_by_index new_file_map;
-    for (const auto &pr : file_map) {
+    decree cleanable_decree = _private_log->get_cleanable_decree();
+    decree max_decree_gpid = _private_log->max_decree(get_gpid());
+    if (max_decree_gpid <= cleanable_decree) {
+        LOG_ERROR_PREFIX("plog_file all error: max_decree_gpid {} , 
cleanable_decree {}",
+                         max_decree_gpid,
+                         cleanable_decree);
+        return;
+    }
+
+    for (auto it = file_map.rbegin(); it != file_map.rend(); ++it) {
         log_file_ptr file;
-        error_s es = log_utils::open_read(pr.second->path(), file);
+        error_s es = log_utils::open_read(it->second->path(), file);
         if (!es.is_ok()) {
             LOG_ERROR_PREFIX("{}", es);
             return;
         }
-        new_file_map.emplace(pr.first, file);
+        new_file_map.emplace(it->first, file);
+
+        // next file map may can not open

Review Comment:
   Upper the first letter and add a dot at the end. Other places are the same.
   
   And which code line does this comment for?



##########
src/replica/duplication/load_from_private_log.cpp:
##########
@@ -156,14 +157,32 @@ void load_from_private_log::find_log_file_to_start()
     // Reopen the files. Because the internal file handle of `file_map`
     // is cleared once WAL replay finished. They are unable to read.
     mutation_log::log_file_map_by_index new_file_map;
-    for (const auto &pr : file_map) {
+    decree cleanable_decree = _private_log->get_cleanable_decree();
+    decree max_decree_gpid = _private_log->max_decree(get_gpid());
+    if (max_decree_gpid <= cleanable_decree) {
+        LOG_ERROR_PREFIX("plog_file all error: max_decree_gpid {} , 
cleanable_decree {}",
+                         max_decree_gpid,
+                         cleanable_decree);
+        return;
+    }
+
+    for (auto it = file_map.rbegin(); it != file_map.rend(); ++it) {
         log_file_ptr file;
-        error_s es = log_utils::open_read(pr.second->path(), file);
+        error_s es = log_utils::open_read(it->second->path(), file);
         if (!es.is_ok()) {
             LOG_ERROR_PREFIX("{}", es);
             return;
         }
-        new_file_map.emplace(pr.first, file);
+        new_file_map.emplace(it->first, file);
+
+        // next file map may can not open
+        gpid pid = get_gpid();
+        decree previous_log_max_decree = file->previous_log_max_decree(pid);
+        // these plog_file has possible be deleted do not open_read next 
plog_file , otherwise it

Review Comment:
   What is `plog_file`? Why you add a underline?



##########
src/replica/duplication/load_from_private_log.cpp:
##########
@@ -156,14 +157,32 @@ void load_from_private_log::find_log_file_to_start()
     // Reopen the files. Because the internal file handle of `file_map`
     // is cleared once WAL replay finished. They are unable to read.
     mutation_log::log_file_map_by_index new_file_map;
-    for (const auto &pr : file_map) {
+    decree cleanable_decree = _private_log->get_cleanable_decree();
+    decree max_decree_gpid = _private_log->max_decree(get_gpid());
+    if (max_decree_gpid <= cleanable_decree) {
+        LOG_ERROR_PREFIX("plog_file all error: max_decree_gpid {} , 
cleanable_decree {}",
+                         max_decree_gpid,
+                         cleanable_decree);
+        return;
+    }
+
+    for (auto it = file_map.rbegin(); it != file_map.rend(); ++it) {

Review Comment:
   Use const reference?



##########
src/replica/duplication/load_from_private_log.cpp:
##########
@@ -156,14 +157,32 @@ void load_from_private_log::find_log_file_to_start()
     // Reopen the files. Because the internal file handle of `file_map`
     // is cleared once WAL replay finished. They are unable to read.
     mutation_log::log_file_map_by_index new_file_map;
-    for (const auto &pr : file_map) {
+    decree cleanable_decree = _private_log->get_cleanable_decree();
+    decree max_decree_gpid = _private_log->max_decree(get_gpid());
+    if (max_decree_gpid <= cleanable_decree) {

Review Comment:
   Is it possible? `CHECK` it if it's impossible.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to