Skip to content

Commit

Permalink
fix regression: ban-lurker skips objects
Browse files Browse the repository at this point in the history
93d8050 made execution of
ban_lurker_test_ban() conditional on bd != b, which effectively caused
objects hanging off bans below request bans to not get tested against
relevant bans.

Because object bans (from the obans list) are being marked completed,
the objects which were skipped would also be missed to get evaluated
against the relevant bans at lookup time unless they were evaluated in
request context. So, in effect, we would simply miss to test bans.

Fixes varnishcache#3007

Maybe related to varnishcache#3006
  • Loading branch information
nigoroll authored and rezan committed Jul 11, 2019
1 parent 51160e7 commit 7091c9e
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 10 deletions.
4 changes: 2 additions & 2 deletions bin/varnishd/cache/cache_ban_lurker.c
Expand Up @@ -304,7 +304,7 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
VSC_C_main->bans_lurker_obj_killed += lok;
VSC_C_main->bans_lurker_obj_killed_cutoff += lokc;
tested = tested_tests = lok = lokc = 0;
if (oc->ban == bt) {
if (oc->ban == bt && bt != bd) {
bt->refcount--;
VTAILQ_REMOVE(&bt->objcore, oc, ban_list);
oc->ban = bd;
Expand Down Expand Up @@ -356,7 +356,7 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
bd = NULL;
VTAILQ_INIT(&obans);
for (; b != NULL; b = VTAILQ_NEXT(b, list)) {
if (bd != NULL && bd != b)
if (bd != NULL)
ban_lurker_test_ban(wrk, vsl, b, &obans, bd,
count > cutoff);
if (b->flags & BANS_FLAG_COMPLETED)
Expand Down
56 changes: 48 additions & 8 deletions bin/varnishtest/tests/c00049.vtc
Expand Up @@ -33,6 +33,15 @@ server s1 {
expect req.url == /4
txresp -hdr "Foo: bar4.1"

rxreq
expect req.url == /r1
txresp
rxreq
expect req.url == /r2
txresp
rxreq
expect req.url == /r3
txresp
} -start

varnish v1 -vcl+backend {} -start
Expand Down Expand Up @@ -132,8 +141,8 @@ varnish v1 -expect bans_deleted == 2
varnish v1 -expect bans_tested == 0
varnish v1 -expect bans_tests_tested == 0
varnish v1 -expect bans_obj_killed == 0
varnish v1 -expect bans_lurker_tested == 8
varnish v1 -expect bans_lurker_tests_tested == 9
varnish v1 -expect bans_lurker_tested == 10
varnish v1 -expect bans_lurker_tests_tested == 11
varnish v1 -expect bans_lurker_obj_killed == 4
varnish v1 -expect bans_dups == 0

Expand All @@ -157,8 +166,8 @@ varnish v1 -expect bans_deleted == 2
varnish v1 -expect bans_tested == 1
varnish v1 -expect bans_tests_tested == 1
varnish v1 -expect bans_obj_killed == 0
varnish v1 -expect bans_lurker_tested == 8
varnish v1 -expect bans_lurker_tests_tested == 9
varnish v1 -expect bans_lurker_tested == 10
varnish v1 -expect bans_lurker_tests_tested == 11
varnish v1 -expect bans_lurker_obj_killed == 4
varnish v1 -expect bans_dups == 0

Expand All @@ -182,8 +191,8 @@ varnish v1 -expect bans_deleted == 5
varnish v1 -expect bans_tested == 2
varnish v1 -expect bans_tests_tested == 2
varnish v1 -expect bans_obj_killed == 1
varnish v1 -expect bans_lurker_tested == 8
varnish v1 -expect bans_lurker_tests_tested == 9
varnish v1 -expect bans_lurker_tested == 10
varnish v1 -expect bans_lurker_tests_tested == 11
varnish v1 -expect bans_lurker_obj_killed == 4
varnish v1 -expect bans_dups == 0

Expand Down Expand Up @@ -217,10 +226,41 @@ varnish v1 -expect bans_deleted == 10
varnish v1 -expect bans_tested == 2
varnish v1 -expect bans_tests_tested == 2
varnish v1 -expect bans_obj_killed == 1
varnish v1 -expect bans_lurker_tested == 8
varnish v1 -expect bans_lurker_tests_tested == 9
varnish v1 -expect bans_lurker_tested == 10
varnish v1 -expect bans_lurker_tests_tested == 11
varnish v1 -expect bans_lurker_obj_killed == 4
varnish v1 -expect bans_lurker_obj_killed_cutoff == 3
varnish v1 -expect bans_dups == 0

varnish v1 -expect n_object == 0

client c1 {
txreq -url /r1
rxresp
} -run

varnish v1 -cliok "ban req.http.nevermatch == 1"

client c1 {
txreq -url /r2
rxresp
} -run

varnish v1 -cliok "ban req.http.nevermatch == 2"

client c1 {
txreq -url /r3
rxresp
} -run

varnish v1 -cliok "ban req.http.nevermatch == 3"

varnish v1 -cliok "ban.list"

varnish v1 -cliok "ban obj.http.status != 0"

delay 1

varnish v1 -cliok "ban.list"

varnish v1 -expect n_object == 0

0 comments on commit 7091c9e

Please sign in to comment.