[FrameworkBundle] prevent cache:clear creating too long paths #16829
Conversation
Tobion
commented
Dec 4, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #15547 #16783 |
| License | MIT |
| Doc PR | - |
| $kernel = $this->getContainer()->get('kernel'); | ||
| $output->writeln(sprintf('Clearing the cache for the <info>%s</info> environment with debug <info>%s</info>', $kernel->getEnvironment(), var_export($kernel->isDebug(), true))); | ||
| $this->getContainer()->get('cache_clearer')->clear($realCacheDir); | ||
|
|
||
| if ($input->getOption('no-warmup')) { | ||
| $filesystem->rename($realCacheDir, $oldCacheDir); |
There was a problem hiding this comment.
this was really dumb. it renamed the folder, just to remove it then.
There was a problem hiding this comment.
Isn't the purpose of that to provide a clean cache dir faster? Since renaming takes less time than removing.
There was a problem hiding this comment.
do you have some evidence for this?
There was a problem hiding this comment.
This is obviously heavily platform dependent, so its difficult to provide good evidence.
As a comparison on a high / low resource system: (Cache size: 11.2MB, 772 files, 512 directories)
| 50 iterations | Rename | Remove |
|---|---|---|
| high | ~3ms | ~687ms |
| low | ~4ms | ~3708ms |
There was a problem hiding this comment.
I'd the purpose of this line is to have an atomic clearing of the cache. If I'm not wrong, we should keep it (and add a comment about it)
There was a problem hiding this comment.
It depends on the filesystem you use. Normally removing a directory only involves removing a reference it to. Also I don't think it matters at the end and I could not find any information that this was done on purpose in git.
There was a problem hiding this comment.
Ok I'll revert that. From what I read, moving a directory is more likely to be atomic than deleting. Most filesystems implement moving a directory on the same partition atomically.
f6463f8 to
6e279c5
Compare
| @@ -100,8 +102,6 @@ protected function execute(InputInterface $input, OutputInterface $output) | |||
| */ | |||
| protected function warmup($warmupDir, $realCacheDir, $enableOptionalWarmers = true) | |||
| { | |||
| $this->getContainer()->get('filesystem')->remove($warmupDir); | |||
|
ping @symfony/deciders this fix is important to merge as the bug is currently causing problems for all windows users running symfony. Together with doctrine/cache#107 it is gone. |
|
👍 |
|
👍 Status: Reviewed |
…ths (Tobion) This PR was merged into the 2.3 branch. Discussion ---------- [FrameworkBundle] prevent cache:clear creating too long paths | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15547 #16783 | License | MIT | Doc PR | - Commits ------- 6e279c5 [FrameworkBundle] prevent cache:clear creating too long paths