Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

daemon: Exit on idle after txn #606

Closed
wants to merge 5 commits into from
Closed

daemon: Exit on idle after txn #606

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6636e0b
app: Call RegisterClient with well-known name, use retry loop
cgwalters Sep 27, 2017
bb0d5bb
daemon: Exit on idle after ~10-15 seconds
cgwalters Mar 7, 2017
af3bc36
fixup! app: Call RegisterClient with well-known name, use retry loop
cgwalters Oct 3, 2017
1c83272
fixup! daemon: Exit on idle after ~10-15 seconds
cgwalters Oct 3, 2017
2cda8d7
fixup! app: Call RegisterClient with well-known name, use retry loop
cgwalters Oct 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/app/rpmostree-builtin-start-daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,17 @@ rpmostree_builtin_start_daemon (int argc,
* and avoid sending any more traffic our way.
* After that, release the name via API directly so we can wait for the result.
* More info:
* https://github.com/projectatomic/rpm-ostree/pull/606
* https://lists.freedesktop.org/archives/dbus/2015-May/016671.html
* https://github.com/cgwalters/test-exit-on-idle
*/
sd_notify (FALSE, "STOPPING=1");
/* The rpmostreed_daemon_run_until_idle_exit() path won't actually set
* FLUSHING right now, let's just forcibly do so if it hasn't been done
* already.
*/
if (appstate <= APPSTATE_FLUSHING)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should technically be strictly <, right?

state_transition (APPSTATE_FLUSHING);
g_dbus_connection_call (bus,
"org.freedesktop.DBus", "/org/freedesktop/DBus",
"org.freedesktop.DBus", "ReleaseName",
Expand Down
87 changes: 79 additions & 8 deletions src/daemon/rpmostreed-daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
#include <stdio.h>

#define RPMOSTREE_MESSAGE_TRANSACTION_STARTED SD_ID128_MAKE(d5,be,a3,7a,8f,c8,4f,f5,9d,bc,fd,79,17,7b,7d,f8)
/* I picked this arbitrarily */
#define IDLE_EXIT_TIMEOUT_SECONDS 10

/**
* SECTION: daemon
Expand All @@ -53,6 +55,8 @@ struct _RpmostreedDaemon {

gboolean running;
GDBusProxy *bus_proxy;
GSource *idle_exit_source;
guint rerender_status_id;
RpmostreedSysroot *sysroot;
gchar *sysroot_path;

Expand All @@ -75,7 +79,7 @@ enum
};

static void rpmostreed_daemon_initable_iface_init (GInitableIface *iface);
static void render_systemd_status (RpmostreedDaemon *self);
static void update_status (RpmostreedDaemon *self);

G_DEFINE_TYPE_WITH_CODE (RpmostreedDaemon, rpmostreed_daemon, G_TYPE_OBJECT,
G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE,
Expand Down Expand Up @@ -127,6 +131,9 @@ daemon_finalize (GObject *object)

g_object_unref (self->connection);
g_hash_table_unref (self->bus_clients);
g_clear_pointer (&self->idle_exit_source, (GDestroyNotify)g_source_unref);
if (self->rerender_status_id > 0)
g_source_remove (self->rerender_status_id);

g_free (self->sysroot_path);
G_OBJECT_CLASS (rpmostreed_daemon_parent_class)->finalize (object);
Expand Down Expand Up @@ -221,7 +228,7 @@ on_active_txn_changed (GObject *object,
}
}

render_systemd_status (self);
update_status (self);
}

static gboolean
Expand Down Expand Up @@ -365,12 +372,39 @@ on_name_owner_changed (GDBusConnection *connection,
rpmostreed_daemon_remove_client (self, name);
}

static gboolean
idle_update_status (void *data)
{
RpmostreedDaemon *self = data;

update_status (self);

return TRUE;
}


static gboolean
on_idle_exit (void *data)
{
RpmostreedDaemon *self = data;

g_clear_pointer (&self->idle_exit_source, (GDestroyNotify)g_source_unref);

sd_notifyf (0, "STOPPING=1\nSTATUS=Exiting due to idle");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just drop the STOPPING=1 here? We already do this once we exit the loop and it covers all other possible non-idle-exit scenarios as well.

self->running = FALSE;
g_main_context_wakeup (NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need this though? This is already running as part of the main context, so isn't it already woken up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we don't need it here. However, if one is doing multithreading, the main loop does need to be woken up. So I always add this as part of the "manage mainloop termination condition" pattern. It's just a write() to an eventfd() underneath so it's cheap.


return FALSE;
}

static void
render_systemd_status (RpmostreedDaemon *self)
update_status (RpmostreedDaemon *self)
{
g_autoptr(GVariant) active_txn = NULL;
gboolean have_active_txn = FALSE;
const char *method, *sender, *path;
const guint n_clients = g_hash_table_size (self->bus_clients);
gboolean currently_idle;

g_object_get (rpmostreed_sysroot_get (), "active-transaction", &active_txn, NULL);

Expand All @@ -381,13 +415,49 @@ render_systemd_status (RpmostreedDaemon *self)
have_active_txn = TRUE;
}

currently_idle = !have_active_txn && n_clients == 0;

if (currently_idle && !self->idle_exit_source)
{
/* I think adding some randomness is a good idea, to mitigate
* pathological cases where someone is talking to us at the same
* frequency as our exit timer. */
const guint idle_exit_secs = IDLE_EXIT_TIMEOUT_SECONDS + g_random_int_range (0, 5);
self->idle_exit_source = g_timeout_source_new_seconds (idle_exit_secs);
g_source_set_callback (self->idle_exit_source, on_idle_exit, self, NULL);
g_source_attach (self->idle_exit_source, NULL);
/* This source ensures we update the systemd status to show admins
* when we may auto-exit.
*/
self->rerender_status_id = g_timeout_add_seconds (1, idle_update_status, self);
sd_journal_print (LOG_INFO, "In idle state; will auto-exit in %u seconds", idle_exit_secs);
}
else if (!currently_idle && self->idle_exit_source)
{
g_source_destroy (self->idle_exit_source);
g_clear_pointer (&self->idle_exit_source, (GDestroyNotify)g_source_unref);
g_source_remove (self->rerender_status_id);
}

if (have_active_txn)
{
sd_notifyf (0, "STATUS=clients=%u; txn=%s caller=%s path=%s", g_hash_table_size (self->bus_clients),
method, sender, path);
}
else if (n_clients > 0)
sd_notifyf (0, "STATUS=clients=%u; idle", n_clients);
else
sd_notifyf (0, "STATUS=clients=%u; idle", g_hash_table_size (self->bus_clients));
{
guint64 readytime = g_source_get_ready_time (self->idle_exit_source);
guint64 curtime = g_source_get_time (self->idle_exit_source);
guint64 timeout_micros = readytime - curtime;
if (timeout_micros < 0)
timeout_micros = 0;

g_assert (currently_idle && self->idle_exit_source);
sd_notifyf (0, "STATUS=clients=%u; idle exit in %" G_GUINT64_FORMAT " seconds", n_clients,
timeout_micros / G_USEC_PER_SEC);
}
}

