[master] 4fa4a80 Honor the persisted GONE flag on reload of bans, and keep the duplicate counters consistent on reloads.

Martin Blix Grydeland martin at varnish-cache.org
Fri Feb 15 14:58:00 CET 2013


commit 4fa4a80cc1bce3adb1da79f8024b5643ce5989b3
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Fri Dec 14 18:36:45 2012 +0100

    Honor the persisted GONE flag on reload of bans, and keep the
    duplicate counters consistent on reloads.
    
    Add a test case for this.
    
    Add a section on bans and offline silos in the docs.

diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index eb9e179..2e1488f 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -634,7 +634,7 @@ static void
 ban_reload(const uint8_t *ban, unsigned len)
 {
 	struct ban *b, *b2;
-	int gone = 0;
+	int dup = 0;
 	double t0, t1, t2 = 9e99;
 
 	ASSERT_CLI();
@@ -651,11 +651,8 @@ ban_reload(const uint8_t *ban, unsigned len)
 			return;
 		if (t1 < t0)
 			break;
-		if (ban_equal(b->spec, ban)) {
-			gone |= BAN_F_GONE;
-			VSC_C_main->bans_dups++;
-			VSC_C_main->bans_gone++;
-		}
+		if (ban_equal(b->spec, ban))
+			dup = 1;
 	}
 
 	VSC_C_main->bans++;
@@ -666,11 +663,14 @@ ban_reload(const uint8_t *ban, unsigned len)
 	b2->spec = malloc(len);
 	AN(b2->spec);
 	memcpy(b2->spec, ban, len);
-	b2->flags |= gone;
 	if (ban[BANS_FLAGS] & BANS_FLAG_REQ) {
 		VSC_C_main->bans_req++;
 		b2->flags |= BAN_F_REQ;
 	}
+	if (dup)
+		VSC_C_main->bans_dups++;
+	if (dup || (ban[BANS_FLAGS] & BANS_FLAG_GONE))
+		ban_mark_gone(b2);
 	if (b == NULL)
 		VTAILQ_INSERT_TAIL(&ban_head, b2, list);
 	else
diff --git a/bin/varnishtest/tests/p00009.vtc b/bin/varnishtest/tests/p00009.vtc
new file mode 100644
index 0000000..36196d7
--- /dev/null
+++ b/bin/varnishtest/tests/p00009.vtc
@@ -0,0 +1,56 @@
+varnishtest "Check that reloaded bans with gone flag are really gone on restart"
+
+shell "rm -f ${tmpdir}/_.per[12]"
+
+server s1 {
+	rxreq
+	txresp -hdr "x-foo: foo"
+
+	accept
+	rxreq
+	txresp -hdr "x-foo: bar"
+} -start
+
+varnish v1 \
+	-arg "-pfeature=+wait_silo" \
+	-arg "-pban_lurker_sleep=0" \
+	-storage "-sper1=persistent,${tmpdir}/_.per1,10m -sper2=persistent,${tmpdir}/_.per2,10m" \
+	-vcl+backend {
+	}
+varnish v1 -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.x-foo == "foo"
+} -run
+
+varnish v1 -cliok "ban req.url == /test"
+varnish v1 -cliok "ban req.url == /test"
+varnish v1 -cliok "ban.list"
+
+# Expect ban_magic plus the 2 we added
+varnish v1 -expect bans == 3
+
+# Expect 1 of the 2 added to be marked dup
+varnish v1 -expect bans_dups == 1
+
+# Expect ban_magic plus our 1 dup to be marked gone
+varnish v1 -expect bans_gone == 2
+
+# Restart 
+varnish v1 -stop
+varnish v1 -start
+varnish v1 -cliok "ban.list"
+
+# Check that our object is still there
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.x-foo == "foo"
+} -run
+
+# Expect our duplicate
+varnish v1 -expect bans_dups == 1
+# One more than before restart due to the new ban_magic
+varnish v1 -expect bans_gone == 3
diff --git a/doc/sphinx/include/storage_backends.rst b/doc/sphinx/include/storage_backends.rst
index ec80601..fc646ba 100644
--- a/doc/sphinx/include/storage_backends.rst
+++ b/doc/sphinx/include/storage_backends.rst
@@ -105,6 +105,11 @@ open at any given point in time. Full silos are *sealed*. When Varnish
 starts after a shutdown it will discard the content of any silo that
 isn't sealed.
 
+Note that taking persistent silos offline and at the same time using
+bans can cause problems. This because bans added while the silo was
+offline will not be applied to the silo when it reenters the cache,
+and can make previously banned objects reappear.
+
 Transient Storage
 -----------------
       



More information about the varnish-commit mailing list