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 cache refresh #5562

Merged
merged 1 commit into from Aug 3, 2023
Merged

Fix cache refresh #5562

merged 1 commit into from Aug 3, 2023

Conversation

Alkarex
Copy link
Member

@Alkarex Alkarex commented Aug 3, 2023

Improvement of #4422

The main problem was due to touch() not automatically clearing the file status cache, and requiring a call to clearstatcache(). Example:

php > touch('/tmp/touch.txt');
php > echo date('c', filemtime('/tmp/touch.txt'));
2023-08-03T17:27:43+02:00
php > touch('/tmp/touch.txt');
php > echo date('c', filemtime('/tmp/touch.txt'));
2023-08-03T17:27:43+02:00
php > clearstatcache(true, '/tmp/touch.txt');
php > echo date('c', filemtime('/tmp/touch.txt'));
2023-08-03T17:28:21+02:00

Improvement of FreshRSS#4422

The main problem was due to `touch()` not automatically clearing the file status cache, and requiring a call to `clearstatcache()`. Example:

```
php > touch('/tmp/touch.txt');
php > echo date('c', filemtime('/tmp/touch.txt'));
2023-08-03T17:27:43+02:00
php > touch('/tmp/touch.txt');
php > echo date('c', filemtime('/tmp/touch.txt'));
2023-08-03T17:27:43+02:00
php > clearstatcache(true, '/tmp/touch.txt');
php > echo date('c', filemtime('/tmp/touch.txt'));
2023-08-03T17:28:21+02:00
```
@Alkarex Alkarex added this to the 1.22.0 milestone Aug 3, 2023
@Alkarex Alkarex added the Bug (confirmed) 🐞 issues that are reproducable label Aug 3, 2023
@@ -385,10 +385,14 @@ public static function actualizeFeed(int $feed_id, string $feed_url, bool $force
}
if ($simplePiePush === null && $feed_id === 0 && (time() <= $feed->lastUpdate() + $ttl)) {
//Too early to refresh from source, but check whether the feed was updated by another user
if ($mtime <= 0 || $feed->lastUpdate() >= $mtime) {
$ε = 10; // negligible offset errors in seconds
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$ε = 10; // negligible offset errors in seconds
$errorInSeconds = 10; // negligible offset errors in seconds

it's more explicit

Copy link
Member Author

@Alkarex Alkarex Aug 3, 2023

Choose a reason for hiding this comment

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

It is not a fixed error, it is a negligible offset, commonly called epsilon :-)

$ε = 10; // negligible offset errors in seconds
if ($mtime <= 0 ||
$feed->lastUpdate() + $ε >= $mtime ||
time() + $ε >= $mtime + FreshRSS_Context::$system_conf->limits['cache_duration']) { // is cache still valid?
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
time() + $ε >= $mtime + FreshRSS_Context::$system_conf->limits['cache_duration']) { // is cache still valid?
time() + $errorInSeconds >= $mtime + FreshRSS_Context::$system_conf->limits['cache_duration']) { // is cache still valid?

@Alkarex Alkarex merged commit 4039f6c into FreshRSS:edge Aug 3, 2023
1 check passed
@Alkarex Alkarex deleted the fix-cache-refresh branch August 3, 2023 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug (confirmed) 🐞 issues that are reproducable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants