Просмотр исходного кода

Add comments to explain safe index accesses.

Add comments to explain why log index access is safe in several places
where it is not obvious from the context. Also explained when the
snapshot index and next sync index could move backwards.
Jing Yang 4 лет назад
Родитель
Сommit
6401d7082b
4 измененных файлов с 11 добавлено и 2 удалено
  1. 3 0
      src/apply_command.rs
  2. 3 2
      src/process_install_snapshot.rs
  3. 3 0
      src/snapshot.rs
  4. 2 0
      src/sync_log_entries.rs

+ 3 - 0
src/apply_command.rs

@@ -57,6 +57,9 @@ where
                     } else if rf.last_applied < rf.commit_index {
                     } else if rf.last_applied < rf.commit_index {
                         let index = rf.last_applied + 1;
                         let index = rf.last_applied + 1;
                         let last_one = rf.commit_index + 1;
                         let last_one = rf.commit_index + 1;
+                        // This is safe because commit_index is always smaller
+                        // than log.end(), see COMMIT_INDEX_INVARIANT.
+                        assert!(last_one <= rf.log.end());
                         let messages: Vec<ApplyCommandMessage<Command>> = rf
                         let messages: Vec<ApplyCommandMessage<Command>> = rf
                             .log
                             .log
                             .between(index, last_one)
                             .between(index, last_one)

+ 3 - 2
src/process_install_snapshot.rs

@@ -38,8 +38,9 @@ impl<C: Clone + Default + serde::Serialize> Raft<C> {
         // The snapshot could not be verified because the index is beyond log
         // The snapshot could not be verified because the index is beyond log
         // start. Fail this request and ask leader to send something that we
         // start. Fail this request and ask leader to send something that we
         // could verify. We cannot rollback to a point beyond commit index
         // could verify. We cannot rollback to a point beyond commit index
-        // anyway. Otherwise if the system fails right after the rollback,
-        // committed entries before log start would be lost forever.
+        // anyway, let alone rolling back before the log start(). If we rollback
+        // and the system fails right after the rollback, committed entries
+        // before log start would be lost forever.
         //
         //
         // The commit index is sent back to leader. The leader would never need
         // The commit index is sent back to leader. The leader would never need
         // to rollback beyond that, since it is guaranteed that committed log
         // to rollback beyond that, since it is guaranteed that committed log

+ 3 - 0
src/snapshot.rs

@@ -27,6 +27,9 @@ impl<T: 'static + Send + FnMut(Index)> RequestSnapshotFnMut for T {}
 impl SnapshotDaemon {
 impl SnapshotDaemon {
     pub(crate) fn save_snapshot(&self, snapshot: Snapshot) {
     pub(crate) fn save_snapshot(&self, snapshot: Snapshot) {
         let mut curr = self.current_snapshot.0.lock();
         let mut curr = self.current_snapshot.0.lock();
+        // The new snapshot can have a last_included_index that is smaller than
+        // the current snapshot, if this instance is a follower and the leader
+        // has installed a new snapshot on it.
         if curr.last_included_index < snapshot.last_included_index {
         if curr.last_included_index < snapshot.last_included_index {
             *curr = snapshot;
             *curr = snapshot;
         }
         }

+ 2 - 0
src/sync_log_entries.rs

@@ -173,6 +173,8 @@ where
                 Self::check_committed(&rf, peer, committed.clone());
                 Self::check_committed(&rf, peer, committed.clone());
 
 
                 rf.current_step[peer_index] = 0;
                 rf.current_step[peer_index] = 0;
+                // Next index moves towards the log end. This is the only place
+                // where that happens.
                 rf.next_index[peer_index] = committed.index;
                 rf.next_index[peer_index] = committed.index;
 
 
                 // Ignore the error. The log syncing thread must have died.
                 // Ignore the error. The log syncing thread must have died.