Ema has submitted this change and it was merged.
Change subject: Reduce vcl_error redundancy
......................................................................
Reduce vcl_error redundancy
Currently all cluster-specific vlc_error definitions call 'return
(deliver)' once they're done with sub errorpage. Further, the
cluster-independent version of vcl_error short-circuits on backend
generated 400s and 413s to immediately deliver the page.
Here we try to simplify the logic a bit by essentially including the
400/413 check in sub errorpage and always calling deliver at the end of
errorpage itself. This way we can remove all the redundant
'return(deliver)' from cluster-specific versions of vcl_error.
Change-Id: Ib93e1444d19ceac6279bc4c8b2343a94e20ee341
---
M modules/varnish/templates/vcl/wikimedia.vcl.erb
M templates/varnish/errorpage.inc.vcl.erb
M templates/varnish/maps-backend.inc.vcl.erb
M templates/varnish/maps-frontend.inc.vcl.erb
M templates/varnish/misc-backend.inc.vcl.erb
M templates/varnish/misc-frontend.inc.vcl.erb
M templates/varnish/text-backend.inc.vcl.erb
M templates/varnish/text-frontend.inc.vcl.erb
M templates/varnish/upload-backend.inc.vcl.erb
M templates/varnish/upload-frontend.inc.vcl.erb
10 files changed, 2 insertions(+), 14 deletions(-)
Approvals:
Ema: Verified; Looks good to me, approved
BBlack: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/modules/varnish/templates/vcl/wikimedia.vcl.erb
b/modules/varnish/templates/vcl/wikimedia.vcl.erb
index 62e4e51..bb2b909 100644
--- a/modules/varnish/templates/vcl/wikimedia.vcl.erb
+++ b/modules/varnish/templates/vcl/wikimedia.vcl.erb
@@ -608,10 +608,6 @@
call https_error_redirect;
<% end -%>
- if (obj.status == 400 || obj.status == 413) {
- return(deliver);
- }
-
<% if scope.function_hiera(["cluster"]) != "cache_parsoid" -%>
<% if @vcl_config.fetch("layer", "") == "frontend" -%>
// retry 503 once in frontend instances, to paper over transient issues
diff --git a/templates/varnish/errorpage.inc.vcl.erb
b/templates/varnish/errorpage.inc.vcl.erb
index b1e1c90..27bc4ea 100644
--- a/templates/varnish/errorpage.inc.vcl.erb
+++ b/templates/varnish/errorpage.inc.vcl.erb
@@ -1,8 +1,8 @@
sub errorpage {
- if (obj.status >= 400) {
+ if (obj.status > 400 && obj.status != 413) {
call synth_errorpage;
- return (deliver);
}
+ return (deliver);
}
sub synth_errorpage {
diff --git a/templates/varnish/maps-backend.inc.vcl.erb
b/templates/varnish/maps-backend.inc.vcl.erb
index 49c8995..efa318d 100644
--- a/templates/varnish/maps-backend.inc.vcl.erb
+++ b/templates/varnish/maps-backend.inc.vcl.erb
@@ -9,5 +9,4 @@
sub vcl_error {
call errorpage;
- return (deliver);
}
diff --git a/templates/varnish/maps-frontend.inc.vcl.erb
b/templates/varnish/maps-frontend.inc.vcl.erb
index 89c4b80..f25699a 100644
--- a/templates/varnish/maps-frontend.inc.vcl.erb
+++ b/templates/varnish/maps-frontend.inc.vcl.erb
@@ -18,5 +18,4 @@
sub vcl_error {
call errorpage;
- return (deliver);
}
diff --git a/templates/varnish/misc-backend.inc.vcl.erb
b/templates/varnish/misc-backend.inc.vcl.erb
index d9b7087..00649db 100644
--- a/templates/varnish/misc-backend.inc.vcl.erb
+++ b/templates/varnish/misc-backend.inc.vcl.erb
@@ -75,7 +75,6 @@
sub vcl_error {
call errorpage;
- return (deliver);
}
sub vcl_fetch {
diff --git a/templates/varnish/misc-frontend.inc.vcl.erb
b/templates/varnish/misc-frontend.inc.vcl.erb
index 673a9bc..14cbd7c 100644
--- a/templates/varnish/misc-frontend.inc.vcl.erb
+++ b/templates/varnish/misc-frontend.inc.vcl.erb
@@ -42,7 +42,6 @@
}
call errorpage;
- return (deliver);
}
sub vcl_fetch {
diff --git a/templates/varnish/text-backend.inc.vcl.erb
b/templates/varnish/text-backend.inc.vcl.erb
index 11539aa..42b651a 100644
--- a/templates/varnish/text-backend.inc.vcl.erb
+++ b/templates/varnish/text-backend.inc.vcl.erb
@@ -121,5 +121,4 @@
sub vcl_error {
call errorpage;
- return (deliver);
}
diff --git a/templates/varnish/text-frontend.inc.vcl.erb
b/templates/varnish/text-frontend.inc.vcl.erb
index 697a455..1fc6452 100644
--- a/templates/varnish/text-frontend.inc.vcl.erb
+++ b/templates/varnish/text-frontend.inc.vcl.erb
@@ -274,7 +274,6 @@
}
call errorpage;
- return (deliver);
}
sub vcl_deliver {
diff --git a/templates/varnish/upload-backend.inc.vcl.erb
b/templates/varnish/upload-backend.inc.vcl.erb
index 97827fb..2a64fe7 100644
--- a/templates/varnish/upload-backend.inc.vcl.erb
+++ b/templates/varnish/upload-backend.inc.vcl.erb
@@ -85,7 +85,6 @@
sub vcl_error {
call errorpage;
- return (deliver);
}
sub vcl_deliver {
diff --git a/templates/varnish/upload-frontend.inc.vcl.erb
b/templates/varnish/upload-frontend.inc.vcl.erb
index e6bb484..83aedeb 100644
--- a/templates/varnish/upload-frontend.inc.vcl.erb
+++ b/templates/varnish/upload-frontend.inc.vcl.erb
@@ -97,7 +97,6 @@
}
call errorpage;
- return (deliver);
}
sub vcl_deliver {
--
To view, visit https://gerrit.wikimedia.org/r/271961
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib93e1444d19ceac6279bc4c8b2343a94e20ee341
Gerrit-PatchSet: 2
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Ema <e...@wikimedia.org>
Gerrit-Reviewer: BBlack <bbl...@wikimedia.org>
Gerrit-Reviewer: Ema <e...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
