Skip to content
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

Open
summersab opened this issue Jan 23, 2023 · 16 comments
Open

Add support for RoadRunner PHP webserver #36290

summersab opened this issue Jan 23, 2023 · 16 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement performance 🚀

Comments

@summersab
Copy link
Contributor

summersab commented Jan 23, 2023

How to use GitHub

  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

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.

@summersab summersab added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Jan 23, 2023
@ChristophWurst
Copy link
Member

so adding support in Nextcloud should not be too difficult.

there is likely a lot of statefullness in the current architecture doesn't doesn't allow keeping the processes running

@summersab
Copy link
Contributor Author

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.

@summersab
Copy link
Contributor Author

@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 3rdparty libs. I'm reading the documentation on PHP runtimes in Symfony here:
https://github.com/php-runtime/runtime

. . . and here:
https://github.com/php-runtime/runtime

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?
  • What runtime does Nextcloud use?
  • It sounds like by simply changing the PHP runtime to use the RoadRunner + Symphony runtime, that should handle session issues and allow NC to work on RoadRunner.

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!

@summersab
Copy link
Contributor Author

summersab commented Mar 29, 2023

TL;DR - what do you think about beginning to support a PSR7 response class in Nextcloud? The response could be instantiated as OC::$response and handled by index.php. Any current responses in the code would still work while this change is implemented throughout the system.

Full message:

I started playing with RoadRunner, and the concept is surprisingly simple.

  • RoadRunner runs in a loop and listens for requests.
  • When a request is received, a PSR7 request object is provided.
  • Set up your application, provide it the PSR7 request (modifying it as necessary to fit the format expected by the application), and execute.
  • Capture the response and submit it back to RoadRunner as a PSR7 response.

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 OC\AppFramework\Http\Output::setOutput() to store the response output instead of printing it. Then, in OC\AppFramework\App::main(), I am creating a PSR7 response and sending it back to the RoadRunner server.

While this works for 70-80% of requests, Nextcloud doesn't currently send responses from a single class. For example, in OC (base.php), there are half a dozen places where a response code is set, and the execution is returned back to index.php - the response is never truly captured anywhere.

Worse is OC_JSON and OC_Util where responses are sent and execution is forcefully stopped using exit(). For RoadRunner to work, these responses must be sent back to the main RoadRunner loop and submitted as a PSR7 response.

There are a few other issues (the v1, v2, and WebDAV routes, RoadRunner currently replies php_sapi_name() == 'cli' which makes everything run as ocs unless it is modified, some internal paths may need to be made dynamic so that a wrapper handler class can execute Nextcloud instead of modifying index.php, etc). However, these are pretty easy to fix.

For now, I think the idea of adding a PSR7 response class to Nextcloud should be considered. The response could be instantiated as OC::$response so that it is available to all apps and internal code, and outputting the response would be handled by index.php. This wouldn't break any existing responses that are scattered throughout the code (like the examples in the previous few paragraphs), but it would make it possible to update Nextcloud over time to be PSR7-compliant.

What do you think? I'm happy to start working on this and making PRs.

@ChristophWurst
Copy link
Member

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.

@summersab
Copy link
Contributor Author

summersab commented Mar 29, 2023

To make my hacked system work, I was just using the Guzzle response class since it is PSR7-compatible, but modifying OCP\AppFramework\Http\Response would certainly work. However:

  1. Code still needs to be changed so that the Response class is actually used
  2. The Response object needs to be accessible to and handled by index.php.

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, OC\AppFramework\Http\Request would also be PSR7 compatible, but at least for RoadRunner, it works just fine how it is.

To handle response objects in index.php, I suggest using the following library that Tobias Nyholm recommended in one of his projects:
https://github.com/laminas/laminas-httphandlerrunner

An alternative would be to include the RoadRunner classes themselves - it includes a response emitter in Spiral\RoadRunner\Http\PSR7Worker. That might make more sense since it would include classes users need to get started with RoadRunner:
https://github.com/spiral/roadrunner-http

@summersab
Copy link
Contributor Author

summersab commented Mar 31, 2023

I wanted to kick around some of my thoughts before working on this and submitting PRs that might get roundly rejected by the team:

  • Would you be opposed to using GuzzleHttp\Psr7\Response, extending it with methods from OCP\AppFramework\Http\Response, and using that as the NC Response class?
  • Doing the same thing for OCP\AppFramework\Http\Request would make sense, but right now, it's not as big of a deal (the Request object is currently instantiated in one place whereas responses are sent all over in the NC code).
  • The responses should probably be handled by index.php. I could use the Laminas or Spiral libraries (might make sense to use Spiral since it would provide RoadRunner support out of the box in the future).
  • I think the response object should be instantiated as far up in the execution chain as possible to catch any errors during execution. My thought is to set it up as OC::$response immediately after the autoloader in OC::init()
    • I COULD register it in OC\Server, but if an error occurs while the server is being constructed, no response would be sent.
    • Perhaps a separate response should be set up in index.php as well? If an error occurs during OC::init(), no response object would exist, and no response would be sent.

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 OC::$response object, so this can be done incrementally without disturbing the entire codebase.

Any comments, questions, concerns . . . ?

@pulsejet
Copy link
Member

pulsejet commented May 20, 2023

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

  1. Whatever code uses dependency injection is very easy to migrate. You just need to clear out the container at the start of each request and bootstrap it again. This way, you can do a gradual migration of dependencies to a different "static" container that persists across requests (e.g. the system and app config are easy). Some class by definition (e.g. the request itself) need to be instantiated per-request while others can be static.
  2. Clearing out the static variables. Unfortunately there are a bunch of them which are request specific, but this isn't very hard.
  3. I had some trouble with cookies and headers, since they get writting directly. But of course, all of this should be theoretically possible to migrate to PSR-7.
  4. Rendering templates. Unfortunately these define functions at times and then are included. This breaks worker mode completely, and so at the very least the templates need to be migrated to have a unique namespace for the functions and not redefine them.

I tried a bunch of frameworks the worker mode with varying experiences.

  1. Swoole. I started with this, but it gets very complicated since it uses coroutines. The issue here is that Swoole handles multiple requests simultaneously per-worker, and so the container also needs to be pinned to the request. But global static variables break completely, and so something like \OC::$server can never work (this is all IIRC, btw). I believe I could get it to go to the login screen, but didn't go further than that. There's some incomplete code here.
  2. RoadRunner. Better than Swoole since each worker only handles one request at one time, but you still need to do all of the above. A major task here, of course, is the migration to PSR-7. I also remember Nextcloud uses PHP sessions, so that won't work out of the box and quite a bit of work. I also find using the PHP CLI and an RPC interface a bit ... weird. As you mentioned you'll need to patch the $cli variable.
  3. FrankenPHP. This project is very experimental but was by far the easiest to use. It uses good old superglobals and most things actually work out of the box. I lost that tree but was able to get to the dashboard (DAV was still broken so no files) in worker mode quite easily. Running some preliminary benchmarks with just some of the larger dependencies globally static, it was already 2-2.5x faster than mod_php. Unfortunately I haven't had the bandwidth to work on this since.

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.

@summersab
Copy link
Contributor Author

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.

@mxmp210
Copy link

mxmp210 commented Sep 2, 2023

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
Two, you can keep the app kernel in memory as long as it has a psr-15 handlers separating global and local contexts.

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.

@summersab
Copy link
Contributor Author

summersab commented Sep 2, 2023

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 OCP\IRequest class is used. Unfortunately, it isn't PSR-7 compatible, but it shouldn't be too difficult fix that.

Responses are the bigger problem. Just do a code search for echo and print, and you'll see the reason. Not only is this not PSR-7 compatible, but it's all throughout the code. Ideally, any response (or better yet, response object) should be returned up the request chain back to the original handler before being emitted. This just isn't the case.

Here is my basic approach to making NC compatible with RR:
Switch to PSR-7 request and response handlers

  1. I recommend the Nyholm PSR-7 library to handle request/response objects and the Laminas library to emit responses
    a. Nyholm is extremely lightweight and 100% PSR-7 compatible (Guzzle is not, apparently). It is also written by a Symfony core dev
    b. Nyholm doesn't include an emitter, and the dev recommends using the Laminas library to emit responses
  2. Convert incoming requests in index.php to use the Nyholm library
    a. Initially, it may be easier to write a "bridge" that converts the PSR-7 request objects to OCP\IRequest objects to simplify the migration process
    b. Migrate the other request handlers to index.php so they are all in the same place
    c. Deprecate and remove OCP\IRequest in favor of the Nyholm library
  3. Convert all responses throughout the codebase to return a Nyholm response object back to index.php
    a. Basically, OC::handleRequest() becomes $response = OC::handleRequest(), and any current responses should return a Nyholm PSR-7 object back to index.php
    b. Add the Laminas library to index.php in order to emit the response
  4. Provide a way to execute the RR runtime loop
    a. When I was initially playing with RR support, I was trying not to touch the core NC code (which is my usual approach whenever possible). Basically, I put NC in its own directory (i.e. ./nextcloud) and created a separate index.php that runs the RR loop
    b. Inside the RR loop, add the request/response handlers from the main index.php
    c. There are a few tweaks that need to be made to base.php to handle NC being in a subdirectory
    d. Alternative approaches: provide separate version of NC for RR (pretty silly since it just requires a separate index.php), somehow modify index.php to detect that RR is being used and run the loop vs handling each request independently, or maybe something else . . . ?

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!

@rustatian
Copy link

Hey guys 👋🏻 One of the RoadRunner creators is here 😄
Found this issue while randomly searching the internet 😄
I'm not a PHP guy, but I would be happy to help. RR has the ability to work with custom plugins/middleware. For example, if you need to handle some specific requests or somehow preprocess them for NextCloud, you can easily write your own plugin (in Go) for the RR and use velox to build your custom RR binary. If you have some PHP-specific questions about RR's PHP part, I can call our PHP team to help you ⚡

@kolaente
Copy link

kolaente commented Oct 1, 2023

I came here from profiling my own Nextcloud instance:

image

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.

@wolfy-j
Copy link

wolfy-j commented Nov 29, 2023

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.

@pulsejet
Copy link
Member

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.

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.

@sevmonster
Copy link

sevmonster commented Dec 24, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement performance 🚀
Projects
None yet
Development

No branches or pull requests

9 participants