Commit 6519159a authored by Poul-Henning Kamp's avatar Poul-Henning Kamp

Change how min/max/default values for parameters are managed.

Make min/max values strings, and test-convert them along with the
default value during startup, to see that we can.  At the same time
normalize the strings with the parameters tweak function.

List min/max values in .rst doc.

param.show will flag default values. (Better the other way around ?)

Add a new feature:

	param.show changed

will only list parameters with non-default values (in short format)
parent d36344d3
......@@ -204,34 +204,51 @@ mcf_param_show(struct cli *cli, const char * const *av, void *priv)
{
int i;
const struct parspec *pp;
int lfmt;
int lfmt = 0, chg = 0;
struct vsb *vsb;
vsb = VSB_new_auto();
(void)priv;
if (av[2] == NULL)
lfmt = 0;
else
if (av[2] != NULL && !strcmp(av[2], "changed"))
chg = 1;
else if (av[2] != NULL)
lfmt = 1;
for (i = 0; i < nparspec; i++) {
pp = parspecs[i];
if (av[2] != NULL &&
strcmp(pp->name, av[2]) &&
strcmp("-l", av[2]))
if (lfmt && strcmp(pp->name, av[2]) && strcmp("-l", av[2]))
continue;
VSB_clear(vsb);
if (pp->func(vsb, pp, NULL))
VCLI_SetResult(cli, CLIS_PARAM);
AZ(VSB_finish(vsb));
if (chg && pp->def != NULL && !strcmp(pp->def, VSB_data(vsb)))
continue;
if (lfmt) {
VCLI_Out(cli, "%s\n", pp->name);
VCLI_Out(cli, "%-*sValue is: ", margin1, " ");
} else {
VCLI_Out(cli, "%-*s", margin2, pp->name);
}
if (pp->func(cli->sb, pp, NULL))
VCLI_SetResult(cli, CLIS_PARAM);
VCLI_Out(cli, "%s", VSB_data(vsb));
if (pp->units != NULL && *pp->units != '\0')
VCLI_Out(cli, " [%s]\n", pp->units);
else
VCLI_Out(cli, "\n");
if (av[2] != NULL) {
VCLI_Out(cli, "%-*sDefault is: %s\n\n",
VCLI_Out(cli, " [%s]", pp->units);
if (pp->def != NULL && !strcmp(pp->def, VSB_data(vsb)))
VCLI_Out(cli, " (default)");
VCLI_Out(cli, "\n");
if (lfmt) {
VCLI_Out(cli, "%-*sDefault is: %s\n",
margin1, "", pp->def);
if (pp->min != NULL)
VCLI_Out(cli, "%-*sMinimum is: %s\n",
margin1, "", pp->min);
if (pp->max != NULL)
VCLI_Out(cli, "%-*sMaximum is: %s\n",
margin1, "", pp->max);
VCLI_Out(cli, "\n");
mcf_wrap(cli, pp->descr);
if (pp->flags & OBJ_STICKY)
mcf_wrap(cli, OBJ_STICKY_TEXT);
......@@ -247,16 +264,14 @@ mcf_param_show(struct cli *cli, const char * const *av, void *priv)
mcf_wrap(cli, WIZARD_TEXT);
if (pp->flags & PROTECTED)
mcf_wrap(cli, PROTECTED_TEXT);
if (!lfmt)
return;
else
VCLI_Out(cli, "\n\n");
VCLI_Out(cli, "\n\n");
}
}
if (av[2] != NULL && !lfmt) {
if (av[2] != NULL && !lfmt && !chg) {
VCLI_SetResult(cli, CLIS_PARAM);
VCLI_Out(cli, "Unknown parameter \"%s\".", av[2]);
}
VSB_delete(vsb);
}
/*--------------------------------------------------------------------
......@@ -381,39 +396,60 @@ MCF_AddParams(struct parspec *ps)
qsort (parspecs, nparspec, sizeof parspecs[0], mcf_parspec_cmp);
}
/*--------------------------------------------------------------------
* Set defaults for all parameters
* Wash a min/max/default value
*/
static void
mcf_wash_param(struct cli *cli, const struct parspec *pp, const char **val,
const char *name, struct vsb *vsb)
{
int err;
AN(*val);
VSB_clear(vsb);
VSB_printf(vsb, "FAILED to set %s for param %s = %s\n",
name, pp->name, *val);
err = pp->func(vsb, pp, *val);
AZ(VSB_finish(vsb));
if (err) {
VCLI_Out(cli, "%s", VSB_data(vsb));
VCLI_SetResult(cli, CLIS_CANT);
return;
}
VSB_clear(vsb);
err = pp->func(vsb, pp, NULL);
AZ(err);
AZ(VSB_finish(vsb));
if (strcmp(*val, VSB_data(vsb))) {
*val = strdup(VSB_data(vsb));
AN(*val);
}
}
/*--------------------------------------------------------------------
* Wash the min/max/default values, and leave the default set.
*/
void
MCF_InitParams(struct cli *cli)
{
const struct parspec *pp;
int i, j, err;
struct parspec *pp;
int i;
struct vsb *vsb;
/*
* We try to set the default twice, and only failures the
* second time around are fatal. This allows for trivial
* interdependencies.
*/
vsb = VSB_new_auto();
AN(vsb);
for (j = 0; j < 2; j++) {
err = 0;
for (i = 0; i < nparspec; i++) {
pp = parspecs[i];
VSB_clear(vsb);
VSB_printf(vsb,
"FAILED to set default for param %s = %s\n",
pp->name, pp->def);
err = pp->func(vsb, pp, pp->def);
AZ(VSB_finish(vsb));
if (err && j) {
VCLI_Out(cli, "%s", VSB_data(vsb));
VCLI_SetResult(cli, CLIS_CANT);
}
}
for (i = 0; i < nparspec; i++) {
pp = parspecs[i];
if (pp->min != NULL)
mcf_wash_param(cli, pp, &pp->min, "Minimum", vsb);
if (pp->max != NULL)
mcf_wash_param(cli, pp, &pp->max, "Maximum", vsb);
AN(pp->def);
mcf_wash_param(cli, pp, &pp->def, "Default", vsb);
}
VSB_delete(vsb);
}
......@@ -462,6 +498,10 @@ MCF_DumpRstParam(void)
if (pp->units != NULL && *pp->units != '\0')
printf("\t* Units: %s\n", pp->units);
printf("\t* Default: %s\n", pp->def);
if (pp->min != NULL)
printf("\t* Minimum: %s\n", pp->min);
if (pp->max != NULL)
printf("\t* Maximum: %s\n", pp->max);
/*
* XXX: we should mark the params with one/two flags
* XXX: that say if ->min/->max are valid, so we
......
......@@ -36,8 +36,8 @@ struct parspec {
const char *name;
tweak_t *func;
volatile void *priv;
double min;
double max;
const char *min;
const char *max;
const char *descr;
int flags;
#define DELAYED_EFFECT (1<<0)
......@@ -54,7 +54,7 @@ struct parspec {
tweak_t tweak_bool;
tweak_t tweak_bytes;
tweak_t tweak_bytes_u;
tweak_t tweak_generic_double;
tweak_t tweak_double;
tweak_t tweak_group;
tweak_t tweak_listen_address;
tweak_t tweak_poolparam;
......@@ -65,8 +65,8 @@ tweak_t tweak_uint;
tweak_t tweak_user;
tweak_t tweak_waiter;
int tweak_generic_uint(struct vsb *vsb,
volatile unsigned *dest, const char *arg, unsigned min, unsigned max);
int tweak_generic_uint(struct vsb *vsb, volatile unsigned *dest,
const char *arg, const char *min, const char *max);
/* mgt_param_tbl.c */
extern struct parspec mgt_parspec[];
......
......@@ -235,13 +235,13 @@ tweak_feature(struct vsb *vsb, const struct parspec *par, const char *arg)
*/
struct parspec VSL_parspec[] = {
{ "vsl_mask", tweak_vsl_mask, NULL, 0, 0,
{ "vsl_mask", tweak_vsl_mask, NULL, NULL, NULL,
"Mask individual VSL messages from being logged.\n"
"\tdefault\tSet default value\n"
"\nUse +/- prefixe in front of VSL tag name, to mask/unmask "
"individual VSL messages.",
0, "default", "" },
{ "debug", tweak_debug, NULL, 0, 0,
{ "debug", tweak_debug, NULL, NULL, NULL,
"Enable/Disable various kinds of debugging.\n"
"\tnone\tDisable all debugging\n\n"
"Use +/- prefix to set/reset individual bits:"
......@@ -249,7 +249,7 @@ struct parspec VSL_parspec[] = {
#include "tbl/debug_bits.h"
#undef DEBUG_BIT
, 0, "none", "" },
{ "feature", tweak_feature, NULL, 0, 0,
{ "feature", tweak_feature, NULL, NULL, NULL,
"Enable/Disable various minor features.\n"
"\tnone\tDisable all features.\n\n"
"Use +/- prefix to enable/disable individual feature:"
......
This diff is collapsed.
......@@ -56,68 +56,77 @@
#include "mgt_cli.h"
/*--------------------------------------------------------------------*/
/*--------------------------------------------------------------------
* Generic handling of double typed parameters
*/
static int
tweak_generic_timeout(struct vsb *vsb, volatile unsigned *dst, const char *arg)
tweak_generic_double(struct vsb *vsb, volatile double *dest,
const char *arg, const char *min, const char *max, const char *fmt)
{
unsigned u;
double u, minv = 0, maxv = 0;
char *p;
if (arg != NULL) {
u = strtoul(arg, NULL, 0);
if (u == 0) {
VSB_printf(vsb, "Timeout must be greater than zero\n");
return (-1);
if (min != NULL) {
p = NULL;
minv = strtod(min, &p);
if (*arg == '\0' || *p != '\0') {
VSB_printf(vsb, "Illegal Min: %s\n", min);
return (-1);
}
}
if (max != NULL) {
p = NULL;
maxv = strtod(max, &p);
if (*arg == '\0' || *p != '\0') {
VSB_printf(vsb, "Illegal Max: %s\n", max);
return (-1);
}
}
*dst = u;
} else
VSB_printf(vsb, "%u", *dst);
return (0);
}
/*--------------------------------------------------------------------*/
int
tweak_timeout(struct vsb *vsb, const struct parspec *par, const char *arg)
{
volatile unsigned *dest;
dest = par->priv;
return (tweak_generic_timeout(vsb, dest, arg));
}
/*--------------------------------------------------------------------*/
static int
tweak_generic_timeout_double(struct vsb *vsb, volatile double *dest,
const char *arg, double min, double max)
{
double u;
char *p;
if (arg != NULL) {
p = NULL;
u = strtod(arg, &p);
if (*arg == '\0' || *p != '\0') {
VSB_printf(vsb, "Not a number(%s)\n", arg);
return (-1);
}
if (u < min) {
if (min != NULL && u < minv) {
VSB_printf(vsb,
"Timeout must be greater or equal to %.g\n", min);
"Timeout must be greater or equal to %s\n", min);
return (-1);
}
if (u > max) {
if (max != NULL && u > maxv) {
VSB_printf(vsb,
"Timeout must be less than or equal to %.g\n", max);
"Timeout must be less than or equal to %s\n", max);
return (-1);
}
*dest = u;
} else
VSB_printf(vsb, "%.6f", *dest);
VSB_printf(vsb, fmt, *dest);
return (0);
}
/*--------------------------------------------------------------------*/
int
tweak_timeout(struct vsb *vsb, const struct parspec *par, const char *arg)
{
int i;
double d;
volatile unsigned *dest;
dest = par->priv;
d = *dest;
i = tweak_generic_double(vsb, &d, arg, par->min, par->max, "%.0f");
if (!i) {
*dest = (unsigned)ceil(d);
}
return (i);
}
/*--------------------------------------------------------------------*/
int
tweak_timeout_double(struct vsb *vsb, const struct parspec *par,
const char *arg)
......@@ -125,45 +134,20 @@ tweak_timeout_double(struct vsb *vsb, const struct parspec *par,
volatile double *dest;
dest = par->priv;
return (tweak_generic_timeout_double(vsb, dest, arg,
par->min, par->max));
return (tweak_generic_double(vsb, dest, arg,
par->min, par->max, "%.3f"));
}
/*--------------------------------------------------------------------*/
int
tweak_generic_double(struct vsb *vsb, const struct parspec *par,
const char *arg)
tweak_double(struct vsb *vsb, const struct parspec *par, const char *arg)
{
volatile double *dest;
char *p;
double u;
dest = par->priv;
if (arg != NULL) {
p = NULL;
u = strtod(arg, &p);
if (*p != '\0') {
VSB_printf(vsb,
"Not a number (%s)\n", arg);
return (-1);
}
if (u < par->min) {
VSB_printf(vsb,
"Must be greater or equal to %.g\n",
par->min);
return (-1);
}
if (u > par->max) {
VSB_printf(vsb,
"Must be less than or equal to %.g\n",
par->max);
return (-1);
}
*dest = u;
} else
VSB_printf(vsb, "%f", *dest);
return (0);
return (tweak_generic_double(vsb, dest, arg,
par->min, par->max, "%g"));
}
/*--------------------------------------------------------------------*/
......@@ -214,12 +198,28 @@ tweak_bool(struct vsb *vsb, const struct parspec *par, const char *arg)
int
tweak_generic_uint(struct vsb *vsb, volatile unsigned *dest, const char *arg,
unsigned min, unsigned max)
const char *min, const char *max)
{
unsigned u;
unsigned u, minv = 0, maxv = 0;
char *p;
if (arg != NULL) {
if (min != NULL) {
p = NULL;
minv = strtoul(min, &p, 0);
if (*arg == '\0' || *p != '\0') {
VSB_printf(vsb, "Illegal Min: %s\n", min);
return (-1);
}
}
if (max != NULL) {
p = NULL;
maxv = strtoul(max, &p, 0);
if (*arg == '\0' || *p != '\0') {
VSB_printf(vsb, "Illegal Max: %s\n", max);
return (-1);
}
}
p = NULL;
if (!strcasecmp(arg, "unlimited"))
u = UINT_MAX;
......@@ -230,12 +230,12 @@ tweak_generic_uint(struct vsb *vsb, volatile unsigned *dest, const char *arg,
return (-1);
}
}
if (u < min) {
VSB_printf(vsb, "Must be at least %u\n", min);
if (min != NULL && u < minv) {
VSB_printf(vsb, "Must be at least %s\n", min);
return (-1);
}
if (u > max) {
VSB_printf(vsb, "Must be no more than %u\n", max);
if (max != NULL && u > maxv) {
VSB_printf(vsb, "Must be no more than %s\n", max);
return (-1);
}
*dest = u;
......@@ -255,9 +255,7 @@ tweak_uint(struct vsb *vsb, const struct parspec *par, const char *arg)
volatile unsigned *dest;
dest = par->priv;
(void)tweak_generic_uint(vsb, dest, arg,
(uint)par->min, (uint)par->max);
return (0);
return (tweak_generic_uint(vsb, dest, arg, par->min, par->max));
}
/*--------------------------------------------------------------------*/
......@@ -287,12 +285,26 @@ fmt_bytes(struct vsb *vsb, uintmax_t t)
static int
tweak_generic_bytes(struct vsb *vsb, volatile ssize_t *dest, const char *arg,
double min, double max)
const char *min, const char *max)
{
uintmax_t r;
uintmax_t r, rmin = 0, rmax = 0;
const char *p;
if (arg != NULL) {
if (min != NULL) {
p = VNUM_2bytes(min, &rmin, 0);
if (p != NULL) {
VSB_printf(vsb, "Invalid min-val: %s\n", min);
return (-1);
}
}
if (max != NULL) {
p = VNUM_2bytes(max, &rmax, 0);
if (p != NULL) {
VSB_printf(vsb, "Invalid max-val: %s\n", max);
return (-1);
}
}
p = VNUM_2bytes(arg, &r, 0);
if (p != NULL) {
VSB_printf(vsb, "Could not convert to bytes.\n");
......@@ -303,19 +315,17 @@ tweak_generic_bytes(struct vsb *vsb, volatile ssize_t *dest, const char *arg,
}
if ((uintmax_t)((ssize_t)r) != r) {
fmt_bytes(vsb, r);
VSB_printf(vsb, " is too large for this architecture.\n");
VSB_printf(vsb,
" is too large for this architecture.\n");
return (-1);
}
if (max != 0. && r > max) {
VSB_printf(vsb, "Must be no more than ");
fmt_bytes(vsb, (uintmax_t)max);
if (max != NULL && r > rmax) {
VSB_printf(vsb, "Must be no more than %s\n", max);
VSB_printf(vsb, "\n");
return (-1);
}
if (r < min) {
VSB_printf(vsb, "Must be at least ");
fmt_bytes(vsb, (uintmax_t)min);
VSB_printf(vsb, "\n");
if (min != NULL && r < rmin) {
VSB_printf(vsb, "Must be at least %s\n", min);
return (-1);
}
*dest = r;
......@@ -332,12 +342,10 @@ tweak_bytes(struct vsb *vsb, const struct parspec *par, const char *arg)
{
volatile ssize_t *dest;
assert(par->min >= 0);
dest = par->priv;
return (tweak_generic_bytes(vsb, dest, arg, par->min, par->max));
}
/*--------------------------------------------------------------------*/
int
......@@ -346,8 +354,6 @@ tweak_bytes_u(struct vsb *vsb, const struct parspec *par, const char *arg)
volatile unsigned *d1;
volatile ssize_t dest;
assert(par->max <= UINT_MAX);
assert(par->min >= 0);
d1 = par->priv;
dest = *d1;
if (tweak_generic_bytes(vsb, &dest, arg, par->min, par->max))
......@@ -568,15 +574,15 @@ tweak_poolparam(struct vsb *vsb, const struct parspec *par, const char *arg)
}
px = *pp;
retval = tweak_generic_uint(vsb, &px.min_pool, av[1],
(uint)par->min, (uint)par->max);
par->min, par->max);
if (retval)
break;
retval = tweak_generic_uint(vsb, &px.max_pool, av[2],
(uint)par->min, (uint)par->max);
par->min, par->max);
if (retval)
break;
retval = tweak_generic_timeout_double(vsb,
&px.max_age, av[3], 0, 1e6);
retval = tweak_generic_double(vsb,
&px.max_age, av[3], "0", "1e6", "%.0f");
if (retval)
break;
if (px.min_pool > px.max_pool) {
......
......@@ -42,7 +42,6 @@
#include "config.h"
#include <limits.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
......@@ -60,7 +59,8 @@ tweak_thread_pool_min(struct vsb *vsb, const struct parspec *par,
{
return (tweak_generic_uint(vsb, &mgt_param.wthread_min, arg,
(unsigned)par->min, mgt_param.wthread_max));
// par->min, mgt_param.wthread_max));
par->min, NULL));
}
/*--------------------------------------------------------------------
......@@ -93,14 +93,15 @@ tweak_thread_pool_max(struct vsb *vsb, const struct parspec *par,
(void)par;
return (tweak_generic_uint(vsb, &mgt_param.wthread_max, arg,
mgt_param.wthread_min, UINT_MAX));
//mgt_param.wthread_min, NULL));
NULL, NULL));
}
/*--------------------------------------------------------------------*/
struct parspec WRK_parspec[] = {
{ "thread_pools", tweak_uint, &mgt_param.wthread_pools,
1, UINT_MAX,
"1", NULL,
"Number of worker thread pools.\n"
"\n"
"Increasing number of worker pools decreases lock "
......@@ -113,7 +114,8 @@ struct parspec WRK_parspec[] = {
"restart to take effect.",
EXPERIMENTAL | DELAYED_EFFECT,
"2", "pools" },
{ "thread_pool_max", tweak_thread_pool_max, NULL, 10, 0,
{ "thread_pool_max", tweak_thread_pool_max, NULL,
"10", NULL,
"The maximum number of worker threads in each pool.\n"
"\n"
"Do not set this higher than you have to, since excess "
......@@ -123,7 +125,8 @@ struct parspec WRK_parspec[] = {
"Minimum is 10 threads.",
DELAYED_EFFECT,
"5000", "threads" },
{ "thread_pool_min", tweak_thread_pool_min, NULL, 10, 0,
{ "thread_pool_min", tweak_thread_pool_min, NULL,
"10", NULL,
"The minimum number of worker threads in each pool.\n"
"\n"
"Increasing this may help ramp up faster from low load "
......@@ -134,7 +137,7 @@ struct parspec WRK_parspec[] = {
"100", "threads" },
{ "thread_pool_timeout",
tweak_timeout_double, &mgt_param.wthread_timeout,
10, UINT_MAX,
"10", NULL,
"Thread idle threshold.\n"
"\n"
"Threads in excess of thread_pool_min, which have been idle "
......@@ -145,7 +148,7 @@ struct parspec WRK_parspec[] = {
"300", "seconds" },
{ "thread_pool_destroy_delay",
tweak_timeout_double, &mgt_param.wthread_destroy_delay,
0.01, UINT_MAX,
"0.01", NULL,
"Wait this long after destroying a thread.\n"
"\n"
"This controls the decay of thread pools when idle(-ish).\n"
......@@ -155,7 +158,7 @@ struct parspec WRK_parspec[] = {
"1", "seconds" },
{ "thread_pool_add_delay",
tweak_timeout_double, &mgt_param.wthread_add_delay,
0, UINT_MAX,
"0", NULL,
"Wait at least this long after creating a thread.\n"
"\n"
"Some (buggy) systems may need a short (sub-second) "
......@@ -168,7 +171,7 @@ struct parspec WRK_parspec[] = {
"0", "seconds" },
{ "thread_pool_fail_delay",
tweak_timeout_double, &mgt_param.wthread_fail_delay,
10e-3, UINT_MAX,
"10e-3", NULL,
"Wait at least this long after a failed thread creation "
"before trying to create another thread.\n"
"\n"
......@@ -186,7 +189,8 @@ struct parspec WRK_parspec[] = {
EXPERIMENTAL,
"0.2", "seconds" },
{ "thread_stats_rate",
tweak_uint, &mgt_param.wthread_stats_rate, 0, UINT_MAX,
tweak_uint, &mgt_param.wthread_stats_rate,
"0", NULL,
"Worker threads accumulate statistics, and dump these into "
"the global stats counters if the lock is free when they "
"finish a request.\n"
......@@ -196,7 +200,7 @@ struct parspec WRK_parspec[] = {
EXPERIMENTAL,
"10", "requests" },
{ "thread_queue_limit", tweak_uint, &mgt_param.wthread_queue_limit,
0, UINT_MAX,
"0", NULL,
"Permitted queue length per thread-pool.\n"
"\n"
"This sets the number of requests we will queue, waiting "
......@@ -204,7 +208,8 @@ struct parspec WRK_parspec[] = {
"be dropped instead of queued.",
EXPERIMENTAL,
"20", "" },
{ "rush_exponent", tweak_uint, &mgt_param.rush_exponent, 2, UINT_MAX,
{ "rush_exponent", tweak_uint, &mgt_param.rush_exponent,
"2", NULL,
"How many parked request we start for each completed "
"request on the object.\n"
"NB: Even with the implict delay of delivery, "
......@@ -213,7 +218,8 @@ struct parspec WRK_parspec[] = {
EXPERIMENTAL,
"3", "requests per request" },
{ "thread_pool_stack",
tweak_stack_size, &mgt_param.wthread_stacksize, 0, UINT_MAX,
tweak_stack_size, &mgt_param.wthread_stacksize,
"0", NULL,
"Worker thread stack size.\n"
"This is likely rounded up to a multiple of 4k by the kernel.\n"
"The kernel/OS has a lower limit which will be enforced.",
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment