Skip to content

Add Runner hooks for -k rotate event #2380

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

Open
yadij wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Runner hooks for -k rotate event #2380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions src/base/RunnersRegistry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ class RegisteredRunner
/// Meant for adjusting the module state based on configuration changes.
virtual void syncConfig() {}

/* Log Rotation events */

/// Called after receiving a log rotate request.
/// Meant for modules that manage log files to rename/rotate them.
virtual void rotateLogs() {}
Copy link
Copy Markdown
Contributor

@rousskov rousskov Feb 20, 2026

Choose a reason for hiding this comment

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

We should fix (or at least agree on how to fix) SMP log rotation problems before introducing a new (and unused) log rotation API. Merging #1223 could be a step in that direction, but we know that more work is needed. My current preference would be for options B+D that do not match the API proposed in this PR.

Copy link
Copy Markdown
Contributor Author

@yadij yadij Feb 21, 2026

Choose a reason for hiding this comment

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

The SMP issues are not relevant to the issues being resolved by the refactor this PR is part of.

Right now we have a circular dependency where SignalEngine calls some components, which in turn rely on Comm, which relies on SignalEngine.

SignalEngine already uses RunnersRegistry for components to subscribe/register their signal handlers (ie. -k shutdown, -k reconfigure, -k check). This PR is just adding the missing hooks for the -k rotate signal handlers to self-register instead of hard-coding that circular dependency.


/// Called after rotating log files.
/// Meant for modules that need to (re-)attach to log files after rotation.
virtual void finishLogRotate() {}
Copy link
Copy Markdown
Contributor

@rousskov rousskov Feb 20, 2026

Choose a reason for hiding this comment

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

I doubt it is a good idea to have "unattached" modules that are waiting for this event.

Copy link
Copy Markdown
Contributor Author

@yadij yadij Feb 21, 2026

Choose a reason for hiding this comment

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

I believe we already agreed there are better ways than Squid currently does. Unfortunately my attempts to form an architectural design that works in any way different have been constantly vetoed for more than a decade. So we are left with modules and helpers that log directly to the log files and detach/reattach on rotation.

This PR is a prerequisite to reducing circular dependencies in the Squid linker libraries within the constraints of that status quo.


/* Shutdown events */

/// Called after receiving a shutdown request and before stopping the main
Expand Down
5 changes: 5 additions & 0 deletions src/main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,8 @@ mainRotate(void)
if (AvoidSignalAction("log rotation", do_rotate))
return;

RunRegisteredHere(RegisteredRunner::rotateLogs);

icmpEngine.Close();
redirectShutdown();
#if USE_AUTH
Expand All @@ -959,6 +961,9 @@ mainRotate(void)
#if ICAP_CLIENT
icapLogRotate(); /*icap.log*/
#endif

RunRegisteredHere(RegisteredRunner::finishLogRotate);

icmpEngine.Open();
redirectInit();
#if USE_AUTH
Expand Down
Loading