gboolean
Expand Down Expand Up @@ -439,7 +509,7 @@ rpmostreed_daemon_add_client (RpmostreedDaemon *self,
g_hash_table_insert (self->bus_clients, (char*)clientdata->address, clientdata);
g_autofree char *clientstr = rpmostree_client_to_string (clientdata);
sd_journal_print (LOG_INFO, "%s added; new total=%u", clientstr, g_hash_table_size (self->bus_clients));
render_systemd_status (self);
update_status (self);
}

/* Returns a string representing the state of the bus name @client.
Expand Down Expand Up @@ -468,8 +538,9 @@ rpmostreed_daemon_remove_client (RpmostreedDaemon *self,
g_dbus_connection_signal_unsubscribe (self->connection, clientdata->name_watch_id);
g_autofree char *clientstr = rpmostree_client_to_string (clientdata);
g_hash_table_remove (self->bus_clients, client);
sd_journal_print (LOG_INFO, "%s vanished; remaining=%u", clientstr, g_hash_table_size (self->bus_clients));
render_systemd_status (self);
const guint remaining = g_hash_table_size (self->bus_clients);
sd_journal_print (LOG_INFO, "%s vanished; remaining=%u", clientstr, remaining);
update_status (self);
}

void
Expand All @@ -482,7 +553,7 @@ void
rpmostreed_daemon_run_until_idle_exit (RpmostreedDaemon *self)
{
self->running = TRUE;
render_systemd_status (self);
update_status (self);
while (self->running)
g_main_context_iteration (NULL, TRUE);
}
Expand Down