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

Fix ezp-21046: view cache is not expiring in one node #665

Closed
wants to merge 1 commit into from
Closed

Fix ezp-21046: view cache is not expiring in one node #665

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 13, 2013

No description provided.

@@ -85,6 +85,7 @@ public function copyFromDFS( $srcFilePath, $dstFilePath = false )
$ret = $this->createFile( $dstFilePath, file_get_contents( $srcFilePath ) );
}

$ret = $this->copyTimestamp( $srcFilePath, $dstFilePath );

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the file is not created correctly, perhaps this should not be executed?
Suggestion: add an if ( $ret ) condition before this line

@bdunogier
Copy link
Member

I'd rather agree with @gggeek here.

Syncing the timestamps would indeed most likely fix the issue, but we are fixing the symptoms, not the illness.

No cache file should be used without checking their validity on the database, as simple as that. It seems that this is the case, since this cache is most likely not valid anymore, right ?

@yannickroger
Copy link
Contributor

https://jira.ez.no/browse/EZP-21046#comment-78484
You can check out my comment. It seems that the problem is not as common as it seemed to be in the first place.

After looking into the code and trying different fixes for it ( adding extra db checks fixes the problem too), I think this solution is the most reliable one with less side effects on this very critical piece of code.

So +1

@bdunogier
Copy link
Member

+1 following our numerous discussions on the topic. Semantically, it makes sense do this sync anyway.

@yannickroger
Copy link
Contributor

Merged in master : 512174a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants