Ema has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/349188 )
Change subject: cache_upload: do not cache tiny objects at the backend layer
......................................................................
cache_upload: do not cache tiny objects at the backend layer
Don not cache objects with CL < 256B at the backend layer.
Full rationale here: T145661#3197012.
Bug: T145661
Change-Id: I74688d43748a5b1eaf0d3ed2a763b3d093f5f109
---
A modules/varnish/files/tests/upload/13-upload-hfp-small-objects.vtc
M modules/varnish/templates/upload-backend.inc.vcl.erb
M modules/varnish/templates/upload-frontend.inc.vcl.erb
3 files changed, 71 insertions(+), 2 deletions(-)
Approvals:
Ema: Verified; Looks good to me, approved
BBlack: Looks good to me, but someone else must approve
diff --git a/modules/varnish/files/tests/upload/13-upload-hfp-small-objects.vtc
b/modules/varnish/files/tests/upload/13-upload-hfp-small-objects.vtc
new file mode 100644
index 0000000..80e162c
--- /dev/null
+++ b/modules/varnish/files/tests/upload/13-upload-hfp-small-objects.vtc
@@ -0,0 +1,59 @@
+varnishtest "upload backends should not cache small objects"
+
+server s1 {
+ rxreq
+ expect req.url == "/test-pass"
+ txresp -bodylen 255
+
+ rxreq
+ expect req.url == "/test-pass"
+ txresp -bodylen 255
+
+ rxreq
+ expect req.url == "/test-miss-hit"
+ txresp -bodylen 1025
+} -start
+
+varnish v1 -arg "-p vcc_err_unref=false" -vcl+backend {
+ backend vtc_backend {
+ .host = "${s1_addr}"; .port = "${s1_port}";
+ }
+
+ include "/usr/share/varnish/tests/wikimedia_upload-backend.vcl";
+} -start
+
+client c1 {
+ txreq -url "/test-pass" -hdr "Host: upload.wikimedia.org"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.X-Cache-Int ~ "pass"
+} -run
+
+varnish v1 -expect cache_hitpass == 0
+
+client c2 {
+ txreq -url "/test-pass" -hdr "Host: upload.wikimedia.org"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.X-Cache-Int ~ "pass"
+} -run
+
+varnish v1 -expect cache_hitpass == 1
+
+client c3 {
+ txreq -url "/test-miss-hit" -hdr "Host: upload.wikimedia.org"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.X-Cache-Int ~ "miss"
+} -run
+
+varnish v1 -expect cache_hitpass == 1
+
+client c4 {
+ txreq -url "/test-miss-hit" -hdr "Host: upload.wikimedia.org"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.X-Cache-Int ~ "hit/1"
+} -run
+
+varnish v1 -expect cache_hitpass == 1
diff --git a/modules/varnish/templates/upload-backend.inc.vcl.erb
b/modules/varnish/templates/upload-backend.inc.vcl.erb
index 66fd12a..6321af8 100644
--- a/modules/varnish/templates/upload-backend.inc.vcl.erb
+++ b/modules/varnish/templates/upload-backend.inc.vcl.erb
@@ -3,7 +3,7 @@
sub pick_stevedore {
// Select a storage size class/bin
- // See T145661 for storage binning rationale
+ // See T145661 for storage binning rationale
if (beresp.http.Content-Length !~ "^[0-9]+$") { // even possible on
upload?
set beresp.storage_hint = "bin1";
}
@@ -56,6 +56,13 @@
sub cluster_be_backend_response_early { }
sub cluster_be_backend_response {
+ // hit-for-pass objects < 256B size (T145661)
+ if (std.integer(beresp.http.Content-Length, 256) < 256 && beresp.status
== 200) {
+ set beresp.http.X-CDIS = "pass";
+ set beresp.uncacheable = true;
+ return (deliver);
+ }
+
call pick_stevedore;
if (beresp.http.Content-Range) {
diff --git a/modules/varnish/templates/upload-frontend.inc.vcl.erb
b/modules/varnish/templates/upload-frontend.inc.vcl.erb
index 97cccc0..c4c273a 100644
--- a/modules/varnish/templates/upload-frontend.inc.vcl.erb
+++ b/modules/varnish/templates/upload-frontend.inc.vcl.erb
@@ -71,9 +71,12 @@
// hit/4 or higher, deliver the object as normal but do not create a new
// cache entry of any kind. We start caching in the frontend when an
// object is accessed for the 5th time across all frontends in this DC.
+ // "Small" objects are excluded here as they result in hit-for-pass at
the
+ // backend layer and thus would always be considered one-hit-wonders.
if (beresp.status == 200
&& bereq.http.X-CDIS == "miss"
- && beresp.http.X-Cache-Int !~ " hit/([4-9]|[0-9]{2,})$") {
+ && beresp.http.X-Cache-Int !~ " hit/([4-9]|[0-9]{2,})$"
+ && std.integer(beresp.http.Content-Length, 256) >= 256) {
set beresp.ttl = 0s;
set beresp.uncacheable = true;
return (deliver);
--
To view, visit https://gerrit.wikimedia.org/r/349188
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I74688d43748a5b1eaf0d3ed2a763b3d093f5f109
Gerrit-PatchSet: 6
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Ema <[email protected]>
Gerrit-Reviewer: BBlack <[email protected]>
Gerrit-Reviewer: Ema <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
