code reading notes from last weekend

docs/storage_broker.md

@@ -1,6 +1,6 @@
 # Storage broker
 
-Storage broker targets two issues:
+Storage broker targets two issues
 - Allowing safekeepers and pageservers learn which nodes also hold their
   timelines, and timeline statuses there.
 - Avoiding O(n^2) connections between storage nodes while doing so.
@@ -19,7 +19,7 @@ Currently, the only message is `SafekeeperTimelineInfo`. Each safekeeper, for
 each active timeline, once in a while pushes timeline status to the broker.
 Other nodes subscribe and receive this info, using it per above.
 
-Broker serves /metrics on the same port as grpc service. 
+Broker serves /metrics on the same port as grpc service.
 
 grpcurl can be used to check which values are currently being pushed:
 ```

safekeeper/src/timeline_manager.rs

@@ -495,6 +495,7 @@ impl Manager {
     }
 
     /// Update is_active flag and returns its value.
+    // Timelines marked active are pushed to the broker by the `push_loop` task.
     fn update_is_active(
         &mut self,
         is_wal_backup_required: bool,

safekeeper/src/wal_backup.rs

@@ -61,7 +61,9 @@ pub(crate) fn is_wal_backup_required(
     state: &StateSnapshot,
 ) -> bool {
     num_computes > 0 ||
-    // Currently only the whole segment is offloaded, so compare segment numbers.
+    // This task backups completed segments only.
+    // The current partial segment is backed up by a separate task/code module (wal_backup_partial).
+    // So, need for completed segment backup <=> last backup was at at older segment.
     (state.commit_lsn.segment_number(wal_seg_size) > state.backup_lsn.segment_number(wal_seg_size))
 }
 
@@ -69,6 +71,11 @@ pub(crate) fn is_wal_backup_required(
 /// is me, run (per timeline) task, if not yet. OTOH, if it is not me and task
 /// is running, kill it.
 pub(crate) async fn update_task(mgr: &mut Manager, need_backup: bool, state: &StateSnapshot) {
+    // Based on the peer information received from broker, each safekeeper figures out
+    // whether it, or one of the peers, is the offloader.
+    // The algorithm is deterministic, so, if all peers have the same information,
+    // the system converges. In unconverged state, multiple peers upload the same
+    // segments, which is inefficient but safe.
     let (offloader, election_dbg_str) =
         determine_offloader(&state.peers, state.backup_lsn, mgr.tli.ttid, &mgr.conf);
     let elected_me = Some(mgr.conf.my_id) == offloader;