[master] 93a85f5 Rip out all the old backend refcounting code, and make backends unique objects for each VCL which instantiate them. (Instead we share TCP connection pools, which are a lot simpler)
Poul-Henning Kamp
phk at FreeBSD.org
Wed Feb 25 16:21:52 CET 2015
commit 93a85f581bfec27cee69356982ed0c011d094ee2
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Wed Feb 25 15:20:33 2015 +0000
Rip out all the old backend refcounting code, and make backends
unique objects for each VCL which instantiate them. (Instead we
share TCP connection pools, which are a lot simpler)
Spotted one utterly trivial memory leak along the way.
diff --git a/bin/varnishd/cache/cache_backend.c b/bin/varnishd/cache/cache_backend.c
index 6d640b8..d695460 100644
--- a/bin/varnishd/cache/cache_backend.c
+++ b/bin/varnishd/cache/cache_backend.c
@@ -175,15 +175,18 @@ vbe_dir_finish(const struct director *d, struct worker *wrk,
VSLb(bo->vsl, SLT_BackendClose, "%d %s", bo->htc->vbc->fd,
bp->display_name);
VBT_Close(bp->tcp_pool, &bo->htc->vbc);
- VBE_DropRefConn(bp, &bo->acct);
+ Lck_Lock(&bp->mtx);
} else {
VSLb(bo->vsl, SLT_BackendReuse, "%d %s", bo->htc->vbc->fd,
bp->display_name);
Lck_Lock(&bp->mtx);
VSC_C_main->backend_recycle++;
VBT_Recycle(bp->tcp_pool, &bo->htc->vbc);
- VBE_DropRefLocked(bp, &bo->acct);
}
+#define ACCT(foo) bp->vsc->foo += bo->acct.foo;
+#include "tbl/acct_fields_bereq.h"
+#undef ACCT
+ Lck_Unlock(&bp->mtx);
bo->htc->vbc = NULL;
bo->htc = NULL;
}
@@ -356,7 +359,7 @@ VRT_fini_vbe(VRT_CTX, struct director **dp, const struct vrt_backend *vrt)
if (vrt->probe != NULL)
VBP_Remove(be, vrt->probe);
- VBE_DropRefVcl(be);
+ VBE_Drop(be);
free(d->vcl_name);
FREE_OBJ(d);
}
diff --git a/bin/varnishd/cache/cache_backend.h b/bin/varnishd/cache/cache_backend.h
index 4a4945a..1a52232 100644
--- a/bin/varnishd/cache/cache_backend.h
+++ b/bin/varnishd/cache/cache_backend.h
@@ -61,11 +61,11 @@ struct backend {
int refcount;
struct lock mtx;
- char *vcl_name;
+ const char *vcl_name;
char *display_name;
- char *ipv4_addr;
- char *ipv6_addr;
- char *port;
+ const char *ipv4_addr;
+ const char *ipv6_addr;
+ const char *port;
struct suckaddr *ipv4;
struct suckaddr *ipv6;
@@ -104,12 +104,9 @@ struct vbc {
};
/* cache_backend_cfg.c */
-void VBE_DropRefConn(struct backend *, const struct acct_bereq *);
-void VBE_DropRefVcl(struct backend *);
-void VBE_DropRefLocked(struct backend *b, const struct acct_bereq *);
+void VBE_Drop(struct backend *);
unsigned VBE_Healthy(const struct backend *b, double *changed);
struct backend *VBE_AddBackend(const struct vrt_backend *vb);
-void VBE_Poll(void);
/* cache_backend_poll.c */
void VBP_Insert(struct backend *b, struct vrt_backend_probe const *p,
diff --git a/bin/varnishd/cache/cache_backend_cfg.c b/bin/varnishd/cache/cache_backend_cfg.c
index 79968f9..7d81cf8 100644
--- a/bin/varnishd/cache/cache_backend_cfg.c
+++ b/bin/varnishd/cache/cache_backend_cfg.c
@@ -54,17 +54,18 @@ static VTAILQ_HEAD(, backend) backends = VTAILQ_HEAD_INITIALIZER(backends);
/*--------------------------------------------------------------------
*/
-static void
-VBE_Nuke(struct backend *b)
+void
+VBE_Drop(struct backend *b)
{
ASSERT_CLI();
+ CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC);
+
+ b->vsc->vcls--;
VTAILQ_REMOVE(&backends, b, list);
free(b->ipv4);
- free(b->ipv4_addr);
free(b->ipv6);
- free(b->ipv6_addr);
- free(b->port);
+ free(b->display_name);
VSM_Free(b->vsc);
VBT_Rel(&b->tcp_pool);
FREE_OBJ(b);
@@ -72,80 +73,6 @@ VBE_Nuke(struct backend *b)
}
/*--------------------------------------------------------------------
- */
-
-void
-VBE_Poll(void)
-{
- struct backend *b, *b2;
-
- ASSERT_CLI();
- VTAILQ_FOREACH_SAFE(b, &backends, list, b2) {
- assert(
- b->admin_health == ah_healthy ||
- b->admin_health == ah_sick ||
- b->admin_health == ah_probe
- );
- if (b->refcount == 0 && b->probe == NULL)
- VBE_Nuke(b);
- }
-}
-
-/*--------------------------------------------------------------------
- * Drop a reference to a backend.
- * The last reference must come from the watcher in the CLI thread,
- * as only that thread is allowed to clean up the backend list.
- */
-
-void
-VBE_DropRefLocked(struct backend *b, const struct acct_bereq *acct_bereq)
-{
- int i;
-
- CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC);
- assert(b->refcount > 0);
-
- if (acct_bereq != NULL) {
-#define ACCT(foo) \
- b->vsc->foo += acct_bereq->foo;
-#include "tbl/acct_fields_bereq.h"
-#undef ACCT
- }
-
- i = --b->refcount;
- Lck_Unlock(&b->mtx);
- if (i > 0)
- return;
-
- ASSERT_CLI();
- VBE_Nuke(b);
-}
-
-void
-VBE_DropRefVcl(struct backend *b)
-{
-
- CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC);
-
- Lck_Lock(&b->mtx);
- b->vsc->vcls--;
- VBE_DropRefLocked(b, NULL);
-}
-
-void
-VBE_DropRefConn(struct backend *b, const struct acct_bereq *acct_bereq)
-{
-
- CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC);
-
- Lck_Lock(&b->mtx);
- assert(b->n_conn > 0);
- b->n_conn--;
- b->vsc->conn--;
- VBE_DropRefLocked(b, acct_bereq);
-}
-
-/*--------------------------------------------------------------------
* Add a backend/director instance when loading a VCL.
* If an existing backend is matched, grab a refcount and return.
* Else create a new backend structure with reference initialized to one.
@@ -161,27 +88,10 @@ VBE_AddBackend(const struct vrt_backend *vb)
AN(vb->vcl_name);
assert(vb->ipv4_suckaddr != NULL || vb->ipv6_suckaddr != NULL);
- /* Run through the list and see if we already have this backend */
- VTAILQ_FOREACH(b, &backends, list) {
- CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC);
- if (strcmp(b->vcl_name, vb->vcl_name))
- continue;
- if (vb->ipv4_suckaddr != NULL &&
- VSA_Compare(b->ipv4, vb->ipv4_suckaddr))
- continue;
- if (vb->ipv6_suckaddr != NULL &&
- VSA_Compare(b->ipv6, vb->ipv6_suckaddr))
- continue;
- b->refcount++;
- b->vsc->vcls++;
- return (b);
- }
-
/* Create new backend */
ALLOC_OBJ(b, BACKEND_MAGIC);
XXXAN(b);
Lck_New(&b->mtx, lck_backend);
- b->refcount = 1;
bprintf(buf, "%s(%s,%s,%s)",
vb->vcl_name,
@@ -191,16 +101,11 @@ VBE_AddBackend(const struct vrt_backend *vb)
b->vsc = VSM_Alloc(sizeof *b->vsc, VSC_CLASS, VSC_type_vbe, buf);
b->vsc->vcls++;
-
- /*
- * This backend may live longer than the VCL that instantiated it
- * so we cannot simply reference the VCL's copy of things.
- */
- REPLACE(b->vcl_name, vb->vcl_name);
+ b->vcl_name = vb->vcl_name;
REPLACE(b->display_name, buf);
- REPLACE(b->ipv4_addr, vb->ipv4_addr);
- REPLACE(b->ipv6_addr, vb->ipv6_addr);
- REPLACE(b->port, vb->port);
+ b->ipv4_addr = vb->ipv4_addr;
+ b->ipv6_addr = vb->ipv6_addr;
+ b->port = vb->port;
b->tcp_pool = VBT_Ref(vb->vcl_name,
vb->ipv4_suckaddr, vb->ipv6_suckaddr);
@@ -365,7 +270,7 @@ do_list(struct cli *cli, struct backend *b, void *priv)
}
CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC);
- VCLI_Out(cli, "\n%-30s %-6d", b->display_name, b->refcount);
+ VCLI_Out(cli, "\n%-30s", b->display_name);
if (b->admin_health == ah_probe)
VCLI_Out(cli, " %-10s", "probe");
diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index 6bdf3f0..5fbad58 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -39,7 +39,6 @@
#include "cache.h"
#include "common/heritage.h"
-#include "cache_backend.h" // struct vbc
#include "vcli.h"
#include "vcli_common.h"
#include "vcli_priv.h"
@@ -79,7 +78,6 @@ cli_cb_before(const struct cli *cli)
ASSERT_CLI();
VSL(SLT_CLI, 0, "Rd %s", cli->cmd);
VCL_Poll();
- VBE_Poll();
Lck_Lock(&cli_mtx);
}
More information about the varnish-commit
mailing list