Re: [PATCH] MINOR: cli: ability to set per-server maxconn

[Andrew Hayworth]

Many thanks!

On Wed, Oct 28, 2015 at 2:04 AM, Willy Tarreau  wrote:
> On Tue, Oct 27, 2015 at 04:50:35PM -0500, Andrew Hayworth wrote:
>> Ah, thanks - I hadn't thought about the case where connections were
>> queued up. In my tests, I had a very low queue timeout. The code you
>> suggested seems to do the trick. Updated patch below.
>
> Applied, thanks. I fixed a minor indent issue, please be careful next
> time :
>
> if (sv->maxconn == sv->minconn) { // static maxconn
> - sv->maxconn = sv->minconn = v;
> +   sv->maxconn = sv->minconn = v;
> } else { // dynamic maxconn
> - sv->maxconn = v;
> +   sv->maxconn = v;
> }
> Thanks,
> willy
>



-- 
- Andrew Hayworth

Re: [PATCH] MINOR: cli: ability to set per-server maxconn

[Willy Tarreau]

On Tue, Oct 27, 2015 at 04:50:35PM -0500, Andrew Hayworth wrote:
> Ah, thanks - I hadn't thought about the case where connections were
> queued up. In my tests, I had a very low queue timeout. The code you
> suggested seems to do the trick. Updated patch below.

Applied, thanks. I fixed a minor indent issue, please be careful next
time :

if (sv->maxconn == sv->minconn) { // static maxconn
- sv->maxconn = sv->minconn = v;
+   sv->maxconn = sv->minconn = v;
} else { // dynamic maxconn
- sv->maxconn = v;
+   sv->maxconn = v;
}
Thanks,
willy

Re: [PATCH] MINOR: cli: ability to set per-server maxconn

[Andrew Hayworth]

On Tue, Oct 20, 2015 at 1:56 AM, Willy Tarreau  wrote:
> Hi Andrew,
>
> Thanks. Normally you also need to try to dequeue pending connections
> when changing the value, because if you increase the limit, you need
> to open the door for new connections. After changing the value, you
> normally need something like this :
>
> if (may_dequeue_tasks(srv, srv->proxy))
> process_srv_queue(srv);
>
> Regards,
> Willy
>

Ah, thanks - I hadn't thought about the case where connections were
queued up. In my tests, I had a very low queue timeout. The code you
suggested seems to do the trick. Updated patch below.

-- 
- Andrew Hayworth

>From 0bad55c2cdd6d4086c11cd445de309693ec72afa Mon Sep 17 00:00:00 2001
From: Andrew Hayworth 
Date: Tue, 27 Oct 2015 21:46:25 +
Subject: [PATCH] MINOR: cli: ability to set per-server maxconn

This commit adds support for setting a per-server maxconn from the stats
socket. The only really notable part of this commit is that we need to
check if maxconn == minconn before changing things, as this indicates
that we are NOT using dynamic maxconn. When we are not using dynamic
maxconn, we should update maxconn/minconn in lockstep.
---
 doc/management.txt |  5 +
 src/dumpstats.c| 34 +-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/doc/management.txt b/doc/management.txt
index d67988b..a53a953 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1356,6 +1356,11 @@ set maxconn frontend  
   delayed until the threshold is reached. The frontend might be specified by
   either its name or its numeric ID prefixed with a sharp ('#').

+set maxconn server  
+  Dynamically change the specified server's maxconn setting. Any positive
+  value is allowed including zero, but setting values larger than the global
+  maxconn does not make much sense.
+
 set maxconn global 
   Dynamically change the global maxconn setting within the range defined by the
   initial global maxconn setting. If it is increased and connections were
diff --git a/src/dumpstats.c b/src/dumpstats.c
index e80e45c..ef52c2b 100644
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -1646,6 +1646,38 @@ static int stats_sock_parse_request(struct
stream_interface *si, char *line)

  return 1;
  }
+ else if (strcmp(args[2], "server") == 0) {
+ struct server *sv;
+ int v;
+
+ sv = expect_server_admin(s, si, args[3]);
+ if (!sv)
+ return 1;
+
+ if (!*args[4]) {
+ appctx->ctx.cli.msg = "Integer value expected.\n";
+ appctx->st0 = STAT_CLI_PRINT;
+ return 1;
+ }
+
+ v = atoi(args[4]);
+ if (v < 0) {
+ appctx->ctx.cli.msg = "Value out of range.\n";
+ appctx->st0 = STAT_CLI_PRINT;
+ return 1;
+ }
+
+ if (sv->maxconn == sv->minconn) { // static maxconn
+  sv->maxconn = sv->minconn = v;
+ } else { // dynamic maxconn
+  sv->maxconn = v;
+ }
+
+ if (may_dequeue_tasks(sv, sv->proxy))
+ process_srv_queue(sv);
+
+ return 1;
+ }
  else if (strcmp(args[2], "global") == 0) {
  int v;

@@ -1681,7 +1713,7 @@ static int stats_sock_parse_request(struct
stream_interface *si, char *line)
  return 1;
  }
  else {
- appctx->ctx.cli.msg = "'set maxconn' only supports 'frontend' and
'global'.\n";
+ appctx->ctx.cli.msg = "'set maxconn' only supports 'frontend',
'server', and 'global'.\n";
  appctx->st0 = STAT_CLI_PRINT;
  return 1;
  }
--
2.1.3


0001-MINOR-cli-ability-to-set-per-server-maxconn.patch
Description: Binary data

Re: [PATCH] MINOR: cli: ability to set per-server maxconn

[Willy Tarreau]

Hi Andrew,

On Mon, Oct 19, 2015 at 02:23:39PM -0500, Andrew Hayworth wrote:
> In another thread "Dynamically change server maxconn possible?",
> someone raised the possibility of setting a per-server maxconn via the
> stats socket. I believe the below patch implements this functionality.
> 
> I'd appreciate any feedback, since I'm not really familiar with this
> part of the code. However, I've tested it by curling slow endpoints
> (the nginx echo_sleep module, specifically) and can confirm that NOSRV
> is returned appropriately according to whatever maxconn settings are
> set via the socket.

Thanks. Normally you also need to try to dequeue pending connections
when changing the value, because if you increase the limit, you need
to open the door for new connections. After changing the value, you
normally need something like this :

if (may_dequeue_tasks(srv, srv->proxy))
process_srv_queue(srv);

Regards,
Willy

Re: [PATCH] MINOR: cli: ability to set per-server maxconn

[Andrew Hayworth]

Apologies for two posts in a row: this version of the patch includes a
blurb for doc/management.txt as well.

- Andrew Hayworth

>From 6c54812a06706460dd2944ce7d51ea29636ed989 Mon Sep 17 00:00:00 2001
From: Andrew Hayworth 
Date: Mon, 19 Oct 2015 19:15:56 +
Subject: [PATCH] MINOR: cli: ability to set per-server maxconn

This commit adds support for setting a per-server maxconn from the stats
socket. The only really notable part of this commit is that we need to
check if maxconn == minconn before changing things, as this indicates
that we are NOT using dynamic maxconn. When we are not using dynamic
maxconn, we should update maxconn/minconn in lockstep.
---
 doc/management.txt |  5 +
 src/dumpstats.c| 31 ++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/doc/management.txt b/doc/management.txt
index d67988b..a53a953 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1356,6 +1356,11 @@ set maxconn frontend  
   delayed until the threshold is reached. The frontend might be specified by
   either its name or its numeric ID prefixed with a sharp ('#').

+set maxconn server  
+  Dynamically change the specified server's maxconn setting. Any positive
+  value is allowed including zero, but setting values larger than the global
+  maxconn does not make much sense.
+
 set maxconn global 
   Dynamically change the global maxconn setting within the range defined by the
   initial global maxconn setting. If it is increased and connections were
diff --git a/src/dumpstats.c b/src/dumpstats.c
index e80e45c..b2bd13b 100644
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -1646,6 +1646,35 @@ static int stats_sock_parse_request(struct
stream_interface *si, char *line)

  return 1;
  }
+ else if (strcmp(args[2], "server") == 0) {
+ struct server *sv;
+ int v;
+
+ sv = expect_server_admin(s, si, args[3]);
+ if (!sv)
+ return 1;
+
+ if (!*args[4]) {
+ appctx->ctx.cli.msg = "Integer value expected.\n";
+ appctx->st0 = STAT_CLI_PRINT;
+ return 1;
+ }
+
+ v = atoi(args[4]);
+ if (v < 0) {
+ appctx->ctx.cli.msg = "Value out of range.\n";
+ appctx->st0 = STAT_CLI_PRINT;
+ return 1;
+ }
+
+ if (sv->maxconn == sv->minconn) { // static maxconn
+  sv->maxconn = sv->minconn = v;
+ } else { // dynamic maxconn
+  sv->maxconn = v;
+ }
+
+ return 1;
+ }
  else if (strcmp(args[2], "global") == 0) {
  int v;

@@ -1681,7 +1710,7 @@ static int stats_sock_parse_request(struct
stream_interface *si, char *line)
  return 1;
  }
  else {
- appctx->ctx.cli.msg = "'set maxconn' only supports 'frontend' and
'global'.\n";
+ appctx->ctx.cli.msg = "'set maxconn' only supports 'frontend',
'server', and 'global'.\n";
  appctx->st0 = STAT_CLI_PRINT;
  return 1;
  }
--
2.1.3


0001-MINOR-cli-ability-to-set-per-server-maxconn.patch
Description: Binary data