php-fpm "oops, unknown child exited with 1"

[SunMar]

Hi,

I've been playing around with this EventStore client a bit. I've tried various other HTTP and TCP clients (like FriendsOfOuro and Rxnet) but they all had problems that were a no-go for me (HTTP implementations have to do active polling which adds massive overhead and latency and RxPHP silently eats any exceptions thrown in an onError). So far I'm really liking this EventStore TCP, you've done an amazon job on it! :)

One issue I've seen so far is though is that I'm constantly seeing the following in my PHP-FPM logs:

ALERT: oops, unknown child (117) exited with code 1. Please open a bug report (https://bugs.php.net).

I've done some deep-dive debugging with XDebug and it looks like there's nothing actually going wrong. It's just that Amphp spawns a child process and the alert is caused by Amp\Process\Internal\Posix\Runner->kill() that gets called during destruction when the child process isn't needed anymore. This does a proc_terminate(), which causes a ChannelException in the process-runner.php child process which then does an exit(1). At that point the child process isn't actually doing anything any more though, so everything works and nothing is going wrong. It is however not a very nice alert to have scattered in the logs.

I'm not very familiar with Amphp though, is this normal behavior, is prooph/event-store-client missing something so that Amphp can do a graceful shutdown of the child process instead of having to forcefully kill it, or am I implementing the client wrong?

My two test implementations (that both have the same issue) are as follows:

To write to a stream:

Loop::run(function () {
    yield $this->eventStore->connectAsync();
    yield $this->eventStore->appendToStreamAsync(...);
    Loop::stop();
});

Note that I have noticed that if I don't do a $this->eventStore->close() or Loop::stop(), the process will get stuck in the event loop forever doing heartbeats. Is that expected behavior? I don't necessarily want to close the connection because if I need it again later it's a waste to have to re-open it, but I'm also not sure if doing Loop::stop() is the right approach, since everything is done asynchronously I do want to make sure when the loop stops all events have been written to the stream and not risk stopping the loop before that has happened.

And to read from a stream:

Loop::run(function () {
    yield $this->eventStore->connectAsync();
    yield $this->eventStore->subscribeToStreamFromAsync(...);
    Loop::stop();
});

Note that the Loop::stop() here is just for debugging so I could check if in this case the child process would exit with exit(1) as well and it does. The end goal of the subscription is of course to keep it running and not close the connection, so in this case I do want the Loop to continue running and waiting for new events, in this scenario it shouldn't stop.

Hopefully you can shed some light on what's going on here, my guess is that I'm implementing the client wrong :). Thank you!

[prolic]

@SunMar you should read that comment: #64 (comment)

I'd recommend using the http client in a traditional application environment (synchronous). Otherwise use amp's call() method, don't start and stop a loop manually. But I'm not sure if Event Store handles lots of connections well.

[SunMar]

@prolic I want to use this in an asynchronous, that doesn't mean though that it should continue forever. There comes a point when there is nothing left to do and at that point the script should finish, preferably gracefully and not via a forced shutdown.

[SunMar]

Note that the examples are just to show how the problem can be reproduced. It is not the full scope of the application.

[prolic]

Just close the event store connection then.

[SunMar]

That is what I did initially, but that results in the same issue where the child process of Amphp is killed forcefully and then gives an alert in php-fpm. It could be that this is just something more in Amphp, but in the Amphp documentation I couldn't find anything except a Loop::stop() to finish.

[prolic]

I don't get it, either you use it async or you have php-fpm running. FPM + async - does it makes sense? Not sure.

[prolic]

Maybe you can provide a little more background of what you are trying to do.

[SunMar]

Well right now I'm mostly experimenting in getting the basics down on implementing your client and writing a new application from scratch using Event Sourcing. I'd like to create some projections that run as a service and that can subscribe to multiple event streams and/or subscriptions. Building that in an asynchronous way seemed to make sense to me.

The reason why I'm (also) using PHP-FPM is because while I'm experimenting I wanted to keep it simple and not bother with multiple packages for communicating with EventStore right now. Doing things asynchronously with PHP-FPM indeed doesn't make sense. I might switch to a different package to handle writing events in the PHP-FPM part of the application, but I'm not sure yet. For now I'm modelling things in a way that I'm gathering all events and do one transactional write of all events at the end, so there will be only one EventStore connection per request at most. Then if I have to choose between one HTTP connection or one TCP connection, TCP is probably more efficient and less overhead. Regardless it's things I'm not sure yet about and still experimenting to figure out what the best solution is.

In the end the PHP-FPM alert is only what uncovered the exit error status exit(1);. It's not specific to PHP-FPM, if I run my projection as a console command via the CLI the same thing happens, it just doesn't trigger an alert so you aren't aware that it's happened. So the PHP-FPM alert just made me dig deeper, uncovering the exit(1); also in the CLI environment. And though in the CLI environment there is no alert, it's still strange to me that a process is forcefully killed rather than being shutdown gracefully, it makes me wonder if I'm doing something wrong in the implementation, am missing a call to trigger a graceful shutdown? Or is this just an Amphp quirk that it can't gracefully shutdown its child processes and always has to kill them?

[prolic]

I don't know where this exit(1); call should be. I don't find it in amphp lib.

If you want to handle a traditional synchronous application, use event-store-http-client. You can still use the tcp client for async subscriptions (another process, cli script).

If you want to have a async application as well, let the app run via cli as well and use https://github.com/amphp/http-server

[SunMar]

Yes I'll probably end up using the event-store-http-client for the synchronous part of the application, and then this library for the asynchronous part.

To be more specific about the Amphp thing, once you end the loop Amp\Process\Internal\Posix\Runner->kill() is triggered (assuming you're running on Linux). On line 150 you have if (!\proc_terminate($handle->proc, 9)) { // Forcefully kill the process using SIGKILL. which forcefully kills the child process if it's still running.

The child process is an sh process that runs vendor/amphp/parallel/lib/Context/Internal/process-runner.php. There on line 68 you have:

$result = new Sync\ExitSuccess(yield call($callable, $channel));

When Amp\Process\Internal\Posix\Runner->kill() is triggered, the sh process is killed, that sh process is the parent process of vendor/amphp/parallel/lib/Context/Internal/process-runner.php (which isn't killed when the parent process is killed, the parent process becomes defunct and the php process keeps running). When that happens yield call($callable, $channel) in process-runner.php will detect that its parent process was killed and throw a Sync\ChannelException. That is caught on line 69 and does the exit(1);:

        $result = new Sync\ExitSuccess(yield call($callable, $channel));
    } catch (Sync\ChannelException $exception) {
        exit(1); // Parent context died, simply exit.

So the new Sync\ExitSuccess() is never triggered, which looks like it's the way to gracefully shutdown the process. But as I said, perhaps this is an Amphp thing and I better ask the Amphp project about this. It could be that this is normal expected behavior, but seeing as there actually is a Sync\ExitSuccess it suggests to me that there should be a way to have the child process gracefully finish rather than just flat out killing it.

[prolic]

I'd suggest you ask that question on amps issue tracker. But tag me, I'm curious as well.

[SunMar]

Will do that, will try first to create a reproducible setup that doesn't use this library to make sure it really is an Amphp thing.

[SunMar]

Created amphp/socket#56 for this. The error appears on their example script as well so it's probably not something specific to this library. While creating a proper reproducible example I did notice however that this library is using outdated versions of amphp/process and amphp/parallel. For those version 1.0 have been released and the composer.json in this library prevents from upgrading to that version. However when I did a search for all the use Amp\ statements, as far as I could tell they're not directly used. The same goes for a few other Amphp libraries. Is prooph/event-store-client really directly dependent on these (and their versions) or could they perhaps be removed? My search found no references in the code to the following entries in composer.json:

    "amphp/cache": "^1.2.0",
    "amphp/dns": "^0.9.13",
    "amphp/parallel": "^0.2.5",
    "amphp/process": "^0.3.3",
    "amphp/uri": "^0.1.3",

[prolic]

These are dependencies of the amp dependencies. For example amphp/socket requires amphp/dns to resolve dns names. amphp/http-server requires amphp/uri, etc.

[SunMar]

Yes, but amphp/dns and amphp/uri are already required by amphp/socket itself. What's the reason for overriding the requirements set by amphp/socket? Because right now the require in prooph/event-store-client is blocking the upgrade of amphp/parallel and amphp/process. Blocking an upgrade can make sense if you're directly dependent on those packages, but if you're not they shouldn't be in require. It's amphp/file that pulls in amphp/parallel and amphp/parallel pulls in amphp/process. Those should determine the required version of their own dependencies, not prooph/event-store-client unless you have a direct dependency on them.

I can create a PR for this as well, removing those dependencies, but I wanted to be sure they're really not used, because I determined the above list just by doing a search for all use statements and maybe there's a direct dependency that I've overlooked because it's not going via a use statement.

[prolic]

You're right. I probably added those in an early stage of development for some reason and forgot to clean up.

[prolic]

PR would be welcome

[prolic]

@SunMar prooph/event-store#354 might be interesting to you as well.

[SunMar]

Created PR #71 for the unused dependencies. Also I've looked at prooph before but got stuck on it and since it doesn't support EventStore (yet) I parked it as something to maybe look into later. Thank you for mentioning that PR, I'll track it and once it's merged revisit the prooph ES framework :).

Also will close this ticket now since the original issue I opened it for is an Amphp thing and doesn't have anything to do with this library. Thank you for the assistance :).

[prolic]

The event-store-client will simply rely on prooph/event-store v8 and some classes (f.e. EventData and ResolvedEvent will be removed from this repository, that's it. Future prooph event-store implementations (based on sql f.e.) will implement the interfaces of v8 and use its common classes as well. So you have one common interface for all event store implementations, no matter if SQL, ArangoDB or Event Store.