Skip to content

fix: log and count opcache restarts - #2634

Open
nicolas-grekas wants to merge 6 commits into
php:mainfrom
nicolas-grekas:opcache-restart-log
Open

nicolas-grekas wants to merge 6 commits into
php:mainfrom
nicolas-grekas:opcache-restart-log

Conversation

@nicolas-grekas

@nicolas-grekas nicolas-grekas commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Split out of #2621, which is closed: this keeps only the reporting, not the thread reboot.

opcache schedules a restart of its shared memory when the cache is full, on exhaustion or hash overflow, and more than opcache.max_wasted_percentage of it was wasted by invalidations. It then carries the restart out at the next request start on whichever thread comes first, and caching is off until then. Under ZTS the restart runs while other threads are still running: the deferral gate accel_is_inactive() probes for a conflicting lock with fcntl F_GETLK, and POSIX fcntl locks belong to the process, so the probe never sees the threads of the process holding them (on Windows the gate is a counter that does see them, so the restart is deferred instead). Workers are the worst case, they hold shared memory references for their whole life instead of for one request.

What an operator sees today is a segfault or a sudden slowdown with nothing in the logs pointing at opcache. opcache does report the event, but only at opcache.log_verbosity_level=4 and in its own log.

This PR makes the event visible and does nothing else. As a log line:

WARN opcache restart scheduled, caching stops until the next request start carries it out while other threads may still reference the old memory: raise opcache.memory_consumption, opcache.max_accelerated_files or opcache.max_wasted_percentage  reason=hash

And as a counter, since a log line cannot be alerted on:

frankenphp_opcache_restarts{reason="hash"} 1

zend_accel_schedule_restart_hook is the only signal reachable from Go for this, so it is set again, but this time it only logs and counts. No thread reboot, so nothing here can loop the way #2564 was worried about. The hook fires when the restart is scheduled, not when it runs: php-src has no hook for that yet, php/php-src#23828 adds one.

Same #if defined(ZTS) && PHP_VERSION_ID >= 80400 guard as before, so PHP 8.4 and up on ZTS builds; elsewhere the counter is not exposed at all rather than sitting at zero. reason follows zend_accel_restart_reason, named like the counters of opcache_get_status(): oom, hash, manual. manual should be unreachable since opcache_reset() is overridden, so seeing it means the override did not take, which is worth knowing too.

On the metric: Metrics keeps its shape, the counter comes through an optional OpcacheMetrics interface with the one OpcacheRestart(reason) method, detected with a type assertion like ServerMetrics in #2617. PrometheusMetrics implements it and backs it with a frankenphp_opcache_restarts counter registered up front like the thread gauges; an implementation passed to WithMetrics() that doesn't have it compiles unchanged and only misses the counter, the restart is logged either way. The known reasons are pre-populated at zero so a rate or an alert on the series works from the first restart on, instead of missing it for lack of a previous sample. The counter is incremented before the log-level gate, so the event is recorded even when warnings are silenced.

The hook path has a unit test: log line, reason mapping and counter. Forcing a real restart stays out of the suite, it needs exhaustion plus wasted memory and is what takes the process down.

@nicolas-grekas

Copy link
Copy Markdown
Contributor Author

Pushed 88313a6: the same event is now also exposed as a frankenphp_opcache_restarts{reason} counter, so it can be alerted on. PR body updated accordingly.

@alexandre-daubois alexandre-daubois left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can see how this would be useful to have such metric making the configuration/debugging easier.

Comment thread frankenphp.go Outdated
Comment thread frankenphp.c Outdated
@henderkes

Copy link
Copy Markdown
Contributor

I'm not convinced of the need to log these as metrics, too. Wouldn't it be enough to properly log them as warnings?

@nicolas-grekas

Copy link
Copy Markdown
Contributor Author

Given that a possible outcome of opcache restarts is a segfault, I'd rather monitor them in a programmatic way rather than by sniffing logs. That's my rationale for proposing it.

@henderkes

Copy link
Copy Markdown
Contributor

The usual metric for this should optimally always be zero. I feel like it being a metric normalises opcache restarts as if they were a normal thing during sapi execution, but they aren't, they cause segfaults in fpm too, just with a smaller blast radius.

@nicolas-grekas

Copy link
Copy Markdown
Contributor Author

Yes, that's a should-always-be-zero metric. I think it's important to give visibility here. This should come with docs saying that - if not zero there's an issue with your config. APM exist because these happen IRL and need proper monitoring, so the metric would be valuable IMHO.

@dunglas

dunglas commented Sep 8, 2026

Copy link
Copy Markdown
Member

Why not but marked as experimental, and deleted when this will be fixed upstream.
I don't want to definitely bloat metrics and the code base with temporary workarounds.

@henderkes henderkes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As experimental this is okay, but please note that we will probably delete the metric when/if the root issue is fixed upstream.

@nicolas-grekas

Copy link
Copy Markdown
Contributor Author

Done in 81b2916: marked experimental in the docs, the help string and the code, with a note that it should always be zero and that it will be removed once opcache handles restarts safely under ZTS.

opcache schedules a restart of its shared memory on exhaustion or hash
overflow, then carries it out at the next request init on any thread.
Under ZTS it does that while other threads are still running, because
the deferral gate (accel_is_inactive()) probes for a conflicting lock
with fcntl F_GETLK, and POSIX fcntl locks belong to the process, so the
probe never sees the threads of the process holding them. Workers are
the worst case: they hold shared memory references for their whole life
rather than for a single request.

The result is a crash or a slowdown with nothing in the logs pointing at
opcache. opcache does report it, but only at
opcache.log_verbosity_level=4 and in its own log.

zend_accel_schedule_restart_hook is the only in-process signal for this,
so it is used to emit one warning naming the restart reason and the two
settings that make restarts less likely. Nothing else is done with it:
the threads are not rebooted, which is what php#2564 removed.
The log line alone cannot be alerted on. Expose the same event as a
counter labelled by reason, pre-populated at zero for the known reasons
so a rate or an alert works from the first restart on.
Should always be zero, to be removed once opcache handles restarts
safely under ZTS.
@nicolas-grekas

nicolas-grekas commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Based on the discussion on metrics and BC break we had on #2617, the counter now comes through an optional OpcacheMetrics interface. So no BC break anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants