Add "X-PPM-Restart" Header where the application can force a restart …

[shyim]

Like described in #437 i need something to restart programmatically a worker or all.

With this addition a Application can send in response header "X-PPM-Restart: worker" to restart the current worker which handled the response or "X-PPM-Restart: all" to restart all workers.

Why we need that? If a application has dynamic proxies and they change in the runtime, the application could send this header to restart the worker to load the new proxy.

I used strpos to find the header, if someone else have a better performant idea to read a header :)

[marcj]

I actually like the idea with the header 👍, but we need to make sure the http client on the other end doesn't receive that header.

[shyim]

Yes you are right :). I am looking where that should happends 🙈

[shyim]

To replace the Header i had to remove the piping and had done it in the data listener.
I have never used before React Stream, so i have no idea if this has performance issues... 🙈

[dnna]

Love the idea with the header :) Not sure either if the piping change has a performance impact though...

PS. Can we document this ability to restart somehow (maybe in the readme or somewhere else)? Would be a shame to forget about it and have to dig the code to find it again later.

[acasademont]

mmm this involves manually mangling with the plain text http response, smells a bit fishy. Right now we don't allow custom ReactPHP middlewares but if we did and someone used the GzipMiddleware the plain text wouldn't be there anymore, just a stream of bytes.

I think this should go on the Slave level, before the bytes are piped to the master and when the headers are readable in the handleRequest function of the SlaveProcess class. The slave would either kill himself or signal the master to kill all slaves.

[marcj]

@acasademont Gzip only compresses the body, not the headers. So, we as the master should always receive the headers.

[acasademont]

You're right! I'm too used to HTTP/2 :p At some point React will probably implement http2 too and then my point would be valid reactphp/http#99

Still feels like the master should only be a dispatcher and shouldn't mess around with what the Slaves work on.

[marcj]

Ah I see :) We have already a IPC pipe from all slaves to master (where we send for example all registered php files, see

php-pm/src/ProcessSlave.php

Line 277 in a23b684

protected function sendCurrentFiles()

), so we could use that again.

Would be something along the lines of:

Slave:

public sendRestartSignal() {
           $this->sendMessage($this->controller, 'restartSlave', ['mode' => 'all']);
           $this->sendMessage($this->controller, 'restartSlave', ['mode' => 'one', 'port' => $this->config['port']]);
}

Master:

protected function commandRestartSlave(array $data, ConnectionInterface $conn)
{
     if ('all' === $data['mode']) {
       //do stuff
    }
}

[andig]

Hi all, not sure which direction this PR is currently heading in- are we waiting for further updates or ha this become obsolete?

[andig]

Closing due to inactivity. Please respond here if you want to reopen this.