relay: Make the max pending tasks per CPU a consensus parameter

Until now, there was this magic number (64) used as the maximum number
of tasks a CPU worker can take at once.

This commit makes it a consensus parameter so our future selves can
think of a better value depending on network conditions.

Part of #40704

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2022-11-09 12:49:23 -05:00
parent 9c8c7804d5
commit e3f6908984
5 changed files with 51 additions and 8 deletions

View File

@ -2,3 +2,5 @@
- Two new consensus parameters are added to control the wait time in queue - Two new consensus parameters are added to control the wait time in queue
of the onionskins. One of them is the torrc MaxOnionQueueDelay options of the onionskins. One of them is the torrc MaxOnionQueueDelay options
which supersedes the consensus parameter. Closes ticket 40704. which supersedes the consensus parameter. Closes ticket 40704.
- Change a hardcoded value for the maximum of per CPU tasks into a
consensus parameter.

View File

@ -32,6 +32,7 @@
#include "feature/relay/onion_queue.h" #include "feature/relay/onion_queue.h"
#include "feature/stats/rephist.h" #include "feature/stats/rephist.h"
#include "feature/relay/router.h" #include "feature/relay/router.h"
#include "feature/nodelist/networkstatus.h"
#include "lib/evloop/workqueue.h" #include "lib/evloop/workqueue.h"
#include "core/crypto/onion_crypto.h" #include "core/crypto/onion_crypto.h"
@ -75,8 +76,42 @@ worker_state_free_void(void *arg)
static replyqueue_t *replyqueue = NULL; static replyqueue_t *replyqueue = NULL;
static threadpool_t *threadpool = NULL; static threadpool_t *threadpool = NULL;
static int total_pending_tasks = 0; static uint32_t total_pending_tasks = 0;
static int max_pending_tasks = 128; static uint32_t max_pending_tasks = 128;
/** Return the consensus parameter max pending tasks per CPU. */
static uint32_t
get_max_pending_tasks_per_cpu(const networkstatus_t *ns)
{
/* Total voodoo. Can we make this more sensible? Maybe, that is why we made it
* a consensus parameter so our future self can figure out this magic. */
#define MAX_PENDING_TASKS_PER_CPU_DEFAULT 64
#define MAX_PENDING_TASKS_PER_CPU_MIN 1
#define MAX_PENDING_TASKS_PER_CPU_MAX INT32_MAX
return networkstatus_get_param(ns, "max_pending_tasks_per_cpu",
MAX_PENDING_TASKS_PER_CPU_DEFAULT,
MAX_PENDING_TASKS_PER_CPU_MIN,
MAX_PENDING_TASKS_PER_CPU_MAX);
}
/** Set the max pending tasks per CPU worker. This uses the consensus to check
* for the allowed number per CPU. The ns parameter can be NULL as in that no
* consensus is available at the time of setting this value. */
static void
set_max_pending_tasks(const networkstatus_t *ns)
{
max_pending_tasks =
get_num_cpus(get_options()) * get_max_pending_tasks_per_cpu(ns);
}
/** Called when the consensus has changed. */
void
cpuworker_consensus_has_changed(const networkstatus_t *ns)
{
tor_assert(ns);
set_max_pending_tasks(ns);
}
/** Initialize the cpuworker subsystem. It is OK to call this more than once /** Initialize the cpuworker subsystem. It is OK to call this more than once
* during Tor's lifetime. * during Tor's lifetime.
@ -106,8 +141,7 @@ cpu_init(void)
tor_assert(r == 0); tor_assert(r == 0);
} }
/* Total voodoo. Can we make this more sensible? */ set_max_pending_tasks(NULL);
max_pending_tasks = get_num_cpus(get_options()) * 64;
} }
/** Magic numbers to make sure our cpuworker_requests don't grow any /** Magic numbers to make sure our cpuworker_requests don't grow any

View File

@ -12,8 +12,13 @@
#ifndef TOR_CPUWORKER_H #ifndef TOR_CPUWORKER_H
#define TOR_CPUWORKER_H #define TOR_CPUWORKER_H
#include "feature/nodelist/networkstatus_st.h"
void cpu_init(void); void cpu_init(void);
void cpuworkers_rotate_keyinfo(void); void cpuworkers_rotate_keyinfo(void);
void cpuworker_consensus_has_changed(const networkstatus_t *ns);
struct workqueue_entry_t; struct workqueue_entry_t;
enum workqueue_reply_t; enum workqueue_reply_t;
enum workqueue_priority_t; enum workqueue_priority_t;

View File

@ -40,6 +40,7 @@
#include "core/or/or.h" #include "core/or/or.h"
#include "app/config/config.h" #include "app/config/config.h"
#include "core/mainloop/connection.h" #include "core/mainloop/connection.h"
#include "core/mainloop/cpuworker.h"
#include "core/mainloop/mainloop.h" #include "core/mainloop/mainloop.h"
#include "core/mainloop/netstatus.h" #include "core/mainloop/netstatus.h"
#include "core/or/channel.h" #include "core/or/channel.h"
@ -1668,6 +1669,7 @@ notify_before_networkstatus_changes(const networkstatus_t *old_c,
relay_consensus_has_changed(new_c); relay_consensus_has_changed(new_c);
hs_dos_consensus_has_changed(new_c); hs_dos_consensus_has_changed(new_c);
rep_hist_consensus_has_changed(new_c); rep_hist_consensus_has_changed(new_c);
cpuworker_consensus_has_changed(new_c);
} }
/* Called after a new consensus has been put in the global state. It is safe /* Called after a new consensus has been put in the global state. It is safe

View File

@ -53,7 +53,7 @@ compute_num_cpus_impl(void)
cpus = cpus_onln; cpus = cpus_onln;
} else if (cpus_onln > 0 && cpus_conf > 0) { } else if (cpus_onln > 0 && cpus_conf > 0) {
if (cpus_onln < cpus_conf) { if (cpus_onln < cpus_conf) {
log_notice(LD_GENERAL, "I think we have %ld CPUS, but only %ld of them " log_info(LD_GENERAL, "I think we have %ld CPUS, but only %ld of them "
"are available. Telling Tor to only use %ld. You can over" "are available. Telling Tor to only use %ld. You can over"
"ride this with the NumCPUs option", "ride this with the NumCPUs option",
cpus_conf, cpus_onln, cpus_onln); cpus_conf, cpus_onln, cpus_onln);