max_execution_time

[AlliBalliBaba]

I guess this is most of all a question for @dunglas. I'm still a bit confused as of the current state of max_execution timers. Are they only working on Linux with ZTS? What signal is being used to trigger the timeout? (Mostly regarding the hanging described in #1216)

I experimented a bit with triggering timeouts from the go side (via SIGPROF). This generally works, but it does not trigger the fatal PHP error. Ideally we would not have to do this from the go side, even if the go scheduler seems to handle this pretty efficiently.

[dunglas]

Yes, timeout is now properly supported (at least in normal mode) for both Linux and FreeBSD in ZTS mode.
The signal used is SIGRTMIN: https://github.com/php/php-src/blob/master/Zend/zend_max_execution_timer.c

It's not possible to use SIGPROF because it is reserved by the Go runtime itself: https://pkg.go.dev/os/signal#hdr-Go_programs_that_use_cgo_or_SWIG

[dunglas]

I'm not sure it's even possible to do this easily in Go:

• Go doesn't support OS-based timers that trigger signals, which are necessary for hard timeouts (maybe soft timeouts could be good enough, but they won't be able to stop scripts doing blocking syscalls for instance)
• the signal must be directed to the C thread running the script (signals are process wide by default, but Linux and FreeBSD provide APIs to direct a signal to a specific thread, which is mandatory for ZTS, and I patched PHP to use theses APIs)

[withinboredom]

I'm actually seeing a weird phenomenon where listening for max_execution_time on the go side will increase server performance.

If I had to guess, it is php probably not cleaning up things properly or skipping the final GC even. I don't know how well PHP handles unexpected signals, but most likely it just bails as quickly as possible assuming a fault.

Sending SIGRTMIN from the go side works pretty well, but it does not cause the fatal error which is probably required.

That's probably because the listener isn't listening for it to throw the fatal error.

even more so considering we might execute PHP directly on go threads in the future.

There's actually a closed PR of mine that did this waaay back in the day. (#841) I would find it ironic if we actually ended up doing it.

they won't be able to stop scripts doing blocking syscalls for instance

I believe this is actually the issue we're trying to solve (long queries running beyond the timeout), and I'm not sure franken-signals are the right approach. The right approach is for people to put timeouts on their database, as we cannot be everyone's nanny and do everything for them.

[dunglas]

I agree. To run PHP scripts on Go-managed threads, we'll have to get rid of signal-based timeouts entirely (that will also mean no more hard timeouts).

The PHP Foundation is working on Go-like signal-less timeouts. There is even a POC, but some work still need to be done before we get there.

[AlliBalliBaba]

If I had to guess, it is php probably not cleaning up things properly or skipping the final GC even. I don't know how well PHP handles unexpected signals, but most likely it just bails as quickly as possible assuming a fault.

I'm actually seeing the server being able to process more requests even if PHP doesn't time out. Just the fact that there's an additional timer running on the go side seems to increase RPS by up to 10% for some weird reason.

the signal must be directed to the C thread running the script (signals are process wide by default, but Linux and FreeBSD provide APIs to direct a signal to a specific thread, which is mandatory for ZTS, and I patched PHP to use theses APIs)

Triggering the signal works fine by just storing the pthread_id on the go side and calling pthread_kill

I believe this is actually the issue we're trying to solve (long queries running beyond the timeout), and I'm not sure franken-signals are the right approach. The right approach is for people to put timeouts on their database, as we cannot be everyone's nanny and do everything for them.

The PHP Foundation is working on Go-like signal-less timeouts. There is even a POC, but some work still need to be done before we get there.

I guess I mostly agree that this is an issue best fixed on the PHP side of things. While I think it would be possible to go thepthread_killroute, I don't know if this is the right approach in the long term.

What would be relatively easy to add instead is a timeout for stalling threads and to also reject closed client connections when stalling.

[AlliBalliBaba]

Actually, I just realized that handling timeouts on the go side is very easy to do, but it does not solve the hanging on SELECT pg_sleep(100). I'll just post this here in case we ever need it for other reasons:

// this function triggers the timeout
static void frankenphp_trigger_timeout(int signo) {
    zend_atomic_bool_store_ex(&EG(timed_out), true);
    zend_atomic_bool_store_ex(&EG(vm_interrupt), true);
}

// this function registers the signal handler
static void frankenphp_init_timeout(void) {
    signal(SIGUSR1, frankenphp_trigger_timeout);
}

// this function triggers the signal from the go side
func (thread *phpThread) kill() {
    C.pthread_kill(thread.pThread, C.SIGUSR1)
}