Skip to content

Logging: add cache freshness fields - #13417

Open
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:cache-freshness-age-logging
Open

Logging: add cache freshness fields#13417
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:cache-freshness-age-logging

Conversation

@bneradt

@bneradt bneradt commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Cache modeling cannot currently observe an object’s effective
freshness limit or its age when ATS serves it, leaving access logs
unable to describe cache lifetime and churn.

This adds cfl and cca log fields and records their values when ATS
prepares cache metadata or serves a cached response. It resets the
values for each lookup and adds replay coverage for writes, hits, and
non-cacheable responses.

Fixes: #13127

Co-authored-by: sheikh-saifi p240592@pwr.nu.edu.pk

Copilot AI review requested due to automatic review settings July 21, 2026 20:12
@bneradt bneradt added this to the 11.0.0 milestone Jul 21, 2026
@bneradt bneradt self-assigned this Jul 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Cache modeling cannot currently observe an object’s effective
freshness limit or its age when ATS serves it, leaving access logs
unable to describe cache lifetime and churn.

This adds cfl and cca log fields and records their values when ATS
prepares cache metadata or serves a cached response. It resets the
values for each lookup and adds replay coverage for writes, hits, and
non-cacheable responses.

Fixes: apache#13127

Co-authored-by: sheikh-saifi <p240592@pwr.nu.edu.pk>
Copilot AI review requested due to automatic review settings July 21, 2026 20:23
@bneradt
bneradt force-pushed the cache-freshness-age-logging branch from ab8b85a to fed4605 Compare July 21, 2026 20:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sheikh-saifi

sheikh-saifi commented Jul 21, 2026

Copy link
Copy Markdown

Thanks for fixing this! the reset in DecideCacheLookup and splitting the write vs hit call sites was exactly what my version was missing. Went through the code and the three test cases (write/hit/uncacheable) and it all makes sense.
Sorry for disappearing after the first review, got pulled into other stuff and should've followed up. Happy to help test or review anything else on this.

@bneradt

bneradt commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for fixing this! the reset in DecideCacheLookup and splitting the write vs hit call sites was exactly what my version was missing. Went through the code and the three test cases (write/hit/uncacheable) and it all makes sense. Sorry for disappearing after the first review, got pulled into other stuff and should've followed up. Happy to help test or review anything else on this.

Sounds good. Thanks for the comments. I definitely plan to attribute your work with the commit, in addition to the PR here.

@cmcfarlen
cmcfarlen requested a review from moonchen July 27, 2026 22:33

@moonchen moonchen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Superseded. Please read my later review on this PR instead. This one had two incorrect line references and one inaccurate claim, both fixed there.

obj = s->cache_info.object_read;
}
cached_response = obj->response_get();
set_cache_freshness_info(s, cached_response, obj->request_sent_time_get(), obj->response_received_time_get(), true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should use s->request_sent_time and s->response_received_time rather than the object's raw timestamps.

what_is_document_freshness() uses the transaction timestamps, which are capped against s->client_request_time. The Age header sent to the client uses those values as well. Using the object's raw timestamps here can therefore make cca differ from both the freshness decision and the response's Age header.

There is also a problem on the api_update_cached_object == CONTINUE path. There, obj is &s->cache_info.object_store, whose timestamps can still be zero after a plugin creates it with TSHttpTxnCachedRespModifiableGet(). Passing those values to calculate_document_age() makes cca roughly the current Unix timestamp.

Passing s->request_sent_time and s->response_received_time fixes both cases.

// unset warning revalidation failed header if it set
// (potentially added by negative revalidating)
delete_warning_value(base_response, HTTPWarningCode::REVALIDATION_FAILED);
set_cache_freshness_info(s, base_response, s->request_sent_time, s->response_received_time, true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This call is skipped by the negative revalidation path earlier in the function. That branch sets action = SERVE_AND_UPDATE and returns before reaching here.

Those transactions serve the stale cached object and extend its stored Expires, but leave both freshness fields at -1. Negative revalidation is enabled by default for 500, 502, 503, and 504 responses. There is a reproducer in the review summary.


cfl Proxy Cache Freshness limit, in seconds, for an object served from or
written to cache. The limit reflects parsed cache response
metadata and the effective cache configuration. The value

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"the effective cache configuration" is broader than what calculate_document_freshness_limit() currently includes.

The calculation uses the response headers plus guaranteed_min/max_lifetime and heuristic_min/max_lifetime, but not s->cache_control. That excludes cache.config directives such as ttl-in-cache and revalidate-after.

For example, with ttl-in-cache=300 and an origin response containing max-age=1, a hit at age 3 logs cfl=1 cca=3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ability to log "freshness_limit" and "current_age"

4 participants