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

What's next: improve warmup before deploying #35

Open
t-richard opened this issue Mar 16, 2021 · 32 comments
Open

What's next: improve warmup before deploying #35

t-richard opened this issue Mar 16, 2021 · 32 comments

Comments

@t-richard
Copy link
Member

There were recent discussions with @mnapoli regarding this bridge.

The idea was to start by documenting some missing parts to run Symfony apps with Bref then we would have "a clear path" to improve this bridge.

I was willing to contribute to this bridge (and still am!) but I have no idea how this could be achieved to provide a smooth experience. I'll try to share my thoughts here.

The current approach is to deploy a warmed var/cache directory then copy/symlink directories to /tmp/cache

It has the advantage of working regarding of the content of var/cache because if the cache is empty/incomplete, files will be generated by Symfony at runtime in a writable directory.

On the other hand, the approach that was recently merged in the docs is nice because it is based on a feature offered by Symfony (it's less "hacky") and is faster because it doesn't imply copying. But it means the cache MUST be deployed along the source code and I found some examples of files generated on cache:warmup that are generated in cacheDir and not buildDir (these files would be copied using the current bridge approach but ignored using the documented approach).

I also recently saw the Runtime component added to Symfony 5.3 which @Nyholm said on the Symfony Slack could be used to provide a Bref Runtime but I'm not sure how this would work.

I'm also wondering how/if this repository could benefit from Flex or being a bundle to automate even more things than cache.

Let me know what are your thoughts/ideas about this

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 18, 2021

Thank you for opening this issue.

I will do some work on the Runtime thing. Im not 100% how it will work myself. =)

I'll take time the next few days to properly write an answer to this issue.

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 19, 2021

Short update: Here is some docs and example application for the Runtime:

https://github.com/php-runtime/bref

@mnapoli
Copy link
Member

mnapoli commented Mar 22, 2021

Hi, runtime discussions apart, I think the main topic to solve right now is the cache dir/build dir.

If I understand correctly, what this package could do is automatically redirect the cache to /tmp/cache, right? I'm not sure I understand your original post @t-richard, would there be downsides with that? Maybe that's a first step that is simple and makes sense?

(assuming this bridge targets Symfony 5.2 and up)

@t-richard
Copy link
Member Author

@Nyholm awesome ! I'll give this a try soon 🙂

@mnapoli it is already redirecting the cache to /tmp/cache but without leveraging the buildDir feature.

For me there are some downsides (more details cliking on the arrows):

1. we will NOT be able to deploy an app without a prewarmed cache

The main downside to me is that we loose the ability to NOT deploy the warmed cache whereas the current approach is allowing that because everything (cache + build) will be in /tmp/cache so it can be generated at runtime.

If we use the buildDir with the default Symfony path (which would be $LAMBDA_TASK_ROOT/var/cache on Lambda) then it's not writable at runtime.

This is intended but might be disturbing for new comers and it kinda "breaks" the actual behavior.

2. it's apparently not working when deploying from MacOS

@chekalsky reported on brefphp/bref#874 that the currently documented approach doesn't work when deploying from MacOS because /tmp is a symlink to /private/tmp and Symfony use realpath on cacheDir so this would need to be adressed by this bridge.

I suggested a workaround on Slack, not sure what the best solution is.

3. some files are generated on warmup in the cacheDir so they would be ignored

I noticed the cache warmup does generate router files (url_matching_routes.php and url_generating_routes.php) in the cache directory I suppose this is the intended behaviour so theses files would be generated on each cold start. there might be others.

I'm not sure how much this could have an impact on big apps with lots of routes as this apprently a slow process.

Does that make sense ?

So there are downsides but 1 should be okay with proper docs; 2 could work with the proposed workaround (not tested myself); 3 not sure if it has a significant impact.

@mnapoli
Copy link
Member

mnapoli commented Mar 23, 2021

Oh I think I'm realizing something: can the cache (not build dir) be prewarmed?

For the sake of the discussion, let's use these fixed terms:

  • cache: everything that goes in the cache dir
  • build: everything that goes in the build dir

I'm sorry but this whole topic is very complex. Every time I get back on that topic, every other week, I have already forgotten everything we already discussed because over many months we've discussed so many possibilities + Symfony is very complex + I have many other topics to follow. So, sorry about that, I'm really struggling 😅

Because of that, I will try to make the discussion simpler.

Goals:

  • help run classic projects on Lambda -> target 80% of users
  • advanced users/advanced use cases (the 20% left) are not a priority (at the moment)
  • optimal performance is not the most important thing: simplicity is

In a Symfony 5.2 project I should:

  • install the bridge
  • do as few extra changes as possible

and Symfony should run on Lambda.

Requirements on deployment should be minimal. For example, ideally we shouldn't require using Docker to deploy.
If that makes the whole thing more complex than it should be (or if it's too hard to address in v1), then it would be OK to require Docker.

We need to start with something that works, even if it's not perfect, and iterate.


Just a few specific notes:

If we use the buildDir with the default Symfony path (which would be $LAMBDA_TASK_ROOT/var/cache on Lambda) then it's not writable at runtime.

This is intended but might be disturbing for new comers and it kinda "breaks" the actual behavior.

I was under the understanding that build dir was completely meant to be read-only at runtime. That was the reason that directory was created. If it's not the case, then we have a problem indeed. But I'm pretty sure it's OK that it's read-only.

Regarding any other cache (i.e. not build dir), it should go into a writable directory and it's fine IMO if it's not pre-generated in v1. We can always improve that a bit later.

Let me know if that makes sense and if I missed something

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 23, 2021

Goals:

  • help run classic projects on Lambda -> target 80% of users
  • advanced users/advanced use cases (the 20% left) are not a priority (at the moment)
  • optimal performance is not the most important thing: simplicity is

In a Symfony 5.2 project I should:

  • install the bridge
  • do as few extra changes as possible

and Symfony should run on Lambda.

I believe that this is already true.

Let me know if that makes sense and if I missed something

You are correct.

@mnapoli
Copy link
Member

mnapoli commented Mar 23, 2021

I believe that this is already true.

👍 but the current version of the bridge doesn't take the build dir into account.

In that section of the README we encourage to warm the cache, and at the time it was indeed important because the build dir wasn't already pregenerated.

Now we can probably:

  • support the build dir
  • explain how to deploy with the build dir pregenerated (and that becomes mandatory now that the build dir would be readonly on Lambda)
  • update that section: warming the cache (≠ build) is less important than before

This section is also maybe obsolete now?

@mnapoli
Copy link
Member

mnapoli commented Mar 23, 2021

But those are good points, let's try to clarify even further the goals then:

  • as a user, I install the bridge
  • I do as few changes as possible to my code
  • I get clear instructions on how to deploy Symfony the simplest way possible, with acceptable performances, and all runtime caches work

Then a secondary goal:

  • extra instructions on how to configure the deployment/code further to get the best performances possible

(that secondary goal could include copying a prewarmed cache, etc. and can involve extra steps)

WDYT?

@t-richard
Copy link
Member Author

update that section: warming the cache (≠ build) is less important than before

Well cache:warmup is in fact mostly creating files in buildDir like the compiled container (the name of the command is confusing due to the fact that cache and build were the same thing for symfony prior v5.2).

This section is also maybe obsolete now?

IMHO the first part is not but the second would be obsolete if using the buildDir and not doing the copy/symlink thing.

@mnapoli
Copy link
Member

mnapoli commented Mar 23, 2021

OK the 1st section (cache:warmup) isn't obsolete, but it will now be a required step, right?

OK and for the second I'm not familiar enough with Symfony caches to know if it still makes sense. If it doesn't, it could be removed indeed.

@t-richard
Copy link
Member Author

OK the 1st section (cache:warmup) isn't obsolete, but it will now be a required step, right?

Yes, using buildDir would make this required

OK and for the second I'm not familiar enough with Symfony caches to know if it still makes sense. If it doesn't, it could be removed indeed.

The first part just says you should not write to filesystem for application cache and rely instead on a service (Redis, RDS, etc)

The second part is specific to the current implementation (see https://github.com/brefphp/symfony-bridge/blob/master/src/BrefKernel.php#L59)

@mnapoli
Copy link
Member

mnapoli commented Mar 23, 2021

OK so to be sure I understand, now all cache (not build) will be moved to /tmp, which means caching in the filesystem will work, right?

Which is a win I think

@t-richard
Copy link
Member Author

t-richard commented Mar 23, 2021

Caching with the filesystem adapter (cache.adapter.filesystem) will write cache to the cacheDir which is writable (located in /tmp).

But this is already the case for the current implementation.

And IMHO, in both case it's a bad idea because it would not be shared across different Lambdas.

But in some way, it might be great because users relying on the filesystem adpater would have a working app out of the box.

@mnapoli
Copy link
Member

mnapoli commented Mar 23, 2021

Yeah I don't think it's that bad of an idea in itself. There are plenty of use cases where using /tmp makes sense: good performances, no network call, no extra service to maintain and associated costs. Those are also advantages, and depending on the needs/projects that could make sense.

I'd prefer it to work out of the box, and in the README we can mention that the filesystem isn't shared between lambdas. Then users make their decision.

But that's great, I think we have a clear vision now?

@shouze
Copy link

shouze commented Mar 29, 2021

FYI guys I've experimented today a lot of errors like this one after migrating from symfony-bridge to simplified getBuilDir():

Error communicating with PHP-FPM to read the HTTP response. A root cause of this can be that the Lambda (or PHP) timed out, for example when trying to connect to a remote API or database, if this happens continuously check for those! Bref will restart PHP-FPM now. Original exception message: hollodotme\FastCGI\Exceptions\ReadFailedException Stream got blocked, or terminated.
[29-Mar-2021 19:55:32] WARNING: [pool default] child 10 exited on signal 11 (SIGSEGV) after 8.074317 seconds from start

My app is a Symfony 5.2.5 app with ApiPlatform 1.6. cache pools are plenty of Api platform generated files (about 3Mib). So as cache pools were the only thing I loose in the migration I had the intuition that it would be cool to mirror them into writeable cache as it was previously done in symfony-bridge. And... yes looks like it was that. It take about 60ms on λ cold start to mirror the 3Mib cache pools with Symfony Filesystem component. Here's what I've changed to my Symfony Kernel:

class Kernel {
    /**
     * {@inheritdoc}
     */
    public function boot(): void
    {
        // When on the lambda, copy the cache dir over to /tmp where it is writable
        if (isset($_SERVER['LAMBDA_TASK_ROOT']) && !is_dir($this->getCacheDir())) {
            $this->warmupCachePools(parent::getCacheDir(), $this->getCacheDir());
        }

        parent::boot();
    }

    private function warmupCachePools(string $readOnlyDir, string $writeDir): void
    {
        if (!is_dir($readOnlyDir)) {
            return;
        }

        $startTime = microtime(true);

        $filesystem = new Filesystem();
        $filesystem->mkdir($writeDir.'/pools');
        $filesystem->mirror($readOnlyDir.'/pools', $writeDir.'/pools');

        $message = sprintf(
            'Symfony cache pools directory prepared in %s ms.',
            number_format((microtime(true) - $startTime) * 1000, 2)
        );
        file_put_contents('php://stderr', date('[c] ').$message.PHP_EOL, FILE_APPEND);
    }
}

@shouze
Copy link

shouze commented Mar 29, 2021

EDIT: it as improved the situation but not so much in fact, still experience some errors. I rollback from now to be sure that's related to described changes.

@shouze
Copy link

shouze commented Mar 29, 2021

EDIT2: I have no 500 since rollback. So I guess FPM sigsegv were related to some write attemps in read only Symfony buildDir 😭
I don't know which ones, would be nice to find out, I'll try to reproduce first on local development by mounting var/cache as readonly layer.

@t-richard
Copy link
Member Author

  1. some files are generated on warmup in the cacheDir so they would be ignored
    I noticed the cache warmup does generate router files (url_matching_routes.php and url_generating_routes.php) in the cache directory I suppose this is the intended behaviour so theses files would be generated on each cold start. there might be others.

That's something I noticed too but not that much of an impact on apps I tested but some others might be relying more on cache than others (like API Platform in your case).

Your solution make senses and would be a nice addition.

The idea could be to do the following in the BrefKernel :

  1. we override getBuildDir to return $this->getProjectDir().'/var/build/'.$this->environment (the important part is build instead of the default cache)
  2. we keep the current behaviour for the cacheDir
  3. we advise users to deploy both folders (var/cache and var/build)

This solves a number of "problems" :

  • we could remove the LAMBDA_TASK_ROOT=bref from the cache:warmup as this would be useless
  • removing it would cause solve problems when deploying from MacOS (no more /tmp realpath'ed locally)
  • it would also avoid issues like the one @shouze got when deploying from inside a /tmp subdirectory (eg. when using some Docker images to deploy)
  • it would allow to deploy a pre-warmed build AND cache as it's done now
  • it would avoid useless copy of the buildDir done in the current implementation (which was impossible whithout a clear separation of directories)
  • it would still be compatible with symfony < 5.2 as getBuildDir would be ignored and we would just fallback to the current implementation

This seems like the best of both worlds to me.

What do you think ?

PS : I'm currently working on this bridge along with a sample application based on the Symfony demo to have a better view on what is required (compared with a skeleton app) while being open-source.

@shouze
Copy link

shouze commented Mar 29, 2021

@t-richard you can't keep cache dir in var/cache as Symfony apps should be able to write into cacheDir at runtime, even if you've warmed it up. To me, no other choice than copying var/cache to /tmp/cache when running λ with the kind of trick in the Kernel boot I've mentioned above.

But it seems it's not enough in some cases and still very risky, there are probably some write attempts in buildDir.

@t-richard
Copy link
Member Author

@shouze well this is the approach taken by this bridge at the moment.

The var/cache directory is deployed warm then its content is copied/symlinked to /tmp/cache at runtime during the kernel boot.

So we could just take the extra step of separating buildDir and cacheDir to distinct directories to avoid unnecessary processing of builded files.

To test this approach, just install this bridge and extend from the BrefKernel then add a getBuildDir method containing return $this->getProjectDir().'/var/build/'.$this->environment (this can be done even outside of lambda so no need to check whether we are on lambda or not).

If I'm not overseeing something, this should work in all cases.

If you still get arrors then it would mean some libraries are trying to write to the buildDir which is not allowed and should not happen.

@shouze
Copy link

shouze commented Mar 30, 2021

@t-richard yes we're agree that buildDir should not be written but I suspect this is the case regarding my issue.

Will check that on friday but I want to reproduce localy because it's too much energy and not so easy to debug once deployed to lambda. I'll reproduce same conditions (read only /var/task) inside a docker container and them will be able to find out why I'm having those fpm SIGSEGV happening.

@mnapoli
Copy link
Member

mnapoli commented Mar 30, 2021

@t-richard +1 with your suggestion. I'm not sure I understand how it solves the MacOS problem and the LAMBDA_TASK_ROOT=bref thing, but it's no problem because what you suggest is clear, if it works that's perfect.

@shouze I have no idea how to help you further, except maybe to try to use the signal technique to debug your timeouts? In #895 we can timeout before Lambda times out with a clear exception and stack trace: I would copy that bit of code in your project to find out what is causing the time out? (soon this feature should be built-in bref so that will be simpler to use)

@t-richard
Copy link
Member Author

@shouze not sure its linked to your issue but while trying this approach myself I stumbled upon a similar issue and fixed it by adding var/build/prod/** to my package.include and ended up with something like so in serverless.yaml

package:
    exclude:
        - node_modules/**
        - tests/**
        - var/**
    include:
        - var/cache/prod/**
        - var/build/prod/**

@t-richard +1 with your suggestion. I'm not sure I understand how it solves the MacOS problem and the LAMBDA_TASK_ROOT=bref thing, but it's no problem because what you suggest is clear, if it works that's perfect.

This would solve MacOS issues because we would rely on the current copy/symlink behaviour which implies that the container contains the default 'kernel.cache_dir' => (\dirname(__DIR__, 3).'/cache/prod') instead of the problematic 'kernel.cache_dir' => '/tmp/cache/prod' that becomes 'kernel.cache_dir' => '/private/tmp/cache/prod' on MacOS.

Symfony will only know about /tmp/cache/prod at runtime where it works but never at build time which could conflict with the OS or other things like what @shouze encountered.

And to generate the correct container we had to use the variable LAMBDA_TASK_ROOT=bref to "fake" being on Lambda and generate the correct values which would not be necessary (and maybe cause problems) with the proposed implementation.

Those two points were clearly not nice from a dev experience point of view.

I've just opened #36 so that you can get an idea of what it really means and I'll continue to work from that.

@ker0x
Copy link
Contributor

ker0x commented Apr 18, 2021

One thing that could be interesting to do it's to create a Symfony Flex recipe that prepare the application to be deploy.

Here is a list of things that could be done by the recipe

  • Adding the .serverless folder to .gitignore
  • Creating a serverless.yml already configured for a simple Symfony application
  • Updating src/Kernel.php with the necessary changes

wdyt ?

@shouze
Copy link

shouze commented Apr 18, 2021

One thing that could be interesting to do it's to create a Symfony Flex recipe that prepare the application to be deploy.

Here is a list of things that could be done by the recipe

  • Adding the .serverless folder to .gitignore

  • Creating a serverless.yml already configured for a simple Symfony application

Personally I use terraform so I will dislike 👎 this serverless.yml file.

  • Updating src/Kernel.php with the necessary changes

Yes why not

wdyt ?

@t-richard
Copy link
Member Author

Already thought about adding a Flex recipe in the original issue

I'm also wondering how/if this repository could benefit from Flex or being a bundle to automate even more things than cache.

But I'm not sure a recipe would be accepted as they are meant to configure Symfony bundles and we are not one ATM.

Updating src/Kernel.php with the necessary changes

Also I don't think modifying the Kernel would be possible. Because we could use copy-from-recipe but if we try to overwrite and existing file I think it would not be taken into account.

But I would be 👍 to generate Serverless Framework related files and config.

@shouze I totally understand that you would not be happy with this but apart from personal preferences :

  • Serverless Framework is the recommended and fully documented approach that is used by
  • Recipes are meant to configure Symfony with required config which covers the common use case and make it work out of the box (like DoctrineBundle creates PostgreSQL related config but you might want to use MySQL)
  • Flex explicitly tells you "Please review, edit and commit them: these files are yours."

For these reasons it seems legitimate to generate these files for Serverless Framework and if users want to use an alternative approach (Terraform, SAM, plain Cloudformation, etc) then they can just remove those files (and if they don't, it will not hurt anyway).

TL;DR; 👍 but not sure it would be accepted as it's not a bundle.

@mnapoli
Copy link
Member

mnapoli commented Apr 18, 2021

I'm 👍 for the 3 changes suggested.

Bref targets serverless.yml, I think this is an acceptable compromise as the recipe's changes would be easy to revert. This is also how the Laravel bridge works.

TL;DR; 👍 but not sure it would be accepted as it's not a bundle.

Good point, noted. I personally won't have time to implement this, but if someone wants to try feel free.

@ker0x
Copy link
Contributor

ker0x commented Apr 24, 2021

I can give a try for the recipe but I will open an issue first on the recipes-contrib repository mainly to ask if we are allowed to create it as the library is not a bundle.

@t-richard
Copy link
Member Author

Hi everyone !

I've been thinking over and over again about Symfony cache on lambda and how to make it work with the ephemeral filesystems.

This seems like an deep issue with Symfony and I don't feel like it is a simple fix in Symfony.

I've tried workarounds in #36 but to no avail and I have no idea how to proceed.

BUT, yesterday I read that Lambda was compatible with EFS (simply put, it's shared filesystem) and was wondering if this is worth considering as an acceptable solution for the cache.

Using EFS would mean being able to write to a single location on the first Lambda invocation (not of a single Lambda instance like the cold start, but the first invocation of a newly created Lambda) then reading from it on next requests (like it would on a classic hosting).
We could create an EFS from Cloudformation then use Serverless Framework to mount the EFS on each Lambda needing it.

I'm not sure if that's THE solution but it could be a (temporary ?) solution to accomodate the limitations that could be documented/added to the recipe.

Hope I was clear enough, let me know what you think.

NB : I haven't tested it yet and I'm unsure about potential limitations with this (added latency, costs, real performance benefits, etc)

@mnapoli
Copy link
Member

mnapoli commented Jun 8, 2021

Hey @t-richard! I did test EFS, and honestly I'm not sure it would work here:

  • it requires to set up a VPC, which:
    • is complex to configure in a generic way (and is complex in any case)
    • is slow to deploy (poor user experience)
    • costs money because then you need to add a NAT Gateway
  • it is slower: there have been some improvements since then (I haven't tested in a while), but last time I tested putting vendor/ in EFS and a single HTTP request would take more than 30 seconds to respond (compared to a few ms without EFS)
  • not sure if costs would be significant (I would expect that it could be)

@t-richard
Copy link
Member Author

Yeah, I tested this out but you basically got it all.

A VPC is a no go for simple apps (not needing a Database or private services that need to be in a VPC) and imply a lot of complexity. And NAT Gateways are super expensiv for small apps.

It's slower on the first request which has to write the cache to the EFS (~8s on a symfony/skeleton app) but other requests seemed to be efficient enough because they only read from it and a cache folder is relatively small compared to a vendor directory.

It would certainly imply more logic to clean the EFS on each deploy so that the new cache is generated.

Costs should be OK because you pay mostly for storage and network usage but again, the cache being relatively slow means it could be cheap enough.

Also the config needs to be repeated on each Lambda function so that could be confusing for new comers.

In the end it's a lot of work, added complexity but it does bring cold starts to basically only Lambda init duration + function execution time which is super cool.

Another dead end I suppose 😅

PS : if you want to see what this could look like in serverless.yml, here is a simple skeleton project deploying 2 functions whith one using this bridge and the other using EFS and the default symfony Kernel (just modified to read cache from the EFS).
https://github.com/t-richard/bref-efs-demo

@mnapoli
Copy link
Member

mnapoli commented Jun 28, 2022

Hey everyone, posting some updates/summary of discussions:

  • We cannot deploy the build dir if we generate it on any machine. Why:
    • the compiled container contains hardcoded paths to the cache dir
    • the cache dir is moved on AWS Lambda to /tmp
  • As such, the only way we can deploy the build dir is if it's built in a Docker image (in /var/task basically)
  • We may be able to deploy the cache dir if we also generate it in a Docker image in /tmp/...

Goals:

  • we are OK supporting only the latest Symfony versions (i.e. those with the build dir)
  • we are OK being a bit opinionated (i.e. for those who want to deploy advanced cache scenarios, they can figure it out)
  • we want a default and guided experience

Conclusion: we could probably document 2 options:

  1. Deploy without the build and cache dir -> simpler, but longer cold start -> great for starting
  2. Warmup in a Docker container and then deploy the build dir and the cache dir -> more complex but best for production
  • The cache dir will be copied from var/cache to /tmp by the current logic in the Kernel

Additionally, we can probably remove these lines now as most of the "read-only" cache is now in the build dir:

// Symlink all other directories
// This is especially important with the Container* directories since it uses require_once statements
if (is_dir("$readOnlyDir/$item")) {
$filesystem->symlink("$readOnlyDir/$item", "$writeDir/$item");
continue;
}

@mnapoli mnapoli changed the title What's next for the Symfony bridge ? What's next: improve warmup before deploying Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants