bryancall commented on code in PR #13869:
URL: https://github.com/apache/trafficserver/pull/13869#discussion_r4339546584
##########
tests/uranium_tests/cache/test_proxy_serve_stale_dns_fail.py:
##########
@@ -28,6 +28,22 @@ def test_proxy_serve_stale_dns_fail(ats_factory: ATSFactory,
curl: Curl, service
"""
_SERVER_NAME = "http://unknown.domain.com/";
+ # A stale copy may be served for max_stale_age seconds past its max-age.
The
+ # checks that expect a stale copy to be served get a window far longer
than the
+ # test runs, so a slow machine cannot push them past it. The checks that
expect a
+ # copy to be too old send X-Stale-Window: narrow, which shrinks the window
to one
+ # second for that request only, so a delay only makes them more certain.
The
+ # header does not change the cache key, so both kinds of request read the
same
+ # cached copy. Only the localhost maps load the rule, so the narrow window
applies
+ # on the proxy the client talks to directly, not on the parent the child
forwards
+ # to.
+ wide_stale_age = 300
+ narrow_stale_age = 1
+ stale_window_rules = (
+ "cond %{REMAP_PSEUDO_HOOK}\n"
+ "cond %{CLIENT-HEADER:X-Stale-Window} =narrow\n"
+ f"set-config proxy.config.http.cache.max_stale_age
{narrow_stale_age}\n")
Review Comment:
`%{REMAP_PSEUDO_HOOK}` is a hook condition, not a value test. It only picks
the hook the rule set runs on, so there is nothing to compare it against, and
the header_rewrite docs write it bare in every example. Leaving it as is.
##########
tests/uranium_tests/cache/test_proxy_serve_stale_dns_fail.py:
##########
@@ -65,10 +84,13 @@ def test_proxy_serve_stale_dns_fail(ats_factory:
ATSFactory, curl: Curl, service
f'{{curl}} -X PUSH -d "{stale_5}"
"http://localhost:{child.http_port}";;'
f'{{curl}} -X PUSH -d "{stale_10}"
"http://localhost:{parent.http_port}";;'
f"sleep 7; {{curl}} -s -v http://localhost:{child.http_port};";
- f"sleep 17; {{curl}} -s -v http://localhost:{child.http_port};";
+ # Serving stale through negative revalidation rewrites the cached
copy, which
+ # restarts its age. Wait until it is past max-age again, so it is too
old only
+ # if the rewrite kept the max-age rather than taking the new Expires.
+ f"sleep 7; {{curl}} -s -v {narrow} http://localhost:{child.http_port};";
f'{{curl_base}} -X PUSH -d "{stale_5}"
"http://localhost:{parent.http_port}";;'
f"sleep 7; {{curl_base}} -s -v http://localhost:{parent.http_port};";
- f"sleep 17; {{curl_base}} -s -v http://localhost:{parent.http_port};";)
+ f"{{curl_base}} -s -v {narrow} http://localhost:{parent.http_port};";)
Review Comment:
The parent doesn't need the wait. The child's first request gets a 500 back
from the parent, and serving stale on that 500 goes through negative
revalidation, which writes the copy back with a new Expires and restarts its
age. That is what the child's second `sleep 7` waits out. The parent's own
requests fail at DNS, and serving stale on a DNS failure returns the cached
copy without writing it back, so its age keeps counting from the push. It is
about 7 seconds old at the last request, past max-age 5 plus the 1 second
narrow window, so the expected 500 holds. I'd rather explain it here than push
a comment-only change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
