Merge pull request #10763 from poettering/pending-reload-fix

when reloading, don't throttle unit/job dbus change signal generation
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2018-11-14 10:44:50 +01:00
committed by GitHub
5 changed files with 53 additions and 40 deletions
+2 -2
View File
@@ -1354,8 +1354,8 @@ static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error *
* is finished. That way the caller knows when the reload
* finished. */
assert(!m->queued_message);
r = sd_bus_message_new_method_return(message, &m->queued_message);
assert(!m->pending_reload_message);
r = sd_bus_message_new_method_return(message, &m->pending_reload_message);
if (r < 0)
return r;
+9 -10
View File
@@ -48,23 +48,22 @@
static void destroy_bus(Manager *m, sd_bus **bus);
int bus_send_queued_message(Manager *m) {
int bus_send_pending_reload_message(Manager *m) {
int r;
assert(m);
if (!m->queued_message)
if (!m->pending_reload_message)
return 0;
/* If we cannot get rid of this message we won't dispatch any
* D-Bus messages, so that we won't end up wanting to queue
* another message. */
/* If we cannot get rid of this message we won't dispatch any D-Bus messages, so that we won't end up wanting
* to queue another message. */
r = sd_bus_send(NULL, m->queued_message, NULL);
r = sd_bus_send(NULL, m->pending_reload_message, NULL);
if (r < 0)
log_warning_errno(r, "Failed to send queued message: %m");
log_warning_errno(r, "Failed to send queued message, ignoring: %m");
m->queued_message = sd_bus_message_unref(m->queued_message);
m->pending_reload_message = sd_bus_message_unref(m->pending_reload_message);
return 0;
}
@@ -1073,8 +1072,8 @@ static void destroy_bus(Manager *m, sd_bus **bus) {
u->bus_track = sd_bus_track_unref(u->bus_track);
/* Get rid of queued message on this bus */
if (m->queued_message && sd_bus_message_get_bus(m->queued_message) == *bus)
m->queued_message = sd_bus_message_unref(m->queued_message);
if (m->pending_reload_message && sd_bus_message_get_bus(m->pending_reload_message) == *bus)
m->pending_reload_message = sd_bus_message_unref(m->pending_reload_message);
/* Possibly flush unwritten data, but only if we are
* unprivileged, since we don't want to sync here */
+1 -1
View File
@@ -5,7 +5,7 @@
#include "manager.h"
int bus_send_queued_message(Manager *m);
int bus_send_pending_reload_message(Manager *m);
int bus_init_private(Manager *m);
int bus_init_api(Manager *m);
+40 -26
View File
@@ -2157,57 +2157,71 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) {
assert(m);
/* Avoid recursion */
if (m->dispatching_dbus_queue)
return 0;
/* Anything to do at all? */
if (!m->dbus_unit_queue && !m->dbus_job_queue && !m->send_reloading_done && !m->queued_message)
return 0;
/* When we are reloading, let's not wait with generating signals, since we need to exit the manager as quickly
* as we can. There's no point in throttling generation of signals in that case. */
if (MANAGER_IS_RELOADING(m) || m->send_reloading_done || m->pending_reload_message)
budget = (unsigned) -1; /* infinite budget in this case */
else {
/* Anything to do at all? */
if (!m->dbus_unit_queue && !m->dbus_job_queue)
return 0;
/* Do we have overly many messages queued at the moment? If so, let's not enqueue more on top, let's sit this
* cycle out, and process things in a later cycle when the queues got a bit emptier. */
if (manager_bus_n_queued_write(m) > MANAGER_BUS_BUSY_THRESHOLD)
return 0;
/* Do we have overly many messages queued at the moment? If so, let's not enqueue more on top, let's
* sit this cycle out, and process things in a later cycle when the queues got a bit emptier. */
if (manager_bus_n_queued_write(m) > MANAGER_BUS_BUSY_THRESHOLD)
return 0;
/* Only process a certain number of units/jobs per event loop iteration. Even if the bus queue wasn't overly
* full before this call we shouldn't increase it in size too wildly in one step, and we shouldn't monopolize
* CPU time with generating these messages. Note the difference in counting of this "budget" and the
* "threshold" above: the "budget" is decreased only once per generated message, regardless how many
* busses/direct connections it is enqueued on, while the "threshold" is applied to each queued instance of bus
* message, i.e. if the same message is enqueued to five busses/direct connections it will be counted five
* times. This difference in counting ("references" vs. "instances") is primarily a result of the fact that
* it's easier to implement it this way, however it also reflects the thinking that the "threshold" should put
* a limit on used queue memory, i.e. space, while the "budget" should put a limit on time. Also note that
* the "threshold" is currently chosen much higher than the "budget". */
budget = MANAGER_BUS_MESSAGE_BUDGET;
/* Only process a certain number of units/jobs per event loop iteration. Even if the bus queue wasn't
* overly full before this call we shouldn't increase it in size too wildly in one step, and we
* shouldn't monopolize CPU time with generating these messages. Note the difference in counting of
* this "budget" and the "threshold" above: the "budget" is decreased only once per generated message,
* regardless how many busses/direct connections it is enqueued on, while the "threshold" is applied to
* each queued instance of bus message, i.e. if the same message is enqueued to five busses/direct
* connections it will be counted five times. This difference in counting ("references"
* vs. "instances") is primarily a result of the fact that it's easier to implement it this way,
* however it also reflects the thinking that the "threshold" should put a limit on used queue memory,
* i.e. space, while the "budget" should put a limit on time. Also note that the "threshold" is
* currently chosen much higher than the "budget". */
budget = MANAGER_BUS_MESSAGE_BUDGET;
}
m->dispatching_dbus_queue = true;
while (budget > 0 && (u = m->dbus_unit_queue)) {
while (budget != 0 && (u = m->dbus_unit_queue)) {
assert(u->in_dbus_queue);
bus_unit_send_change_signal(u);
n++, budget--;
n++;
if (budget != (unsigned) -1)
budget--;
}
while (budget > 0 && (j = m->dbus_job_queue)) {
while (budget != 0 && (j = m->dbus_job_queue)) {
assert(j->in_dbus_queue);
bus_job_send_change_signal(j);
n++, budget--;
n++;
if (budget != (unsigned) -1)
budget--;
}
m->dispatching_dbus_queue = false;
if (budget > 0 && m->send_reloading_done) {
if (m->send_reloading_done) {
m->send_reloading_done = false;
bus_manager_send_reloading(m, false);
n++, budget--;
n++;
}
if (budget > 0 && m->queued_message) {
bus_send_queued_message(m);
if (m->pending_reload_message) {
bus_send_pending_reload_message(m);
n++;
}
+1 -1
View File
@@ -248,7 +248,7 @@ struct Manager {
/* This is used during reloading: before the reload we queue
* the reply message here, and afterwards we send it */
sd_bus_message *queued_message;
sd_bus_message *pending_reload_message;
Hashmap *watch_bus; /* D-Bus names => Unit object n:1 */