r3443 - trunk/varnish-cache/bin/varnishd

phk at projects.linpro.no phk at projects.linpro.no
Tue Nov 25 15:09:39 CET 2008


Author: phk
Date: 2008-11-25 15:09:39 +0100 (Tue, 25 Nov 2008)
New Revision: 3443

Added:
   trunk/varnish-cache/bin/varnishd/mgt_pool.c
Modified:
   trunk/varnish-cache/bin/varnishd/Makefile.am
   trunk/varnish-cache/bin/varnishd/cache_pool.c
   trunk/varnish-cache/bin/varnishd/mgt_param.c
   trunk/varnish-cache/bin/varnishd/vparam.h
Log:
Make it possible to declare paramters in other source files, and move
the thread-pool related params into a new file mgt_pool.c as proof.

The paramters happen in management process context, and should therefore
not end up in cache_* files for namespace and sanity reasons.



Modified: trunk/varnish-cache/bin/varnishd/Makefile.am
===================================================================
--- trunk/varnish-cache/bin/varnishd/Makefile.am	2008-11-25 13:44:52 UTC (rev 3442)
+++ trunk/varnish-cache/bin/varnishd/Makefile.am	2008-11-25 14:09:39 UTC (rev 3443)
@@ -45,6 +45,7 @@
 	mgt_child.c \
 	mgt_cli.c \
 	mgt_param.c \
+	mgt_pool.c \
 	mgt_vcc.c \
 	rfc2616.c \
 	shmlog.c \

Modified: trunk/varnish-cache/bin/varnishd/cache_pool.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache_pool.c	2008-11-25 13:44:52 UTC (rev 3442)
+++ trunk/varnish-cache/bin/varnishd/cache_pool.c	2008-11-25 14:09:39 UTC (rev 3443)
@@ -629,3 +629,5 @@
 	AZ(pthread_create(&tp, NULL, wrk_herder_thread, NULL));
 	AZ(pthread_detach(tp));
 }
+
+/*--------------------------------------------------------------------*/

Modified: trunk/varnish-cache/bin/varnishd/mgt_param.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_param.c	2008-11-25 13:44:52 UTC (rev 3442)
+++ trunk/varnish-cache/bin/varnishd/mgt_param.c	2008-11-25 14:09:39 UTC (rev 3443)
@@ -55,13 +55,26 @@
 #include "vss.h"
 
 #define MAGIC_INIT_STRING	"\001"
-static struct params master;
+struct params master;
 static int nparspec;
 static struct parspec const ** parspec;
 static int margin;
 
 /*--------------------------------------------------------------------*/
 
