Giter Club home page Giter Club logo

Comments (20)

prolic avatar prolic commented on May 24, 2024

@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.

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

@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.

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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

from event-store-client.

prolic avatar prolic commented on May 24, 2024

Just close the event store connection then.

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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.

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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?

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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.

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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.

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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",

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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.

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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.

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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

from event-store-client.

prolic avatar prolic commented on May 24, 2024

PR would be welcome

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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

from event-store-client.

SunMar avatar SunMar commented on May 24, 2024

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 :).

from event-store-client.

prolic avatar prolic commented on May 24, 2024

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.

from event-store-client.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.