[master] 128fb32 Kick the ban_lurker into action whenever we do something that makes work for it. Have it sleep as long as possible, rather than hot-polling for work to do.
Poul-Henning Kamp
phk at FreeBSD.org
Fri Oct 23 00:49:02 CEST 2015
commit 128fb323827894042561c7d62286a7e471cacf27
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Thu Oct 22 22:17:34 2015 +0000
Kick the ban_lurker into action whenever we do something that
makes work for it. Have it sleep as long as possible, rather
than hot-polling for work to do.
diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index 6d66999..3bf9073 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -552,6 +552,9 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
b0->refcount++;
}
+ if (oc->ban->refcount == 0 && VTAILQ_NEXT(oc->ban, list) == NULL)
+ ban_kick_lurker();
+
Lck_Unlock(&ban_mtx);
if (b == oc->ban) { /* not banned */
@@ -703,6 +706,7 @@ ccf_ban_list(struct cli *cli, const char * const *av, void *priv)
Lck_Lock(&ban_mtx);
bl->refcount--;
+ ban_kick_lurker(); // XXX: Mostly for testcase b00009.vtc
Lck_Unlock(&ban_mtx);
}
@@ -775,8 +779,10 @@ BAN_Shutdown(void)
{
void *status;
+ Lck_Lock(&ban_mtx);
ban_shutdown = 1;
ban_kick_lurker();
+ Lck_Unlock(&ban_mtx);
AZ(pthread_join(ban_thread, &status));
AZ(status);
diff --git a/bin/varnishd/cache/cache_ban_build.c b/bin/varnishd/cache/cache_ban_build.c
index b807c9a..801774f 100644
--- a/bin/varnishd/cache/cache_ban_build.c
+++ b/bin/varnishd/cache/cache_ban_build.c
@@ -319,6 +319,8 @@ BAN_Commit(struct ban_proto *bp)
}
}
}
+ if (!(b->flags & BANS_FLAG_REQ))
+ ban_kick_lurker();
Lck_Unlock(&ban_mtx);
BAN_Abandon(bp);
diff --git a/bin/varnishd/cache/cache_ban_lurker.c b/bin/varnishd/cache/cache_ban_lurker.c
index 821d8c8..352b219 100644
--- a/bin/varnishd/cache/cache_ban_lurker.c
+++ b/bin/varnishd/cache/cache_ban_lurker.c
@@ -45,10 +45,10 @@ pthread_cond_t ban_lurker_cond;
void
ban_kick_lurker(void)
{
- Lck_Lock(&ban_mtx);
+
+ Lck_AssertHeld(&ban_mtx);
ban_generation++;
AZ(pthread_cond_signal(&ban_lurker_cond));
- Lck_Unlock(&ban_mtx);
}
static void
@@ -191,14 +191,17 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
* Ban lurker thread
*/
-static int
+static double
ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
{
struct ban *b, *bt;
struct banhead_s obans;
- double d;
+ double d, dt, n;
int i;
+ dt = 49.62; // Random, non-magic
+ if (cache_param->ban_lurker_sleep == 0)
+ return (dt);
/* Make a list of the bans we can do something about */
VTAILQ_INIT(&obans);
Lck_Lock(&ban_mtx);
@@ -206,26 +209,27 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
Lck_Unlock(&ban_mtx);
i = 0;
d = VTIM_real() - cache_param->ban_lurker_age;
- while (b != NULL) {
- if (b->flags & BANS_FLAG_COMPLETED) {
- ;
- } else if (b->flags & BANS_FLAG_REQ) {
- ;
- } else if (b == VTAILQ_LAST(&ban_head, banhead_s)) {
- ;
- } else if (ban_time(b->spec) > d) {
- ;
- } else {
+ for (; b != NULL; b = VTAILQ_NEXT(b, list)) {
+ if (b->flags & BANS_FLAG_COMPLETED)
+ continue;
+ if (b->flags & BANS_FLAG_REQ)
+ continue;
+ if (b == VTAILQ_LAST(&ban_head, banhead_s))
+ continue; // XXX: why ?
+ n = ban_time(b->spec) - d;
+ if (n < 0) {
VTAILQ_INSERT_TAIL(&obans, b, l_list);
i++;
+ } else if (n < dt) {
+ dt = n;
}
- b = VTAILQ_NEXT(b, list);
}
if (DO_DEBUG(DBG_LURKER))
- VSLb(vsl, SLT_Debug, "lurker: %d actionable bans", i);
+ VSLb(vsl, SLT_Debug, "lurker: %d actionable bans, dt = %lf", i, dt);
if (i == 0)
- return (0);
+ return (dt);
+ dt = cache_param->ban_lurker_sleep;
/* Go though all the bans to test the objects */
VTAILQ_FOREACH_REVERSE(bt, &ban_head, banhead_s, list) {
if (bt == VTAILQ_LAST(&obans, banhead_s)) {
@@ -247,7 +251,7 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
if (VTAILQ_EMPTY(&obans))
break;
}
- return (1);
+ return (dt);
}
void * __match_proto__(bgthread_t)
@@ -255,6 +259,7 @@ ban_lurker(struct worker *wrk, void *priv)
{
struct vsl_log vsl;
volatile double d;
+ unsigned gen = ban_generation + 1;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
AZ(priv);
@@ -262,13 +267,17 @@ ban_lurker(struct worker *wrk, void *priv)
VSL_Setup(&vsl, NULL, 0);
while (!ban_shutdown) {
- d = cache_param->ban_lurker_sleep;
- if (d <= 0.0 || !ban_lurker_work(wrk, &vsl))
- d = 0.609; // Random, non-magic
+ d = ban_lurker_work(wrk, &vsl);
+ if (DO_DEBUG(DBG_LURKER))
+ VSLb(&vsl, SLT_Debug, "lurker: sleep = %lf", d);
ban_cleantail();
d += VTIM_real();
Lck_Lock(&ban_mtx);
- (void)Lck_CondWait(&ban_lurker_cond, &ban_mtx, d);
+ if (gen == ban_generation) {
+ (void)Lck_CondWait(&ban_lurker_cond, &ban_mtx, d);
+ ban_batch = 0;
+ }
+ gen = ban_generation;
Lck_Unlock(&ban_mtx);
}
pthread_exit(0);
diff --git a/bin/varnishtest/tests/c00049.vtc b/bin/varnishtest/tests/c00049.vtc
index 40f7b94..8d10e68 100644
--- a/bin/varnishtest/tests/c00049.vtc
+++ b/bin/varnishtest/tests/c00049.vtc
@@ -40,6 +40,7 @@ varnish v1 -vcl+backend {} -start
varnish v1 -cliok "param.set ban_lurker_age 0"
varnish v1 -cliok "param.set ban_lurker_sleep 0"
varnish v1 -cliok "param.set debug +lurker"
+varnish v1 -cliok "param.set debug +syncvsl"
client c1 {
@@ -114,6 +115,8 @@ varnish v1 -expect bans_dups == 0
varnish v1 -cliok "param.set ban_lurker_sleep .01"
+varnish v1 -cliok "ban.list"
+
delay 2
varnish v1 -cliok "ban.list"
diff --git a/bin/varnishtest/tests/p00009.vtc b/bin/varnishtest/tests/p00009.vtc
index 6ec15d4..21d65a6 100644
--- a/bin/varnishtest/tests/p00009.vtc
+++ b/bin/varnishtest/tests/p00009.vtc
@@ -53,5 +53,4 @@ client c1 {
# Expect our duplicate
varnish v1 -expect bans_dups == 1
-# One more than before restart due to the new ban_magic
-varnish v1 -expect bans_completed == 3
+varnish v1 -expect bans_completed == 1
diff --git a/bin/varnishtest/tests/r01030.vtc b/bin/varnishtest/tests/r01030.vtc
index 302b918..3f7343a 100644
--- a/bin/varnishtest/tests/r01030.vtc
+++ b/bin/varnishtest/tests/r01030.vtc
@@ -40,9 +40,9 @@ client c1 {
} -run
#delay 0.1
-varnish v1 -expect bans_lurker_tests_tested == 0
+#varnish v1 -expect bans_lurker_tests_tested == 0
-delay 1.5
+#delay 1.5
varnish v1 -expect bans_lurker_tests_tested >= 1
varnish v1 -cliok "param.set ban_lurker_sleep 5.01"
@@ -58,7 +58,7 @@ client c2 {
} -run
#delay 0.1
-varnish v1 -expect bans_lurker_tests_tested == 1
+#varnish v1 -expect bans_lurker_tests_tested == 1
-delay 1.1
+#delay 1.1
varnish v1 -expect bans_lurker_tests_tested == 2
diff --git a/include/tbl/params.h b/include/tbl/params.h
index d2650f4..1968df4 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -135,10 +135,10 @@ PARAM(
/* units */ "seconds",
/* flags */ 0,
/* s-text */
- "The ban lurker only process bans when they are this old. "
- "When a ban is added, the most frequently hit objects will "
- "get tested against it as part of object lookup. This parameter "
- "prevents the ban-lurker from kicking in, until the rush is over.",
+ "The ban lurker will ignore bans until they are this old. "
+ "When a ban is added, the active traffic will be tested against it "
+ "as part of object lookup. This parameter "
+ "holds the ban-lurker off, until the rush is over.",
/* l-text */ "",
/* func */ NULL
)
@@ -152,9 +152,9 @@ PARAM(
/* units */ NULL,
/* flags */ 0,
/* s-text */
- "The ban lurker slees ${ban_lurker_sleep} after examining this "
- "many objects. Use this to pace the ban-lurker if it eats too "
- "many resources.",
+ "The ban lurker sleeps ${ban_lurker_sleep} after examining this "
+ "many objects."
+ " Use this to pace the ban-lurker if it eats too many resources.",
/* l-text */ "",
/* func */ NULL
)
@@ -169,7 +169,8 @@ PARAM(
/* flags */ 0,
/* s-text */
"How long the ban lurker sleeps after examining ${ban_lurker_batch} "
- "objects.\n"
+ "objects."
+ " Use this to pace the ban-lurker if it eats too many resources.\n"
"A value of zero will disable the ban lurker entirely.",
/* l-text */ "",
/* func */ NULL
More information about the varnish-commit
mailing list