+static const struct parspec *
+mcf_findpar(const char *name)
+{
+	int i;
+
+	for (i = 0; i < nparspec; i++)
+		if (!strcmp(parspec[i]->name, name)) 
+			return (parspec[i]);
+	return (NULL);
+}
+
+/*--------------------------------------------------------------------*/
+
 static void
 tweak_generic_timeout(struct cli *cli, volatile unsigned *dst, const char *arg)
 {
@@ -99,7 +112,7 @@
 
 /*--------------------------------------------------------------------*/
 
-static void
+void
 tweak_timeout(struct cli *cli, const struct parspec *par, const char *arg)
 {
 	volatile unsigned *dest;
@@ -160,7 +173,7 @@
 
 /*--------------------------------------------------------------------*/
 
-static void
+void
 tweak_generic_uint(struct cli *cli, volatile unsigned *dest, const char *arg,
     unsigned min, unsigned max)
 {
@@ -191,7 +204,7 @@
 
 /*--------------------------------------------------------------------*/
 
-static void
+void
 tweak_uint(struct cli *cli, const struct parspec *par, const char *arg)
 {
 	volatile unsigned *dest;
@@ -283,29 +296,6 @@
 /*--------------------------------------------------------------------*/
 
 static void
-tweak_thread_pool_min(struct cli *cli, const struct parspec *par,
-    const char *arg)
-{
-
-	tweak_generic_uint(cli, &master.wthread_min, arg,
-	    par->umin, master.wthread_max);
-}
-
-/*--------------------------------------------------------------------*/
-
-static void
-tweak_thread_pool_max(struct cli *cli, const struct parspec *par,
-    const char *arg)
-{
-
-	(void)par;
-	tweak_generic_uint(cli, &master.wthread_max, arg,
-	    master.wthread_min, UINT_MAX);
-}
-
-/*--------------------------------------------------------------------*/
-
-static void
 clean_listen_sock_head(struct listen_sock_head *lsh)
 {
 	struct listen_sock *ls, *ls2;
@@ -493,107 +483,6 @@
 		"flush of the cache use \"url.purge .\"",
 		0,
 		"120", "seconds" },
-	{ "thread_pools", tweak_uint, &master.wthread_pools, 1, UINT_MAX,
-		"Number of worker thread pools.\n"
-		"\n"
-		"Increasing number of worker pools decreases lock "
-		"contention.\n"
-		"\n"
-		"Too many pools waste CPU and RAM resources, and more than "
-		"one pool for each CPU is probably detrimal to performance.\n"
-		"\n"
-		"Can be increased on the fly, but decreases require a "
-		"restart to take effect.",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"2", "pools" },
-	{ "thread_pool_max", tweak_thread_pool_max, NULL, 1, 0,
-		"The maximum number of worker threads in all pools combined.\n"
-		"\n"
-		"Do not set this higher than you have to, since excess "
-		"worker threads soak up RAM and CPU and generally just get "
-		"in the way of getting work done.\n",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"500", "threads" },
-	{ "thread_pool_min", tweak_thread_pool_min, NULL, 2, 0,
-		"The minimum number of threads in each worker pool.\n"
-		"\n"
-		"Increasing this may help ramp up faster from low load "
-		"situations where threads have expired.\n"
-		"\n"
-		"Minimum is 2 threads.",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"5", "threads" },
-	{ "thread_pool_timeout", tweak_timeout, &master.wthread_timeout, 1, 0,
-		"Thread idle threshold.\n"
-		"\n"
-		"Threads in excess of thread_pool_min, which have been idle "
-		"for at least this long are candidates for purging.\n"
-		"\n"
-		"Minimum is 1 second.",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"300", "seconds" },
-	{ "thread_pool_purge_delay",
-		tweak_timeout, &master.wthread_purge_delay, 100, 0,
-		"Wait this long between purging threads.\n"
-		"\n"
-		"This controls the decay of thread pools when idle(-ish).\n"
-		"\n"
-		"Minimum is 100 milliseconds.",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"1000", "milliseconds" },
-	{ "thread_pool_add_threshold",
-		tweak_uint, &master.wthread_add_threshold, 0, UINT_MAX,
-		"Overflow threshold for worker thread creation.\n"
-		"\n"
-		"Setting this too low, will result in excess worker threads, "
-		"which is generally a bad idea.\n"
-		"\n"
-		"Setting it too high results in insuffient worker threads.\n",
-		EXPERIMENTAL,
-		"2", "requests" },
-	{ "thread_pool_add_delay",
-		tweak_timeout, &master.wthread_add_delay, 0, UINT_MAX,
-		"Wait at least this long between creating threads.\n"
-		"\n"
-		"Setting this too long results in insuffient worker threads.\n"
-		"\n"
-		"Setting this too short increases the risk of worker "
-		"thread pile-up.\n",
-		EXPERIMENTAL,
-		"20", "milliseconds" },
-	{ "thread_pool_fail_delay",
-		tweak_timeout, &master.wthread_fail_delay, 100, UINT_MAX,
-		"Wait at least this long after a failed thread creation "
-		"before trying to create another thread.\n"
-		"\n"
-		"Failure to create a worker thread is often a sign that "
-		" the end is near, because the process is running out of "
-		"RAM resources for thread stacks.\n"
-		"This delay tries to not rush it on needlessly.\n"
-		"\n"
-		"If thread creation failures are a problem, check that "
-		"thread_pool_max is not too high.\n"
-		"\n"
-		"It may also help to increase thread_pool_timeout and "
-		"thread_pool_min, to reduce the rate at which treads are "
-		"destroyed and later recreated.\n",
-		EXPERIMENTAL,
-		"200", "milliseconds" },
-	{ "overflow_max", tweak_uint, &master.overflow_max, 0, UINT_MAX,
-		"Percentage permitted overflow queue length.\n"
-		"\n"
-		"This sets the ratio of queued requests to worker threads, "
-		"above which sessions will be dropped instead of queued.\n",
-		EXPERIMENTAL,
-		"100", "%" },
-	{ "rush_exponent", tweak_uint, &master.rush_exponent, 2, UINT_MAX,
-		"How many parked request we start for each completed "
-		"request on the object.\n"
-		"NB: Even with the implict delay of delivery, "
-		"this parameter controls an exponential increase in "
-		"number of worker threads.  ",
-		EXPERIMENTAL,
-		"3", "requests per request" },
 	{ "sess_workspace", tweak_uint, &master.sess_workspace, 1024, UINT_MAX,
 		"Bytes of HTTP protocol workspace allocated for sessions. "
 		"This space must be big enough for the entire HTTP protocol "
@@ -961,24 +850,21 @@
 void
 MCF_ParamSet(struct cli *cli, const char *param, const char *val)
 {
-	int i;
 	const struct parspec *pp;
 
-	for (i = 0; i < nparspec; i++) {
-		pp = parspec[i];
-		if (!strcmp(pp->name, param)) {
-			pp->func(cli, pp, val);
-			if (cli->result != CLIS_OK) {
-			} else if (child_pid >= 0 && pp->flags & MUST_RESTART) {
-				cli_out(cli, "Change will take effect"
-				    " when child is restarted");
-			} else if (pp->flags & MUST_RELOAD) {
-				cli_out(cli, "Change will take effect"
-				    " when VCL script is reloaded");
-			}
-			MCF_ParamSync();
-			return;
+	pp = mcf_findpar(param);
+	if (pp != NULL) {
+		pp->func(cli, pp, val);
+		if (cli->result != CLIS_OK) {
+		} else if (child_pid >= 0 && pp->flags & MUST_RESTART) {
+			cli_out(cli, "Change will take effect"
+			    " when child is restarted");
+		} else if (pp->flags & MUST_RELOAD) {
+			cli_out(cli, "Change will take effect"
+			    " when VCL script is reloaded");
 		}
+		MCF_ParamSync();
+		return;
 	}
 	cli_result(cli, CLIS_PARAM);
 	cli_out(cli, "Unknown parameter \"%s\".", param);
@@ -1015,6 +901,8 @@
 
 	n = 0;
 	for (pp = ps; pp->name != NULL; pp++) {
+		if (mcf_findpar(pp->name) != NULL)
+			fprintf(stderr, "Duplicate param: %s\n", pp->name);
 		if (strlen(pp->name) + 1 > margin)
 			margin = strlen(pp->name) + 1;
 		n++;
@@ -1054,6 +942,7 @@
 {
 
 	MCF_AddParams(input_parspec);
+	MCF_AddParams(WRK_parspec);
 
 	/* XXX: We do this twice, to get past any interdependencies */
 	MCF_SetDefaults(NULL);

Added: trunk/varnish-cache/bin/varnishd/mgt_pool.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/mgt_pool.c	                        (rev 0)
+++ trunk/varnish-cache/bin/varnishd/mgt_pool.c	2008-11-25 14:09:39 UTC (rev 3443)
@@ -0,0 +1,184 @@
+/*-
+ * Copyright (c) 2006 Verdens Gang AS
+ * Copyright (c) 2006-2008 Linpro AS
+ * All rights reserved.
+ *
+ * Author: Poul-Henning Kamp <phk at phk.freebsd.dk>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $Id: cache_pool.c 3429 2008-11-24 17:47:21Z phk $
+ *
+ * We maintain a number of worker thread pools, to spread lock contention.
+ *
+ * Pools can be added on the fly, as a means to mitigate lock contention,
+ * but can only be removed again by a restart. (XXX: we could fix that)
+ *
+ * Two threads herd the pools, one eliminates idle threads and aggregates
+ * statistics for all the pools, the other thread creates new threads
+ * on demand, subject to various numerical constraints.
+ *
+ * The algorithm for when to create threads needs to be reactive enough
+ * to handle startup spikes, but sufficiently attenuated to not cause
+ * thread pileups.  This remains subject for improvement.
+ */
+
+#include "config.h"
+#include <limits.h>
+#include <sys/types.h>
+
+#include "cli_priv.h"
+#include "mgt.h"
+
+#include "vparam.h"
+#include "heritage.h"
+
+/*--------------------------------------------------------------------*/
+
+static void
+tweak_thread_pool_min(struct cli *cli, const struct parspec *par,
+    const char *arg)
+{
+
+	tweak_generic_uint(cli, &master.wthread_min, arg,
+	    par->umin, master.wthread_max);
+}
+
+/*--------------------------------------------------------------------*/
+
+static void
+tweak_thread_pool_max(struct cli *cli, const struct parspec *par,
+    const char *arg)
+{
+
+	(void)par;
+	tweak_generic_uint(cli, &master.wthread_max, arg,
+	    master.wthread_min, UINT_MAX);
+}
+
+/*--------------------------------------------------------------------*/
+
+const struct parspec WRK_parspec[] = {
+	{ "thread_pools", tweak_uint, &master.wthread_pools, 1, UINT_MAX,
+		"Number of worker thread pools.\n"
+		"\n"
+		"Increasing number of worker pools decreases lock "
+		"contention.\n"
+		"\n"
+		"Too many pools waste CPU and RAM resources, and more than "
+		"one pool for each CPU is probably detrimal to performance.\n"
+		"\n"
+		"Can be increased on the fly, but decreases require a "
+		"restart to take effect.",
+		EXPERIMENTAL | DELAYED_EFFECT,
+		"2", "pools" },
+	{ "thread_pool_max", tweak_thread_pool_max, NULL, 1, 0,
+		"The maximum number of worker threads in all pools combined.\n"
+		"\n"
+		"Do not set this higher than you have to, since excess "
+		"worker threads soak up RAM and CPU and generally just get "
+		"in the way of getting work done.\n",
+		EXPERIMENTAL | DELAYED_EFFECT,
+		"500", "threads" },
+	{ "thread_pool_min", tweak_thread_pool_min, NULL, 2, 0,
+		"The minimum number of threads in each worker pool.\n"
+		"\n"
+		"Increasing this may help ramp up faster from low load "
+		"situations where threads have expired.\n"
+		"\n"
+		"Minimum is 2 threads.",
+		EXPERIMENTAL | DELAYED_EFFECT,
+		"5", "threads" },
+	{ "thread_pool_timeout", tweak_timeout, &master.wthread_timeout, 1, 0,
+		"Thread idle threshold.\n"
+		"\n"
+		"Threads in excess of thread_pool_min, which have been idle "
+		"for at least this long are candidates for purging.\n"
+		"\n"
+		"Minimum is 1 second.",
+		EXPERIMENTAL | DELAYED_EFFECT,
+		"300", "seconds" },
+	{ "thread_pool_purge_delay",
+		tweak_timeout, &master.wthread_purge_delay, 100, 0,
+		"Wait this long between purging threads.\n"
+		"\n"
+		"This controls the decay of thread pools when idle(-ish).\n"
+		"\n"
+		"Minimum is 100 milliseconds.",
+		EXPERIMENTAL | DELAYED_EFFECT,
+		"1000", "milliseconds" },
+	{ "thread_pool_add_threshold",
+		tweak_uint, &master.wthread_add_threshold, 0, UINT_MAX,
+		"Overflow threshold for worker thread creation.\n"
+		"\n"
+		"Setting this too low, will result in excess worker threads, "
+		"which is generally a bad idea.\n"
+		"\n"
+		"Setting it too high results in insuffient worker threads.\n",
+		EXPERIMENTAL,
+		"2", "requests" },
+	{ "thread_pool_add_delay",
+		tweak_timeout, &master.wthread_add_delay, 0, UINT_MAX,
+		"Wait at least this long between creating threads.\n"
+		"\n"
+		"Setting this too long results in insuffient worker threads.\n"
+		"\n"
+		"Setting this too short increases the risk of worker "
+		"thread pile-up.\n",
+		EXPERIMENTAL,
+		"20", "milliseconds" },
+	{ "thread_pool_fail_delay",
+		tweak_timeout, &master.wthread_fail_delay, 100, UINT_MAX,
+		"Wait at least this long after a failed thread creation "
+		"before trying to create another thread.\n"
+		"\n"
+		"Failure to create a worker thread is often a sign that "
+		" the end is near, because the process is running out of "
+		"RAM resources for thread stacks.\n"
+		"This delay tries to not rush it on needlessly.\n"
+		"\n"
+		"If thread creation failures are a problem, check that "
+		"thread_pool_max is not too high.\n"
+		"\n"
+		"It may also help to increase thread_pool_timeout and "
+		"thread_pool_min, to reduce the rate at which treads are "
+		"destroyed and later recreated.\n",
+		EXPERIMENTAL,
+		"200", "milliseconds" },
+	{ "overflow_max", tweak_uint, &master.overflow_max, 0, UINT_MAX,
+		"Percentage permitted overflow queue length.\n"
+		"\n"
+		"This sets the ratio of queued requests to worker threads, "
+		"above which sessions will be dropped instead of queued.\n",
+		EXPERIMENTAL,
+		"100", "%" },
+	{ "rush_exponent", tweak_uint, &master.rush_exponent, 2, UINT_MAX,
+		"How many parked request we start for each completed "
+		"request on the object.\n"
+		"NB: Even with the implict delay of delivery, "
+		"this parameter controls an exponential increase in "
+		"number of worker threads.  ",
+		EXPERIMENTAL,
+		"3", "requests per request" },
+	{ NULL, NULL, NULL }
+};
+

Modified: trunk/varnish-cache/bin/varnishd/vparam.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/vparam.h	2008-11-25 13:44:52 UTC (rev 3442)
+++ trunk/varnish-cache/bin/varnishd/vparam.h	2008-11-25 14:09:39 UTC (rev 3443)
@@ -48,3 +48,14 @@
 	const char	*def;
 	const char	*units;
 };
+
+void tweak_generic_uint(struct cli *cli,
+    volatile unsigned *dest, const char *arg, unsigned min, unsigned max);
+void tweak_uint(struct cli *cli, const struct parspec *par, const char *arg);
+void tweak_timeout(struct cli *cli,
+    const struct parspec *par, const char *arg);
+
+extern struct params master;
+
+/* mgt_pool.c */
+extern const struct parspec WRK_parspec[];



More information about the varnish-commit mailing list