Process lifecycle fixes and a running-config dump by mbertheau · Pull Request #208 · Aiven-Open/journalpump

mbertheau · 2026-09-08T06:59:34Z

This is one PR of otherwise separate journalpump process fixes. They are individually very small changes. I decided to avoid the overhead of opening 10 separate PRs for them for now.

SIGHUP used to call reload_config in the signal handler. A JSON error there killed the process. The handler now sets a flag; the run loop applies it. SIGTERM only stops the loop. shutdown and the stale-reader close run from cleanup after run() returns.

ServiceDaemon.__init__ used to install those handlers before JournalPump had stale_readers and reader_by_fd. A signal in that window crashed on attributes that did not exist yet.

After a successful apply, journalpump writes the config it is using to $RUNTIME_DIRECTORY/config.json, or /run/journalpump/config.json if systemd did not set the variable. The shipped unit sets RuntimeDirectory=journalpump. The input file is not that record: a failed reload leaves the process on the previous apply.

The console script entry point had no main(). The installed command only worked because import ran run_exit() as a side effect. get_resume_cursor and get_reader(reinit=...) had no callers.

make systest-on-podman runs the system tests in a Fedora container to be independent of local system configuration, which, on Ubuntu for example, needs AppArmor massaging. The SIGHUP and SIGTERM tests start python -m journalpump and assert the process stays up or exits 0.

The README no longer claims json_state_file_path defaults to journalpump_state.json. A missing key means no cursor file. msg_buffer_max_length and msg_buffer_max_bytes are documented as top-level, which is where configure_readers reads them.

Made with Cursor

make systest-on-podman runs the system tests in a reusable Fedora container so they do not depend on the host's journald, rsyslogd, or AppArmor. make systest writes to the host journal and starts a second rsyslogd with a config under pytest's /tmp. On Ubuntu, AppArmor denies that open. CI already unloads that profile when GITHUB_ACTIONS is set and leaves every other Ubuntu host failing. Tests that only read recorded journal files pass there; the rsyslog sender tests do not. A privileged container and a first-run dnf/pip are the cost of leaving make systest and CI alone. The container is stopped, not deleted, so later runs skip the install. fedora:latest is taken at create time; a leftover container from another tag is replaced. journald is started each run because PID 1 is sleep. podman rm journalpump-systest forces a rebuild if requirements change. Co-authored-by: Cursor <cursoragent@cursor.com>

Construction never enters configure_readers' teardown loop: JournalPump.__init__ sets self.readers to {} first. Only a process that already has readers, then gets SIGHUP after the readers object changes, reaches reader.unregister_from_poll and dies. This test starts python -m journalpump, waits for systemd READY=1, rewrites the reader names, and sends SIGHUP. After SIGHUP it expects RELOADING=1 then READY=1. It fails today with AttributeError at configure_readers, which is the point: ServiceDaemon.main does not catch that exception, so the daemon exits. An in-process os.kill(getpid(), SIGHUP) would raise too, but would not show the process-level death operators see. Co-authored-by: Cursor <cursoragent@cursor.com>

shutdown() unregisters every reader; the run loop then calls _close_stale_readers, which unregisters again. The second call raises KeyError, the process exits 1, and systemd records a failed unit. A SIGTERM that lands in poll() does not hit this: the loop purges the stale fd and registers the reader again. This test holds the WatchdogSec notify so stop() arrives while ping_watchdog is still on the stack, after the first iteration has registered the reader. It fails today with KeyError in unregister_from_poll. Co-authored-by: Cursor <cursoragent@cursor.com>

kmichel-aiven · 2026-09-08T09:59:25Z

+    JournalPump.run_exit()
+
+
+if __name__ == "__main__":


Idiomatic usage is actually to not add this check when in __main__.py: https://docs.python.org/3/library/__main__.html#id1

Corrected, thanks! The entry points setting in setup.py is the one that was wrong.

kmichel-aiven · 2026-09-08T10:01:10Z

+
+
+class JournalpumpProcess:
+    # typing.Self, which ruff wants here, needs 3.11; journalpump still supports 3.10.


Not blocking, but it's ok to bump the min version to what we really need and stop support for older pythons.

Yeah, 3.13 and 3.14 it is then :)

kmichel-aiven · 2026-09-08T19:56:46Z

 ExecReload=/bin/kill -HUP $MAINPID
 ExecStart=/usr/bin/journalpump /var/lib/journalpump/journalpump.json
 WorkingDirectory=/var/lib/journalpump
+RuntimeDirectory=journalpump


The example should also show usage of RuntimeDirectoryMode= to something more restrictive that the default 755: the journalpump config can contain secrets (for instance username:password in senders urls) so journalpump shouldn't automatically create a copy of the config readable by anyone except the running user.

kmichel-aiven · 2026-09-08T19:57:36Z

+def running_config_path(*, configured_path: str | None) -> Path:
+    if configured_path:
+        return Path(configured_path)
+    return Path(os.environ.get("RUNTIME_DIRECTORY") or "/run/journalpump") / "config.json"


Related to the comment about RuntimeDirectoryMode=, writing the current config should be an opt-in and not automatically fallback to /run/journalpump if RUNTIME_DIRECTORY is unset.

kmichel-aiven · 2026-09-08T20:03:05Z

+            running_config.parent.mkdir(parents=True, exist_ok=True)
+            with atomic_replace_file(running_config) as fp:
+                json.dump(self.config, fp, indent=4, sort_keys=True)


To handle the case where it's not run through systemd with the recommended settings (and also to keep some amount of defense in depth), the modes should be set here we creating the folder and files, including the little dance during atomic_replace_file.

setup.py pointed the installed command at journalpump.__main__:main, a name that did not exist. Importing that module called JournalPump.run_exit(), and sys.exit() aborted the wrapper before it looked up main. cli.main returns a status. The console script and python -m journalpump both call it. __main__.py is only the -m shim. run_exit is gone. Co-authored-by: Cursor <cursoragent@cursor.com>

ServiceDaemon.__init__ loads config and installs SIGHUP/SIGTERM handlers before JournalPump finished constructing. A signal in that window ran shutdown or configure_readers against attributes that did not exist yet. First-start configure_readers skipped those attributes because there were no old readers. SIGTERM did not: it always walked the new readers and called stale_readers.add. Co-authored-by: Cursor <cursoragent@cursor.com>

configure_readers reads both from the process config, not from each reader. A per-reader setting had no effect. msg_buffer_max_bytes was not documented at all. Co-authored-by: Cursor <cursoragent@cursor.com>

SIGHUP used to call reload_config() in the handler, which rebuilt readers and senders under whatever the loop was doing. A JSON error there escaped into the middle of that iteration and killed the process. The handler now only sets a flag. run() applies it at the start of each iteration. A broken config file stays a log line. Co-authored-by: Cursor <cursoragent@cursor.com>

JournalPump.sigterm used to save state, stop senders, and unregister poll fds in the handler, then let the current iteration keep reading and queuing. The inherited handler already sets running = False. That is enough. shutdown and the stale-reader close now run from cleanup, which main() calls after run() returns. Co-authored-by: Cursor <cursoragent@cursor.com>

JournalPump.configure_readers already picks the resume cursor from saved sender state. This method duplicated that logic, including the unfinished "pick oldest cursor" note, and had no callers. Co-authored-by: Cursor <cursoragent@cursor.com>

get_reader was only called to create a missing journald reader. reinit=True was never passed, so the branch that closed and rebuilt an existing reader could not run. Co-authored-by: Cursor <cursoragent@cursor.com>

A missing json_state_file_path means journalpump does not persist cursors. The README said the default was journalpump_state.json in the working directory. Someone who omitted the key would expect positions to survive a restart. Co-authored-by: Cursor <cursoragent@cursor.com>

mbertheau · 2026-09-09T08:19:33Z

Converting to draft to test changes before re-review.

setup.py requires 3.13 or newer. CI, ruff, and mypy follow. __enter__ methods use typing.Self. Co-authored-by: Cursor <cursoragent@cursor.com>

The input JSON can change on disk after load. After each successful config apply, journalpump writes the dict it is using to $RUNTIME_DIRECTORY/config.json. The shipped unit sets RuntimeDirectory=journalpump, which exports RUNTIME_DIRECTORY=/run/journalpump. When that variable is unset, journalpump uses /run/journalpump so a non-systemd start still has a well-known path. json_running_config_path overrides both. The dump is json.dump(self.config), so secret_filter patterns are compiled onto copies. Compiling in place left a _Regexp on that dict; the dump raised TypeError and the file was never written. Production reader configs include secret_filters. A write failure is logged and does not stop the daemon. Co-authored-by: Cursor <cursoragent@cursor.com>

mbertheau and others added 3 commits September 8, 2026 08:51

mbertheau force-pushed the mbertheau-misc-fixes branch 2 times, most recently from 67b434a to c746f1d Compare September 8, 2026 13:02

mbertheau marked this pull request as ready for review September 8, 2026 13:07

kmichel-aiven requested changes Sep 8, 2026

View reviewed changes

mbertheau and others added 8 commits September 9, 2026 09:36

Document msg_buffer_max_length and msg_buffer_max_bytes as top-level.

9be9a41

configure_readers reads both from the process config, not from each reader. A per-reader setting had no effect. msg_buffer_max_bytes was not documented at all. Co-authored-by: Cursor <cursoragent@cursor.com>

Drop unused get_reader reinit argument.

336188a

get_reader was only called to create a missing journald reader. reinit=True was never passed, so the branch that closed and rebuilt an existing reader could not run. Co-authored-by: Cursor <cursoragent@cursor.com>

mbertheau marked this pull request as draft September 9, 2026 08:19

mbertheau force-pushed the mbertheau-misc-fixes branch from c746f1d to 489bda1 Compare September 9, 2026 09:54

mbertheau and others added 2 commits September 9, 2026 12:02

Drop Python 3.10 through 3.12.

6bde095

setup.py requires 3.13 or newer. CI, ruff, and mypy follow. __enter__ methods use typing.Self. Co-authored-by: Cursor <cursoragent@cursor.com>

mbertheau force-pushed the mbertheau-misc-fixes branch from 489bda1 to 107dd34 Compare September 9, 2026 10:04

mbertheau marked this pull request as ready for review September 9, 2026 16:10

mbertheau requested a review from kmichel-aiven September 9, 2026 16:15

kmichel-aiven approved these changes Sep 10, 2026

View reviewed changes

kmichel-aiven merged commit 86819f7 into Aiven-Open:master Sep 10, 2026
6 checks passed

mbertheau deleted the mbertheau-misc-fixes branch September 10, 2026 07:44

Navigation Menu

Process lifecycle fixes and a running-config dump - #208

Process lifecycle fixes and a running-config dump#208
kmichel-aiven merged 13 commits into
Aiven-Open:masterfrom
mbertheau:mbertheau-misc-fixes

mbertheau commented Sep 8, 2026 •

edited

Loading

Uh oh!

kmichel-aiven Sep 8, 2026

Uh oh!

mbertheau Sep 9, 2026

Uh oh!

kmichel-aiven Sep 8, 2026

Uh oh!

mbertheau Sep 9, 2026

Uh oh!

kmichel-aiven Sep 8, 2026

Uh oh!

mbertheau Sep 9, 2026

Uh oh!

kmichel-aiven Sep 8, 2026

Uh oh!

mbertheau Sep 9, 2026

Uh oh!

kmichel-aiven Sep 8, 2026

Uh oh!

mbertheau Sep 9, 2026

Uh oh!

mbertheau commented Sep 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		class JournalpumpProcess:
		# typing.Self, which ruff wants here, needs 3.11; journalpump still supports 3.10.

Conversation

mbertheau commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbertheau commented Sep 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mbertheau commented Sep 8, 2026 •

edited

Loading