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

Upstream sync #31

Merged
merged 2 commits into from
Sep 29, 2024
Merged

Upstream sync #31

merged 2 commits into from
Sep 29, 2024

Conversation

Alkarex
Copy link
Member

@Alkarex Alkarex commented Sep 29, 2024

When we switched to PSR-16 inspired `DataCache` in bb38d74, we introduced a regression due to impedance mismatch between PSR-16 and HTTP Caching mechanisms:
- HTTP caching uses two step invalidation – if a cache entry is accessed after its TTL passes, a request is made with a stored HTTP caching header (e.g. ETag) and the cache is considered invalid only if the server returns something else than 304 Not Modified response.
- On the other hand, PSR-16 cache does not allow access to entries past its TTL, forcing us to fetch the content every time, even when the remote server could tell us it is still fine.

To work around this, bb38d74 decided to store `cache_expiration_time` field in the cache entry so that we could bypass the PSR-16’s strict TTL and handle expiration ourselves. But there were few places where we forgot to set it. This patch fixes that.

Unfortunately, because we also pass the same value as `cache_expiration_time` to `DataCache::set_data` as TTL, the early eviction still happens. We will also need to set the TTL to `null` (unlimited) so that the `DataCache` does not evict the entries behind our back.

Partly fixes simplepie#830 ([BUG] Cache constantly updating)
@Alkarex Alkarex merged commit 88f13f2 into freshrss Sep 29, 2024
20 checks passed
@Alkarex Alkarex deleted the upstream-sync branch September 29, 2024 11:19
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

Successfully merging this pull request may close these issues.

1 participant