Skip to content

Clear cache command shouldn't use cache #23592

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

Closed
BourotBenjamin opened this issue Jul 19, 2017 · 21 comments
Closed

Clear cache command shouldn't use cache #23592

BourotBenjamin opened this issue Jul 19, 2017 · 21 comments

Comments

@BourotBenjamin
Copy link

BourotBenjamin commented Jul 19, 2017

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 2.8.25

When I try to use the command
php bin/console cache:clear --env=prod

I got the error message :
PHP Fatal error: Uncaught Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The parameter "test_param" must be defined. in /var/www/Youmiam/var/cache/prod/appProdProjectContainer.php:13062

Even If I add the parameter in my parameters file, the error still remains because it tries to load it from the cache.

@ostrolucky
Copy link
Contributor

ostrolucky commented Jul 19, 2017

Related problem #22243

@Simperfit
Copy link
Contributor

IMO you should remove the cache directory when deploying in production, then warming it before making the application public.

@ostrolucky
Copy link
Contributor

ostrolucky commented Jul 19, 2017

Well yes, but what is cache:clear command for then?

@linaori
Copy link
Contributor

linaori commented Jul 19, 2017

You should never remove the cache in production. If anything, you should be using a different checkout/tag/build next to it and simply have your webserver point to the new one.

@Simperfit
Copy link
Contributor

@iltar It's the same thing as making the application not public, removing the cache, warming it and re-activing the application.

It depends on how you deploy things.

@linaori
Copy link
Contributor

linaori commented Jul 20, 2017

Then you have down time, which you usually seek to avoid

@BourotBenjamin
Copy link
Author

If I shouldn't use this command, why does she exists ?
If it exists, it should work !

@ostrolucky
Copy link
Contributor

@iltar Not necessarily true. In our deployment we just remove server from load balancer, then clear and warmup cache, put it in load balancer again and do the same with rest of the servers. No downtime and much less overhead than maintaining/building multiple separate code bases on same server.

In any case, point is that this command should either not exist or work reliably. Answer "don't use it" doesn't fix it.

Also, it's still officially recommended step when deploying Symfony application. But anytime issue such as this one arises, everybody is always quick to say "don't do it that way". Don't you think something is wrong?

On the same doc page there are also listed some deployment scripts. The ones which offer integration with Symfony are:

  • Capistrano
  • Magallanes
  • Deployer

Magallanes and Deployer remove cache via cache:clear command. Capistrano removes it via rm -rf. None of them does deployments without clearing cache as @iltar recommends.

@linaori
Copy link
Contributor

linaori commented Jul 20, 2017

I'm personally a fan of rm -rf var/cache. The command for me is overhead. In the case of pre-build applications, the cache is often present already, as it's part of the warm up.

As long as nothing is hitting your application via either web or CLI during the moment you remove the cache, the command should work as you expect it to. When you clear the cache in <4.0.0, you will have to pass --no-warmup, as it's deprecated behavior. Why your parameter is missing, is not something I know of, nor why it happens without warmup.

The appProdProjectContainer is a generated file, which should be a fresh instance after clearing the cache (with --no-warmup). If this is not the case, it might be a problem in your application that only triggers under certain conditions. Adding the parameter to that file, will require you to remove the cache files, one way or another, and then warm up the cache again so the container is recompiled.

@Simperfit
Copy link
Contributor

Then IMO we should modify the docs to tell people to not use this command in production and to rebuild a fresh env or do a rm -rf var/cache with not access whatsoever on the server at the moment you do it.

@nicolas-grekas
Copy link
Member

It's a chicken and egg issue, one we cannot fix, because when the command runs, the container already booted (using cache as you said). rm -rf might be the solution. Closing.

@BourotBenjamin
Copy link
Author

The problem isn't resolved at all ! o_O
You can't just say 'rm -rf might be the solution. Closing.' as the cache command should do this !

If you are not supposed to use the command in 'prod' environnement you shouldn't have it in your documentation and you should return an error message that explains that you can't use the comment in prod environnement.
You can't just let the command crash and say "Nah, it's okay." :o

And if the command should be usable in prod environnement she has to be fixed.

If the command still there, she should work. That's all.

@nicolas-grekas
Copy link
Member

That's OSS, if you can make it work, please do :)
But on my side (not only me), we dunno how to because of that chicken-and-egg issue.
The command works fine most of the time, so yes, it's still useful to me. But having an issue tracking an unsolvable issue isn't really worth it to me...

@fabpot
Copy link
Member

fabpot commented Jul 28, 2017

I propose to deprecate the command altogether and remove it in 4.0.

@fabpot
Copy link
Member

fabpot commented Jul 28, 2017

The only "problem" is the cache-clearers that you can add to the command.

@Simperfit
Copy link
Contributor

@fabpot Maybe we should provide a way within symfony to clear the cache without using the container or to deprecate the command and to add a new one for that type of cache-clearers.

@theofidry
Copy link
Contributor

@BourotBenjamin the issue is also being closed as the move has been made: it existed for a reason but now should be used with --no-warmup. IIRC using it without that option is now deprecated as well

@curry684
Copy link
Contributor

Why don't we just provide a script in bin folder similar to the console command itself? Running "bin/clear-cache", with optional environment, that performs a platform appropriate recursive delete of either var/cache or the underlying env folder should be simple and always safe.

@curry684
Copy link
Contributor

See symfony/symfony-standard#1105

@Pierstoval
Copy link
Contributor

I propose to deprecate the command altogether and remove it in 4.0.

I don't agree. Clearing the cache "the clean way" or "the easiest way" should still be possible, especially for people using something like Windows for development (remembering php bin/console c:c is much easier than learning batch, which most Windows users don't know, especially for a strange stuff like rmdir /S /Q var/cache/dev which is not very intuitive).

Plus, cache clearers are important, because we might use other cache than the file one, even in dev (like doctrine cache for sessions, etc.).

@fabpot Maybe we should provide a way within symfony to clear the cache without using the container or to deprecate the command and to add a new one for that type of cache-clearers.

According to what we have in the kernel, maybe create a KernelCacheClearerInterface that will make the kernel able to clear the whole getCacheDir() without having to create a full container. As @fabpot said, the only problem is cache clearers, something should be done to clear their cache.

But I don't think it's a "that big" issue, about cache clearers. Maybe the container could be created, and if an exception is thrown, don't call them at all (because when one has an exception when compiling the container, we can bet it's not in production...).

What do you think?

@nicolas-grekas
Copy link
Member

Would be partly fixed by #23488

fabpot added a commit that referenced this issue Aug 18, 2017
… fix and un-deprecate cache:clear with warmup (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel][FrameworkBundle] Add RebootableInterface, fix and un-deprecate cache:clear with warmup

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #23592, #8500
| License       | MIT
| Doc PR        | -

This PR fixes the reasons why we deprecated cache:clear without --no-warmup.

Internally, it requires some cooperation from the Kernel, so that a deprecation is needed.
This allows the same kernel instance to be rebooted under a new cache directory, used during the warmup.
It leverages the new capability to create two independent non colliding containers.

Commits
-------

a4fc492 [HttpKernel][FrameworkBundle] Add RebootableInterface, fix and un-deprecate cache:clear with warmup
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

9 participants