On 2018/8/22 04:04, Willy Tarreau wrote:
> Hi Patrick,
>
> On Thu, Aug 09, 2018 at 06:46:28PM -0400, Patrick Hemmer wrote:
>> This adds the sample fetch 'be_conn_free([<backend>])'. This sample fetch
>> provides the total number of unused connections across available servers
>> in the specified backend.
> Thanks for writing this one, I recently figured I needed the same for my
> build farm :-)
>
>> +be_conn_free([<backend>]) : integer
>> + Returns an integer value corresponding to the number of available
>> connections
>> + across available servers in the backend. Queue slots are not included.
>> Backup
>> + servers are also not included, unless all other servers are down. If no
>> + backend name is specified, the current one is used. But it is also
>> possible
>> + to check another backend. It can be used to use a specific farm when the
>> + nominal one is full. See also the "be_conn" and "connslots" criteria.
>> +
>> + OTHER CAVEATS AND NOTES: at this point in time, the code does not take
>> care
>> + of dynamic connections. Also, if any of the server maxconn, or maxqueue
>> is 0,
>> + then this fetch clearly does not make sense, in which case the value
>> returned
>> + will be -1.
> I disagree with making a special case above for maxconn 0. In fact for me
> it just means that such a server cannot accept connections, so it simply
> doesn't count in the sum, just as if it were not present.
On a server, maxconn=0 means unlimited, not that it can't accept
connections. If we return 0, how will the caller differentiate between
unlimited, and no free connections?
This is the same behavior provided by the `connslots` fetch, and is also
where I stole the doc snippet from.
>
>> + px = iterator->proxy;
>> + if (!srv_currently_usable(iterator) || ((iterator->flags &
>> SRV_F_BACKUP) &&
>> + (px->srv_act ||
>> + (iterator !=
>> px->lbprm.fbck && !(px->options & PR_O_USE_ALL_BK)))))
>> + continue;
> Please slightly reorder the condition to improve the indent above for
> better readability, for example :
>
> if (!srv_currently_usable(iterator) ||
> ((iterator->flags & SRV_F_BACKUP) &&
> (px->srv_act || (iterator != px->lbprm.fbck &&
> !(px->options & PR_O_USE_ALL_BK)))))
>
> After checking, I'm OK with the condition :-)
>
>> + if (iterator->maxconn == 0) {
>> + /* configuration is stupid */
>> + smp->data.u.sint = -1; /* FIXME: stupid value! */
>> + return 1;
>> + }
>> +
>> + smp->data.u.sint += (iterator->maxconn - iterator->cur_sess);
> Here I'd simply suggest this to replace the whole block :
>
> if (iterator->maxconn > iterator->cur_sess)
> smp->data.u.sint += (iterator->maxconn -
> iterator->cur_sess);
>
> And then it can properly count available connections through all
> available servers, regardless of their individual configuration.
>
> Otherwise I'm fine with this patch.
>
> Thanks,
> Willy
Also made 2 additional changes. One is to handle dynamic maxconn. The
other is to handle the case where the maxconn is adjusted (via stats
socket) to be less than the number of currently active connections,
which would result in the value wrapping.
Will update the srv_conn_free fetch with similar changes pending outcome
of this one.
-Patrick
From 151d032b9cb396df47435742c03817818878f5af Mon Sep 17 00:00:00 2001
From: Patrick Hemmer <hapr...@stormcloud9.net>
Date: Thu, 14 Jun 2018 17:10:27 -0400
Subject: [PATCH 1/2] MINOR: add be_conn_free sample fetch
This adds the sample fetch 'be_conn_free([<backend>])'. This sample fetch
provides the total number of unused connections across available servers in the
specified backend.
---
doc/configuration.txt | 15 ++++++++++++++-
src/backend.c | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 6e33f5994..f65efce95 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -13682,7 +13682,20 @@ be_conn([<backend>]) : integer
possibly including the connection being evaluated. If no backend name is
specified, the current one is used. But it is also possible to check another
backend. It can be used to use a specific farm when the nominal one is full.
- See also the "fe_conn", "queue" and "be_sess_rate" criteria.
+ See also the "fe_conn", "queue", "be_conn_free", and "be_sess_rate" criteria.
+
+be_conn_free([<backend>]) : integer
+ Returns an integer value corresponding to the number of available connections
+ across available servers in the backend. Queue slots are not included. Backup
+ servers are also not included, unless all other servers are down. If no
+ backend name is specified, the current one is used. But it is also possible
+ to check another backend. It can be used to use a specific farm when the
+ nominal one is full. See also the "be_conn" and "connslots" criteria.
+
+ OTHER CAVEATS AND NOTES: at this point in time, the code does not take care
+ of dynamic connections. Also, if any of the server maxconn, or maxqueue is 0,
+ then this fetch clearly does not make sense, in which case the value returned
+ will be -1.
be_sess_rate([<backend>]) : integer
Returns an integer value corresponding to the sessions creation rate on the
diff --git a/src/backend.c b/src/backend.c
index 1aadca590..4309ca825 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1792,6 +1792,46 @@ smp_fetch_be_conn(const struct arg *args, struct sample
*smp, const char *kw, vo
return 1;
}
+/* set temp integer to the number of available connections across available
+ * servers on the backend.
+ * Accepts exactly 1 argument. Argument is a backend, other types will lead to
+ * undefined behaviour.
+ */
+static int
+smp_fetch_be_conn_free(const struct arg *args, struct sample *smp, const char
*kw, void *private)
+{
+ struct server *iterator;
+ struct proxy *px;
+ unsigned int maxconn;
+
+ smp->flags = SMP_F_VOL_TEST;
+ smp->data.type = SMP_T_SINT;
+ smp->data.u.sint = 0;
+
+ for (iterator = args->data.prx->srv; iterator; iterator =
iterator->next) {
+ if (iterator->cur_state == SRV_ST_STOPPED)
+ continue;
+
+ px = iterator->proxy;
+ if (!srv_currently_usable(iterator) ||
+ ((iterator->flags & SRV_F_BACKUP) &&
+ (px->srv_act || (iterator != px->lbprm.fbck &&
!(px->options & PR_O_USE_ALL_BK)))))
+ continue;
+
+ if (iterator->maxconn == 0) {
+ /* configuration is stupid */
+ smp->data.u.sint = -1; /* FIXME: stupid value! */
+ return 1;
+ }
+
+ maxconn = srv_dynamic_maxconn(iterator);
+ if (maxconn > iterator->cur_sess)
+ smp->data.u.sint += maxconn - iterator->cur_sess;
+ }
+
+ return 1;
+}
+
/* set temp integer to the total number of queued connections on the backend.
* Accepts exactly 1 argument. Argument is a backend, other types will lead to
* undefined behaviour.
@@ -1897,6 +1937,7 @@ static int sample_conv_nbsrv(const struct arg *args,
struct sample *smp, void *p
static struct sample_fetch_kw_list smp_kws = {ILH, {
{ "avg_queue", smp_fetch_avg_queue_size, ARG1(1,BE), NULL,
SMP_T_SINT, SMP_USE_INTRN, },
{ "be_conn", smp_fetch_be_conn, ARG1(1,BE), NULL,
SMP_T_SINT, SMP_USE_INTRN, },
+ { "be_conn_free", smp_fetch_be_conn_free, ARG1(1,BE), NULL,
SMP_T_SINT, SMP_USE_INTRN, },
{ "be_id", smp_fetch_be_id, 0, NULL,
SMP_T_SINT, SMP_USE_BKEND, },
{ "be_name", smp_fetch_be_name, 0, NULL,
SMP_T_STR, SMP_USE_BKEND, },
{ "be_sess_rate", smp_fetch_be_sess_rate, ARG1(1,BE), NULL,
SMP_T_SINT, SMP_USE_INTRN, },
--
2.18.0
