Merge branch 'dirvote_act_refactor_v2_squashed'

changes/ticket25937

@@ -0,0 +1,9 @@
+  o Minor features (mainloop):
+    - Move responsibility for
+      consensus voting
+      from a once-per-second callback to a callback that is only scheduled as
+      needed.  Once enough items are removed from our once-per-second
+      callback, we can eliminate it entirely to conserve CPU when idle.
+      Closes ticket
+      25937.
+

src/or/config.c

@@ -1,3 +1,4 @@
+
 /* Copyright (c) 2001 Matej Pfajfar.
  * Copyright (c) 2001-2004, Roger Dingledine.
  * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
@@ -746,6 +747,8 @@ static int options_transition_affects_workers(
       const or_options_t *old_options, const or_options_t *new_options);
 static int options_transition_affects_descriptor(
       const or_options_t *old_options, const or_options_t *new_options);
+static int options_transition_affects_dirauth_timing(
+      const or_options_t *old_options, const or_options_t *new_options);
 static int normalize_nickname_list(config_line_t **normalized_out,
                                    const config_line_t *lst, const char *name,
                                    char **msg);
@@ -1745,6 +1748,32 @@ options_transition_affects_guards(const or_options_t *old_options,
   return 0;
 }
 
+/**
+ * Return true if changing the configuration from <b>old</b> to <b>new</b>
+ * affects the timing of the voting subsystem
+ */
+static int
+options_transition_affects_dirauth_timing(const or_options_t *old_options,
+                                          const or_options_t *new_options)
+{
+  tor_assert(old_options);
+  tor_assert(new_options);
+
+  if (authdir_mode_v3(old_options) != authdir_mode_v3(new_options))
+    return 1;
+  if (! authdir_mode_v3(new_options))
+    return 0;
+  YES_IF_CHANGED_INT(V3AuthVotingInterval);
+  YES_IF_CHANGED_INT(V3AuthVoteDelay);
+  YES_IF_CHANGED_INT(V3AuthDistDelay);
+  YES_IF_CHANGED_INT(TestingV3AuthInitialVotingInterval);
+  YES_IF_CHANGED_INT(TestingV3AuthInitialVoteDelay);
+  YES_IF_CHANGED_INT(TestingV3AuthInitialDistDelay);
+  YES_IF_CHANGED_INT(TestingV3AuthVotingStartOffset);
+
+  return 0;
+}
+
 /** Fetch the active option list, and take actions based on it. All of the
  * things we do should survive being done repeatedly.  If present,
  * <b>old_options</b> contains the previous value of the options.
@@ -2329,8 +2358,10 @@ options_act(const or_options_t *old_options)
 
   /* We may need to reschedule some directory stuff if our status changed. */
   if (old_options) {
-    if (authdir_mode_v3(options) && !authdir_mode_v3(old_options))
+    if (options_transition_affects_dirauth_timing(old_options, options)) {
       dirvote_recalculate_timing(options, time(NULL));
+      reschedule_dirvote(options);
+    }
     if (!bool_eq(directory_fetches_dir_info_early(options),
                  directory_fetches_dir_info_early(old_options)) ||
         !bool_eq(directory_fetches_dir_info_later(options),

src/or/dirauth/dirvote.c

@@ -2729,12 +2729,16 @@ get_detached_signatures_from_pending_consensuses(pending_consensus_t *pending,
   return signatures;
 }
 
-/** Entry point: Take whatever voting actions are pending as of <b>now</b>. */
-void
+/**
+ * Entry point: Take whatever voting actions are pending as of <b>now</b>.
+ *
+ * Return the time at which the next action should be taken.
+ */
+time_t
 dirvote_act(const or_options_t *options, time_t now)
 {
   if (!authdir_mode_v3(options))
-    return;
+    return TIME_MAX;
   tor_assert_nonfatal(voting_schedule.voting_starts);
   /* If we haven't initialized this object through this codeflow, we need to
    * recalculate the timings to match our vote. The reason to do that is if we
@@ -2750,33 +2754,41 @@ dirvote_act(const or_options_t *options, time_t now)
     tor_free(keys);
     dirvote_recalculate_timing(options, now);
   }
-  if (voting_schedule.voting_starts < now && !voting_schedule.have_voted) {
+
+#define IF_TIME_FOR_NEXT_ACTION(when_field, done_field) \
+  if (! voting_schedule.done_field) {                   \
+    if (voting_schedule.when_field > now) {             \
+      return voting_schedule.when_field;                \
+    } else {
+#define ENDIF \
+    }           \
+  }
+
+  IF_TIME_FOR_NEXT_ACTION(voting_starts, have_voted) {
     log_notice(LD_DIR, "Time to vote.");
     dirvote_perform_vote();
     voting_schedule.have_voted = 1;
-  }
-  if (voting_schedule.fetch_missing_votes < now &&
-      !voting_schedule.have_fetched_missing_votes) {
+  } ENDIF
+  IF_TIME_FOR_NEXT_ACTION(fetch_missing_votes, have_fetched_missing_votes) {
     log_notice(LD_DIR, "Time to fetch any votes that we're missing.");
     dirvote_fetch_missing_votes();
     voting_schedule.have_fetched_missing_votes = 1;
-  }
-  if (voting_schedule.voting_ends < now &&
-      !voting_schedule.have_built_consensus) {
+  } ENDIF
+  IF_TIME_FOR_NEXT_ACTION(voting_ends, have_built_consensus) {
     log_notice(LD_DIR, "Time to compute a consensus.");
     dirvote_compute_consensuses();
     /* XXXX We will want to try again later if we haven't got enough
      * votes yet.  Implement this if it turns out to ever happen. */
     voting_schedule.have_built_consensus = 1;
-  }
-  if (voting_schedule.fetch_missing_signatures < now &&
-      !voting_schedule.have_fetched_missing_signatures) {
+  } ENDIF
+  IF_TIME_FOR_NEXT_ACTION(fetch_missing_signatures,
+                          have_fetched_missing_signatures) {
     log_notice(LD_DIR, "Time to fetch any signatures that we're missing.");
     dirvote_fetch_missing_signatures();
     voting_schedule.have_fetched_missing_signatures = 1;
-  }
-  if (voting_schedule.interval_starts < now &&
-      !voting_schedule.have_published_consensus) {
+  } ENDIF
+  IF_TIME_FOR_NEXT_ACTION(interval_starts,
+                          have_published_consensus) {
     log_notice(LD_DIR, "Time to publish the consensus and discard old votes");
     dirvote_publish_consensus();
     dirvote_clear_votes(0);
@@ -2787,7 +2799,14 @@ dirvote_act(const or_options_t *options, time_t now)
     /* XXXX We will want to try again later if we haven't got enough
      * signatures yet.  Implement this if it turns out to ever happen. */
     dirvote_recalculate_timing(options, now);
-  }
+    return voting_schedule.voting_starts;
+  } ENDIF
+
+  tor_assert_nonfatal_unreached();
+  return now + 1;
+
+#undef ENDIF
+#undef IF_TIME_FOR_NEXT_ACTION
 }
 
 /** A vote networkstatus_t and its unparsed body: held around so we can

src/or/dirauth/dirvote.h

@@ -96,7 +96,7 @@
  */
 #ifdef HAVE_MODULE_DIRAUTH
 
-void dirvote_act(const or_options_t *options, time_t now);
+time_t dirvote_act(const or_options_t *options, time_t now);
 void dirvote_free_all(void);
 
 void dirvote_parse_sr_commits(networkstatus_t *ns, smartlist_t *tokens);
@@ -114,11 +114,12 @@ int dirvote_add_signatures(const char *detached_signatures_body,
 
 #else /* HAVE_MODULE_DIRAUTH */
 
-static inline void
+static inline time_t
 dirvote_act(const or_options_t *options, time_t now)
 {
   (void) options;
   (void) now;
+  return TIME_MAX;
 }
 
 static inline void

src/or/main.c

@@ -1341,6 +1341,7 @@ CALLBACK(check_for_reachability_bw);
 CALLBACK(check_onion_keys_expiry_time);
 CALLBACK(clean_caches);
 CALLBACK(clean_consdiffmgr);
+CALLBACK(dirvote);
 CALLBACK(downrate_stability);
 CALLBACK(expire_old_ciruits_serverside);
 CALLBACK(fetch_networkstatus);
@@ -1402,6 +1403,7 @@ STATIC periodic_event_item_t periodic_events[] = {
 
   /* Directory authority only. */
   CALLBACK(check_authority_cert, PERIODIC_EVENT_ROLE_DIRAUTH, 0),
+  CALLBACK(dirvote, PERIODIC_EVENT_ROLE_DIRAUTH, PERIODIC_EVENT_FLAG_NEED_NET),
 
   /* Relay only. */
   CALLBACK(check_canonical_channels, PERIODIC_EVENT_ROLE_RELAY,
@@ -1434,6 +1436,7 @@ STATIC periodic_event_item_t periodic_events[] = {
  * can access them by name.  We also keep them inside periodic_events[]
  * so that we can implement "reset all timers" in a reasonable way. */
 static periodic_event_item_t *check_descriptor_event=NULL;
+static periodic_event_item_t *dirvote_event=NULL;
 static periodic_event_item_t *fetch_networkstatus_event=NULL;
 static periodic_event_item_t *launch_descriptor_fetches_event=NULL;
 static periodic_event_item_t *check_dns_honesty_event=NULL;
@@ -1533,6 +1536,7 @@ initialize_periodic_events(void)
   STMT_BEGIN name ## _event = find_periodic_event( #name ); STMT_END
 
   NAMED_CALLBACK(check_descriptor);
+  NAMED_CALLBACK(dirvote);
   NAMED_CALLBACK(fetch_networkstatus);
   NAMED_CALLBACK(launch_descriptor_fetches);
   NAMED_CALLBACK(check_dns_honesty);
@@ -1718,10 +1722,6 @@ run_scheduled_events(time_t now)
     accounting_run_housekeeping(now);
   }
 
-  if (authdir_mode_v3(options)) {
-    dirvote_act(options, now);
-  }
-
   /* 3a. Every second, we examine pending circuits and prune the
    *    ones which have been pending for more than a few seconds.
    *    We do this before step 4, so it can try building more if
@@ -1973,6 +1973,40 @@ check_authority_cert_callback(time_t now, const or_options_t *options)
   return CHECK_V3_CERTIFICATE_INTERVAL;
 }
 
+/**
+ * Scheduled callback: Run directory-authority voting functionality.
+ *
+ * The schedule is a bit complicated here, so dirvote_act() manages the
+ * schedule itself.
+ **/
+static int
+dirvote_callback(time_t now, const or_options_t *options)
+{
+  if (!authdir_mode_v3(options)) {
+    tor_assert_nonfatal_unreached();
+    return 3600;
+  }
+
+  time_t next = dirvote_act(options, now);
+  if (BUG(next == TIME_MAX)) {
+    /* This shouldn't be returned unless we called dirvote_act() without
+     * being an authority.  If it happens, maybe our configuration will
+     * fix itself in an hour or so? */
+    return 3600;
+  }
+  return safe_timer_diff(now, next);
+}
+
+/** Reschedule the directory-authority voting event.  Run this whenever the
+ * schedule has changed. */
+void
+reschedule_dirvote(const or_options_t *options)
+{
+  if (periodic_events_initialized && authdir_mode_v3(options)) {
+    periodic_event_reschedule(dirvote_event);
+  }
+}
+
 /**
  * Periodic callback: If our consensus is too old, recalculate whether
  * we can actually use it.
@@ -2006,14 +2040,8 @@ save_state_callback(time_t now, const or_options_t *options)
   const time_t next_write = get_or_state()->next_write;
   if (next_write == TIME_MAX) {
     return 86400;
-  } else if (BUG(next_write <= now)) {
-    /* This can't happen due to clock jumps, since the value of next_write
-     * is based on the same "now" that we passed to or_state_save().
-     */
-    return PERIODIC_EVENT_NO_UPDATE;
-  } else {
-    return (int)(next_write - now);
   }
+  return safe_timer_diff(now, next_write);
 }
 
 /** Reschedule the event for saving the state file.

src/or/main.h

@@ -62,6 +62,7 @@ void reset_all_main_loop_timers(void);
 void reschedule_descriptor_update_check(void);
 void reschedule_directory_downloads(void);
 void reschedule_or_state_save(void);
+void reschedule_dirvote(const or_options_t *options);
 void mainloop_schedule_postloop_cleanup(void);
 void rescan_periodic_events(const or_options_t *options);
 

src/or/networkstatus.c

@@ -2001,6 +2001,7 @@ networkstatus_set_current_consensus(const char *consensus,
      * object so we can use the timings in there needed by some subsystems
      * such as hidden service and shared random. */
     dirvote_recalculate_timing(options, now);
+    reschedule_dirvote(options);
 
     nodelist_set_consensus(c);