-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for RoadRunner PHP webserver #36290
Comments
there is likely a lot of statefullness in the current architecture doesn't doesn't allow keeping the processes running |
Likely true. However, I think it may be a worthwhile direction to consider and pursue. It looks like RoadRunner may require PSR-7 support, and I've seen your work in #31310. It will take some effort, but I think the benefits would be significant. ownCloud's rewrite gives it a huge performance boost, but it also means that every app has to be rewritten. That will take years. Supporting RoadRunner is the best of both worlds: it provides a major performance boost while allowing existing apps and code to work. |
@ChristophWurst, I wondered if you could provide a little insight into this. I am pretty familiar with the NC core, but I'm not very familiar with the . . . and here: If I understand correctly (I probably don't), the runtime provides a standard interface for request, response, input, and output libraries. By using a different runtime, you can support different environments such as Bref, Swoole, RoadRunner, etc.
Is that even close to being correct? I'm happy to experiment with getting NC to work on RoadRunner, but I just need to know where to start, I suppose. Thanks! |
TL;DR - what do you think about beginning to support a PSR7 response class in Nextcloud? The response could be instantiated as Full message: I started playing with RoadRunner, and the concept is surprisingly simple.
I have hacked Nextcloud to somewhat work with RoadRunner (statefulness doesn't seem to be an issue, actually), but a few things need to be done in order to make it compatible. The biggest issue is PSR7 responses. I have modified While this works for 70-80% of requests, Nextcloud doesn't currently send responses from a single class. For example, in Worse is There are a few other issues (the For now, I think the idea of adding a PSR7 response class to Nextcloud should be considered. The response could be instantiated as What do you think? I'm happy to start working on this and making PRs. |
Can we adjust \OCP\AppFramework\Http\Response to be compatible with PSR7? Implementing the three methods of https://github.com/php-fig/http-message/blob/master/src/ResponseInterface.php looks doable. |
To make my hacked system work, I was just using the Guzzle response class since it is PSR7-compatible, but modifying
Both of those tasks can be done - it's not difficult, but there just a lot of internal responses being sent. In an ideal world, To handle response objects in An alternative would be to include the RoadRunner classes themselves - it includes a response emitter in |
I wanted to kick around some of my thoughts before working on this and submitting PRs that might get roundly rejected by the team:
Like I said, my goal would be to implement this without impacting any existing responses currently in place in the NC source. Any existing responses would still work until they are migrated to using the new Any comments, questions, concerns . . . ? |
@summersab just stumbled across this and wanted to offer my two cents. Note: my POV is mostly performance and not the added features. Running Nextcloud over RoadRunner, by itself, wouldn't make it any faster. The major overhead here (I've profiled it extensively) is the request bootstrapping, which will happen regardless. The real performance gains come when using worker mode, i.e. not bootstrapping everything on every request. This is tricky, but not quite impossible. I tried this and got the following.
I tried a bunch of frameworks the worker mode with varying experiences.
If you do go down this rabbit hole, though, I'd caution against keeping hopes of getting it merged upstream. I had (or rather, tried to have) some discussion with the Nextcloud team about this and their outlook is quite negative on this ("rewrites are bad"? well I never suggested one). My only conclusion is that performance is not a high priority for Nextcloud GmbH, unfortunately. Maybe it's because it can't make good marketing material (I'd disagree, but then I don't have an MBA). Don't get me wrong: I love Nextcloud very much and so I continue to try making it better, but contributing any non-trivial change to the core platform as an outsider is extremely frustrating, and even the simpler ones can take weeks to review and months or years to get merged, even when they provide clear and significant benefits (example, one of mine, an extreme example). EDIT: this post is a bit old now. In the recent past, my experience of the review process has been much better. |
Sorry for not replying earlier, @pulsejet. I didn't follow all of your points - I probably need to get into the weeds and discover these things for myself (I'm a hands-on learner). As for Nextcloud GmbH not being receptive to non-trivial changes to the core . . . well, they're German, and I don't mean that as an insult. Culturally, they're very focused on good design, stability, and solid construction, but they aren't exactly open to radical change. There's a reason BMWs and Mercedes are excellent vehicles.* At the very least, I think it would be good to make NC PSR-7 compatible with how it handles responses. Right now, the core responds from at least a dozen places throughout the code - that isn't very good design. All responses should be returned to index.php and emitted from there as PSR-7 responses. @ChristophWurst, would you be open to some PRs that return these responses to index.php and handling them from there? *Two unrelated anecdotes that always amused me. A well-traveled professor of mine told me about a team in Germany that designed a Ferris wheel for some sort of engineering competition. He asked them if they had tested it, and they looked at him, befuddled. They responded, "We know it will work. Our designs and math are correct." They were so confident in their engineering that they didn't even need to test it. In the end, it worked exactly as expected - perfectly German. The same professor also told me about cars in Germany and some of the safety features they have which aren't present on US models. If I recall, one of the features was that the car wouldn't start unless the trunk/boot was closed. When he asked about the purpose of this feature, his German colleague again looked at him, befuddled. "Why would you ever need to start your car with the boot open? It is designed that way on purpose." Yeah, that's not how it works in the US, for sure - we're quite a bit more receptive to doing things just because we can and flying by the seat of our pants. |
My two cents after following the comments, there are two major restructuring that's required for any php app to work with PSR-7 and PSR-15 runtimes. One state should be local to request and response cycle Doing this would allow NC to run in its own context while the outer loop will keep processing messages with psr-7 formats regardless of whatever runtime you're using outside - the app context memory will be shared among requests making it faster. I'm not sure about NC core ( still new to it), but there are caveats in doing these mainly from my experience with turning any app into in memory context ( traditional monoliths having own frameworks), which would affect things like accessing external services among requests, i.e., database, redis etc. needs to be stateless as well, which should also be scalable enough ( enough pool to serve requests ) in architecture. Otherwise, requests will starve for resources from the main app context ( mainly app kernel ). So, if transitioning from a traditional app, a first step would be moving out these dependencies and isolating them with fuxed access patterns, which would then be injected in app context for consumption as internal service. Mainly, these problems can be solved with app containers and service access patterns, but writing them might also change how NC works internally and may affect parts of code that consume these services. Once that's done, returning all responses in PSR-7 format will ensure all code is compatible with any of these runtimes. Where they execute code is different story and performance will vary. Integrating workers and making apu available with grpc or using timers on these runtime shouldn't be part of NC core but different app contexts can be introduced to service crons or background tasks that cab be hooked to these runtimes. As there's no standard for those type of worker stuff, it should be left to individual impmementor on how they wish to deal with it depending on the type of runtime they choose. |
All good feedback. From what I've been able to see, Nextcloud itself isn't stateful - each new request is handled independently, everything is torn down when the response is emitted, and global constants are basically ephemeral. This significantly simplifies being able to support RoadRunner. The only thing I don't have much experience with are things like Redis, memcache, etc - I don't know how these may impact running on RR. If my understanding is correct, adding RR support basically requires using PSR-7 request/response objects and handling them all from a single place. Doing so allows the handlers to be inside the main RR loop. Right now, incoming requests are handled from a few locations (index.php, public.php, and remote.php to name a few). In each location, the Responses are the bigger problem. Just do a code search for Here is my basic approach to making NC compatible with RR:
It's definitely doable, and I hacked together a halfway-functional version of NC running on RR a few months back. I hope to spend a little time on this in the coming months, so any feedback you have on my approach is appreciated! |
Hey guys 👋🏻 One of the RoadRunner creators is here 😄 |
I came here from profiling my own Nextcloud instance: The profile seems to indicate Nextcloud takes a long time (almost half of the total request time) to bootstrap everything, load apps etc. - for every request. With roadrunner this would only need to be done once for all requests, allowing for significant performance improvments. |
The second RR creator is here (also, I'm a PHP guy). Feel free to let us know if you need any help. We've built several DAM systems, and some of the RR features (like queues, x-send files, and temporal) are designed with such systems in mind, potentially benefiting NextCloud as well. |
Having done the same profile multiple times myself, I agree with these results. Depending on what the downstream app is doing, bootstrapping time can even be >90% of the total request processing time. |
When I got into debugging NC a bit I was incredibly taken aback at how much NC reloads resources repeatedly per request (the four R's of the devil). I don't have the background knowledge to help with this endeavor, but I have long lamented NC's lackluster performance. I throw tons of resources at it and tweak my FPM configs and things barely help. In fact I got to this issue because I was looking for alternatives to PHP-FPM that might help with speed, but it seems the issue runs deeper than that. I think the entire ecosystem would benefit for an alternative implementation like this, especially if per-request CGI-like functionality can be retained for backwards-compatibility. Ultimately there is nothing to lose (besides increased test suite coverage, as well as support on the part of Nextcloud GmbH if they choose to support configurations like this) and a whole lot to gain. I am not a rich man but I would be game to start a bounty on this issue. I hear Bountysource is not doing so well, so if there are any alternatives I'd love to hear them. |
How to use GitHub
Is your feature request related to a problem? Please describe.
The release of ownCloud Infinite Scale poses serious competition for Nextcloud. In issue #16726, there has been much discussion regarding the future of Nextcloud. Should Nextcloud follow in the footsteps of ownCloud and migrate to Golang? Given the many changes in Nextcloud since originally forking, this would be a monumental undertaking. Additionally, it would require rewriting every application (which would likely result in many apps being abandoned).
Describe the solution you'd like
Nextcloud should provide support for the RoadRunner webserver:
https://github.com/roadrunner-server/roadrunner
RoadRunner is written in Golang and provides many modern features to PHP that are not currently available with Apache (websockets, for example). Additionally, by keeping PHP processes in memory, it provides significant speed improvements to existing PHP applications.
RoadRunner is a very mature project that is used by many production products including Laravel Octane. It has support for Symfony, so adding support in Nextcloud should not be too difficult.
By using RoadRunner, Nextcloud would take advantage of the performance of Golang and features like websockets without requiring developers to rewrite their apps for a new backend. It would also allow Nextcloud to scale similarly to ownCloud Infinite Scale.
The text was updated successfully, but these errors were encountered: