Any way to limit number of new _backend_ connections per-second?
Hi I have an existing service that uses an old internal custom proxy service that I'd like to replace with haproxy. The old system uses http1.1 style keepalive requests and generally has a large number of long term frontend connections (e.g. 1000 or so), but requests are relatively slow (say 1 per second per connection), and so are able to be sent to a relatively small number of backend connections (20 or so on average) because each request is processed in 10's milliseconds or so. Although old, the internal custom proxy has one feature that is quite nice. It can limit the global rate that it will open new _backend_ connections (not requests) to a configurable value (e.g. max 5 new backend connections per-second). When all backends are in use, any excess requests are queued in the proxy service until a backend becomes available, whether that's because an existing request on an existing connection completed, or enough time has passed to open a new backend connection. This process is quite nice for two reasons: 1. It stops a sudden and large spike of incoming requests from causing a sudden spike of new backend connections 2. It stops a an unexpected slow down in backend processing from also causing a sudden spike of new backend connections Given the backend service is a forking service, a sudden large spike of new backend connections would cause a sudden large number of forks, which can then generate a load spike, unexpected memory pressure, etc. By limiting the global rate of new backend connections, we create some back pressure which helps reduce the chance of a spike in frontend requests causing cascading problems. Looking at the haproxy docs, I'm basically looking at `rate-limit sessions`, but rather than only for frontends, I want to be able to control that for backends as well. I've looked through the documentation quite a bit, but I can't see anything obvious that allows this to be controlled. Is there some option that would allow this that I'm missing? Thanks Rob Mueller r...@fastmail.com
[PATCH] spelling fixes
Hello,
yet another spelling fix.
Ilya
From b3eeb7f08e1904825571406f5f4bbd7892ac2983 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin
Date: Wed, 7 Dec 2022 09:46:19 +0500
Subject: [PATCH] CLEANUP: assorted typo fixes in the code and comments
This is 34th iteration of typo fixes
---
doc/configuration.txt | 2 +-
doc/internals/api/event_hdl.txt | 2 +-
doc/lua-api/index.rst | 2 +-
doc/peers-v2.0.txt | 2 +-
doc/peers.txt | 2 +-
include/haproxy/event_hdl-t.h | 2 +-
src/quic_conn.c | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index c6cb49f00..c45f0b4b6 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3104,7 +3104,7 @@ tune.quic.socket-owner { listener | connection }
The "listener" value indicates that QUIC transfers will occur on the shared
listener socket. This option can be a good compromise for small traffic as it
allows to reduce FD consumption. However, performance won't be optimal due to
- a higher CPU usage if listeners are shared accross a lot of threads or a
+ a higher CPU usage if listeners are shared across a lot of threads or a
large number of QUIC connections can be used simultaneously.
tune.rcvbuf.client
diff --git a/doc/internals/api/event_hdl.txt b/doc/internals/api/event_hdl.txt
index fdcc97369..4414bc495 100644
--- a/doc/internals/api/event_hdl.txt
+++ b/doc/internals/api/event_hdl.txt
@@ -874,7 +874,7 @@ If EVENT_HDL_ASYNC_EVENT_DATA is not big enough to store
your new
event family struct, a compilation assert triggered by EVENT_HDL_CB_DATA
will occur. In addition to this, an extra runtime BUG_ON will make
sure the condition is met when publishing the event.
-The goal here is to force haproxy to fail explicitely so you know that
+The goal here is to force haproxy to fail explicitly so you know that
something must be done on your side.
3.1 PUBLISHING AN EVENT
diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index 2f2744ffa..4b6e5af55 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -364,7 +364,7 @@ Core class
Returns HAProxy core information. We can find information like the uptime,
the pid, memory pool usage, tasks number, ...
- These informations are also returned by the management socket via the command
+ This information is also returned by the management socket via the command
"show info". See the management socket documentation for more information
about the content of these variables.
diff --git a/doc/peers-v2.0.txt b/doc/peers-v2.0.txt
index 5f3aa1183..051cb6094 100644
--- a/doc/peers-v2.0.txt
+++ b/doc/peers-v2.0.txt
@@ -154,7 +154,7 @@ encoded integer value |
- for other key type
-The value length is annonced in table definition message
+The value length is announced in table definition message
0
value
diff --git a/doc/peers.txt b/doc/peers.txt
index a5da40f3c..7ce2fcb46 100644
--- a/doc/peers.txt
+++ b/doc/peers.txt
@@ -252,7 +252,7 @@
| 4 | Heartbeat message. |
+++
- About hearbeat messages: a peer sends heartbeat messages to peers it is
+ About heartbeat messages: a peer sends heartbeat messages to peers it is
connected to after periods of 3s of inactivity (i.e. when there is no
stick-table to synchronize for 3s). After a successful peer protocol
handshake between two peers, if one of them does not send any other peer
diff --git a/include/haproxy/event_hdl-t.h b/include/haproxy/event_hdl-t.h
index 3cd19624d..7ce9a1a1a 100644
--- a/include/haproxy/event_hdl-t.h
+++ b/include/haproxy/event_hdl-t.h
@@ -226,7 +226,7 @@ struct event_hdl_sub {
/* ---*/
/* user defined event types are listed here
- * please reflect any change in theses macros in the subtype map
+ * please reflect any change in these macros in the subtype map
* defined below that is used to perform string to event type and
* event type to string conversions
*/
diff --git a/src/quic_conn.c b/src/quic_conn.c
index 0fc2f5eb9..8b09d5780 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1435,7 +1435,7 @@ static int quic_packet_encrypt(unsigned char *payload,
size_t payload_len,
/* Decrypt packet using encryption level for connection.
* Decryption is done in place in packet buffer.
*
- * Returns 1 on sucess else 0.
+ * Returns 1 on success else 0.
*/
static int qc_pkt_decrypt(struct quic_conn *qc, struct quic_enc_level *qel,
struct quic_rx_packet *pkt)
--
2.35.3.windows.1
Re: Reproducible CI build with OpenSSL and "latest" keyword
вт, 6 дек. 2022 г. в 23:29, Willy Tarreau : > On Tue, Dec 06, 2022 at 06:59:30PM +0100, Tim Düsterhus wrote: > > William, > > > > On 12/6/22 15:37, William Lallemand wrote: > > > As I already mentionned, I don't really like the "latest" keyword for > > > the OpenSSL version as it prevent us to have reproducible builds. > > > It updates versions without warning, even major ones. > > > > I agree and also was not really happy with the 'latest' back when it was > > introduced in the first place, but didn't care strongly enough to speak > up > > then. > > No problem, it's always easier to see what we've missed after it reaches > its limits :-) > > > > What I suggest is to stop using "latest" for the "git push" CI, but > > > using it only in a separate CI (once a day/week I don't know). And only > > > use fixed version of the libraries on the CI so builds are not broken > by > > > external components. Because in my opinion the "git push" CI is to test > > > our code, not the libraries. > > > > > > > I don't even think such a weekly job is necessary [1]. Add an item to the > > release checklist "check if any new SSL versions are available and add > them > > to matrix.py" and this should be fine, all SSL versions will then be > updated > > every 6 months and can also be updated on demand for important releases. > > It's similar to how I simply rerun the Coccinelle patches from time to > time > > to fix whatever crept in since the last release. > > I'm seeing a slightly different approach here. We need to keep in mind > that: > - dev must be as forward-thinking as possible; as such, being aware > that something is going to break soon is useful, particularly when > it comes to forthcoming changes in third party dependencies. I.e. > openssl 3 breakage was reported upfront and that was fortunate > because a lot of changes were required to adapt to it, long before > it was even released. So I don't know if the weekly job is the best > option or not, but right now, watching form time to time how it goes > with openssl 3.1-dev is useful. Here I'm speaking about a branch, it > thus means that we're following what significant changes may bother > us. It's purely integration testing, and we could decide that one > major upgrade will not be the target for the current version because > it requires too much work. But if we focus on such a target we should > be as up-to-date as possible (no need to alarm on past breakage if > the issue is since fixed). That's where following a -latest from a > lib can be useful. > > - dev must also tell us if we're going to cause breakage on what we > expect to support. E.g. once we see that 3.1-dev looks good and we > decide to enable it, we'll also need to be able to quickly follow it, > because until it's released, it may revert some changes or change its > API and cause breakage. It's not like a stable version we can trust, > and we need to know as well if we finally prefer to give up because > going back-and-forth indicates a third-party dependency is not stable > enough for example, or we could mention before the release that it's > only experimental for now. > > - even when adopting a stable release, we'll suffer from bugs in third > party dependencies just like we suffer from our own bugs. If OpenSSL > 3.1.0 is released and we decide to follow its stable branch, maybe > some tests will randomly fail from time to time. And we probably > don't want to manually change its version every week to follow the > first post-release fixes that we may need to stabilize the CI. In > this case again, following the latest from a given branch would be > useful, but it becomes less critical. Indeed, generally the most > annoying issues should stabilize during the very first versions, and > we don't need to care as much about updating the lib once the CI works > fine again. > > - however, once we release a version, the most important is to make sure > there are as few moving parts as possible. We should not change any > dependency automatically for the CI. Triggering the build on the same > release two weeks apart should execute the same tests, otherwise we > can think that we broke something during a backport while it's not > the case (that's in fact how we first noticed the problem). > > Thus I think that: > - for dev, adopting the latest version of a chosen branch (released or > dev) if possible would be desirable so that we're informed ASAP that > the branch we're expecting to support is experiencing/causing some > trouble ; > > - for dev, watching weekly or so the level of support of future versions > that we have not yet planed to support would help prepare internals > where needed ; > > - for stable, we'd just stick to the version that was in effect that the > moment of the release (i.e. the version the tests were
Re: [PATCH] MINOR : converter: add param converter
Any update on this?
[PATCH] CI: Add `schedule` to vtest.yml
William,
On 12/6/22 19:40, William Lallemand wrote:
> I disagree, porting to a new API is not something you would do just
> before a release, you need to do it progressively if possible, because
> it could introduce heavy development and sometimes discussions with the
> library developers and unfortunately that could take time.
I understand now. I thought this was primarily about bumping new patch
versions, not about testing new feature branches.
Please find a patch attached that will start to run the vtest.yml workflow
weekly early on Thursday morning, making the results available at the start
of the Thursday workday.
If that change is working correctly, we should see that the "Generate Build
Matrix" step outputs "Generating matrix for type 'schedule'." for those
scheduled jobs. You can then add whatever extra jobs you desire to matrix.py
by checking for `build_type == "schedule"`, like this:
ssl_versions = [
"stock",
"OPENSSL_VERSION=1.0.2u",
"OPENSSL_VERSION=3.0.0",
]
if build_type == "schedule":
ssl_versions.append("OPENSSL_VERSION=latest")
Best regards
Tim Düsterhus
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
This will run the vtest.yml workflow weekly on every Thursday morning in
addition to running on every push.
As the `github.event_name` is passed as a parameter to `matrix.py`, this allows
to run specific jobs (e.g. heavy jobs or unstable ones) only on schedule if the
need arises.
---
.github/workflows/vtest.yml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
index fb7b1d968a..65892cf6b8 100644
--- a/.github/workflows/vtest.yml
+++ b/.github/workflows/vtest.yml
@@ -10,6 +10,8 @@ name: VTest
on:
push:
+ schedule:
+ - cron: "37 5 * * 4" # 05:37 UTC on Thursday
permissions:
contents: read
--
2.38.1
Re: Reproducible CI build with OpenSSL and "latest" keyword
Tim, On Tue, Dec 06, 2022 at 06:59:30PM +0100, Tim Düsterhus wrote: > > What I suggest is to stop using "latest" for the "git push" CI, but > > using it only in a separate CI (once a day/week I don't know). And only > > use fixed version of the libraries on the CI so builds are not broken by > > external components. Because in my opinion the "git push" CI is to test > > our code, not the libraries. > > > > I don't even think such a weekly job is necessary [1]. > Add an item to the release checklist "check if any new SSL versions > are available and add them to matrix.py" and this should be fine, all > SSL versions will then be updated every 6 months and can also be > updated on demand for important releases. Well, I don't want to see the CI fail just for testing this, having the weekly one gives you the status before integration and is also a reminder. > It's similar to how I simply rerun the Coccinelle > patches from time to time to fix whatever crept in since the last release. > I disagree, porting to a new API is not something you would do just before a release, you need to do it progressively if possible, because it could introduce heavy development and sometimes discussions with the library developers and unfortunately that could take time. That would be too bad to postpone support for a new version because nobody looked at this during the development cycle, and the changes are too heavy to be integrated. -- William Lallemand
Re: Reproducible CI build with OpenSSL and "latest" keyword
On Tue, Dec 06, 2022 at 06:59:30PM +0100, Tim Düsterhus wrote: > William, > > On 12/6/22 15:37, William Lallemand wrote: > > As I already mentionned, I don't really like the "latest" keyword for > > the OpenSSL version as it prevent us to have reproducible builds. > > It updates versions without warning, even major ones. > > I agree and also was not really happy with the 'latest' back when it was > introduced in the first place, but didn't care strongly enough to speak up > then. No problem, it's always easier to see what we've missed after it reaches its limits :-) > > What I suggest is to stop using "latest" for the "git push" CI, but > > using it only in a separate CI (once a day/week I don't know). And only > > use fixed version of the libraries on the CI so builds are not broken by > > external components. Because in my opinion the "git push" CI is to test > > our code, not the libraries. > > > > I don't even think such a weekly job is necessary [1]. Add an item to the > release checklist "check if any new SSL versions are available and add them > to matrix.py" and this should be fine, all SSL versions will then be updated > every 6 months and can also be updated on demand for important releases. > It's similar to how I simply rerun the Coccinelle patches from time to time > to fix whatever crept in since the last release. I'm seeing a slightly different approach here. We need to keep in mind that: - dev must be as forward-thinking as possible; as such, being aware that something is going to break soon is useful, particularly when it comes to forthcoming changes in third party dependencies. I.e. openssl 3 breakage was reported upfront and that was fortunate because a lot of changes were required to adapt to it, long before it was even released. So I don't know if the weekly job is the best option or not, but right now, watching form time to time how it goes with openssl 3.1-dev is useful. Here I'm speaking about a branch, it thus means that we're following what significant changes may bother us. It's purely integration testing, and we could decide that one major upgrade will not be the target for the current version because it requires too much work. But if we focus on such a target we should be as up-to-date as possible (no need to alarm on past breakage if the issue is since fixed). That's where following a -latest from a lib can be useful. - dev must also tell us if we're going to cause breakage on what we expect to support. E.g. once we see that 3.1-dev looks good and we decide to enable it, we'll also need to be able to quickly follow it, because until it's released, it may revert some changes or change its API and cause breakage. It's not like a stable version we can trust, and we need to know as well if we finally prefer to give up because going back-and-forth indicates a third-party dependency is not stable enough for example, or we could mention before the release that it's only experimental for now. - even when adopting a stable release, we'll suffer from bugs in third party dependencies just like we suffer from our own bugs. If OpenSSL 3.1.0 is released and we decide to follow its stable branch, maybe some tests will randomly fail from time to time. And we probably don't want to manually change its version every week to follow the first post-release fixes that we may need to stabilize the CI. In this case again, following the latest from a given branch would be useful, but it becomes less critical. Indeed, generally the most annoying issues should stabilize during the very first versions, and we don't need to care as much about updating the lib once the CI works fine again. - however, once we release a version, the most important is to make sure there are as few moving parts as possible. We should not change any dependency automatically for the CI. Triggering the build on the same release two weeks apart should execute the same tests, otherwise we can think that we broke something during a backport while it's not the case (that's in fact how we first noticed the problem). Thus I think that: - for dev, adopting the latest version of a chosen branch (released or dev) if possible would be desirable so that we're informed ASAP that the branch we're expecting to support is experiencing/causing some trouble ; - for dev, watching weekly or so the level of support of future versions that we have not yet planed to support would help prepare internals where needed ; - for stable, we'd just stick to the version that was in effect that the moment of the release (i.e. the version the tests were run on during dev). And that would make sense considering that a release is nothing more than freezing changes between dev and stable. I'm particularly conscious that there may be technical limitations for the -
Re: Reproducible CI build with OpenSSL and "latest" keyword
William, On 12/6/22 15:37, William Lallemand wrote: As I already mentionned, I don't really like the "latest" keyword for the OpenSSL version as it prevent us to have reproducible builds. It updates versions without warning, even major ones. I agree and also was not really happy with the 'latest' back when it was introduced in the first place, but didn't care strongly enough to speak up then. What I suggest is to stop using "latest" for the "git push" CI, but using it only in a separate CI (once a day/week I don't know). And only use fixed version of the libraries on the CI so builds are not broken by external components. Because in my opinion the "git push" CI is to test our code, not the libraries. I don't even think such a weekly job is necessary [1]. Add an item to the release checklist "check if any new SSL versions are available and add them to matrix.py" and this should be fine, all SSL versions will then be updated every 6 months and can also be updated on demand for important releases. It's similar to how I simply rerun the Coccinelle patches from time to time to fix whatever crept in since the last release. Best regards Tim Düsterhus
Re: Reproducible CI build with OpenSSL and "latest" keyword
вт, 6 дек. 2022 г. в 21:22, William Lallemand : > On Tue, Dec 06, 2022 at 07:54:33PM +0500, Илья Шипицин wrote: > > I recall I even promised to do something, but I did not :-) > > > > automatically determine "which is latest 3.0.x" does not make much sense, > > it is stable branch, very conservative. we can stick to 3.0.7, for > example. > > I do not expect any breaking change between 3.0.7 and 3.0.8 > > I recall a discussion like this indeed, couldn't find the previous > thread. There was cases of broken minor release for LibreSSL for > example, so it's better to stick to a release for the push CI and > upgrade once the weekly CI passed. > > > we can move "latest" to weekly, np. as for stable branches CI, I think > them > > do not have to follow the same rules as development branch, we can have > > different matrix for stable and development. > > I think we could it this way: > > - weekly "latest" for libreSSL, OpenSSL, whateverSSL for HAProxy master > branch > - git push CI with fixed version in HAProxy master > > > > I think I got the idea. > > looks like you use the same github actions for stable branches. > > > > Indeed, at some point the master branch become a stable branch, so we > inherit all of this. > > > either I will manage to make them different or I will stick to > > 3.0.something. hopefully tomorrow > > IMHO it should never be "latest" anywhere else than the weekly, we don't > want to be disturbed by this when pushing our code. > > I don't think we need a weekly for the stable branch, so it could be > conditionned by a check on the version, for example only started if > there is '-dev' in the version. > > So we should probably: > > - Revert "latest" to "3.0.7" in the master, and backport the patch in the > previous supported branches. (as far as 2.4 I think) > I want to check how github actions are good on branch filtering. if they are, it adds flexibility. if they are not, ok, we will maintain the same actions for all branches. > > - Introduce "3.1.0-alpha1" to master > > - Introduce "latest" to weekly master > > -- > William Lallemand > >
Re: Reproducible CI build with OpenSSL and "latest" keyword
On Tue, Dec 06, 2022 at 07:54:33PM +0500, Илья Шипицин wrote: > I recall I even promised to do something, but I did not :-) > > automatically determine "which is latest 3.0.x" does not make much sense, > it is stable branch, very conservative. we can stick to 3.0.7, for example. > I do not expect any breaking change between 3.0.7 and 3.0.8 I recall a discussion like this indeed, couldn't find the previous thread. There was cases of broken minor release for LibreSSL for example, so it's better to stick to a release for the push CI and upgrade once the weekly CI passed. > we can move "latest" to weekly, np. as for stable branches CI, I think them > do not have to follow the same rules as development branch, we can have > different matrix for stable and development. I think we could it this way: - weekly "latest" for libreSSL, OpenSSL, whateverSSL for HAProxy master branch - git push CI with fixed version in HAProxy master > I think I got the idea. > looks like you use the same github actions for stable branches. > Indeed, at some point the master branch become a stable branch, so we inherit all of this. > either I will manage to make them different or I will stick to > 3.0.something. hopefully tomorrow IMHO it should never be "latest" anywhere else than the weekly, we don't want to be disturbed by this when pushing our code. I don't think we need a weekly for the stable branch, so it could be conditionned by a check on the version, for example only started if there is '-dev' in the version. So we should probably: - Revert "latest" to "3.0.7" in the master, and backport the patch in the previous supported branches. (as far as 2.4 I think) - Introduce "3.1.0-alpha1" to master - Introduce "latest" to weekly master -- William Lallemand
Re: Reproducible CI build with OpenSSL and "latest" keyword
I think I got the idea. looks like you use the same github actions for stable branches. either I will manage to make them different or I will stick to 3.0.something. hopefully tomorrow вт, 6 дек. 2022 г. в 19:54, Илья Шипицин : > I recall I even promised to do something, but I did not :-) > > automatically determine "which is latest 3.0.x" does not make much sense, > it is stable branch, very conservative. we can stick to 3.0.7, for example. > I do not expect any breaking change between 3.0.7 and 3.0.8 > > we can move "latest" to weekly, np. as for stable branches CI, I think > them do not have to follow the same rules as development branch, we can > have different matrix for stable and development. > > вт, 6 дек. 2022 г. в 19:37, William Lallemand : > >> Hello, >> >> We are experiencing difficulties with the way the CI matrix is >> generated with the SSL libraries. >> >> As I already mentionned, I don't really like the "latest" keyword for >> the OpenSSL version as it prevent us to have reproducible builds. >> It updates versions without warning, even major ones. >> >> Since OpenSSL 3.1.0-aplha1 was released we are affected by the problem, >> we stopped building with 3.0.x without noticing, and our internal CI for >> the stable branches start failing because of that. Majour versions must >> never change in the previous branches. >> >> What I suggest is to stop using "latest" for the "git push" CI, but >> using it only in a separate CI (once a day/week I don't know). And only >> use fixed version of the libraries on the CI so builds are not broken by >> external components. Because in my opinion the "git push" CI is to test >> our code, not the libraries. >> >> What do you guys think? >> >> -- >> William Lallemand >> >
Re: Reproducible CI build with OpenSSL and "latest" keyword
I recall I even promised to do something, but I did not :-) automatically determine "which is latest 3.0.x" does not make much sense, it is stable branch, very conservative. we can stick to 3.0.7, for example. I do not expect any breaking change between 3.0.7 and 3.0.8 we can move "latest" to weekly, np. as for stable branches CI, I think them do not have to follow the same rules as development branch, we can have different matrix for stable and development. вт, 6 дек. 2022 г. в 19:37, William Lallemand : > Hello, > > We are experiencing difficulties with the way the CI matrix is > generated with the SSL libraries. > > As I already mentionned, I don't really like the "latest" keyword for > the OpenSSL version as it prevent us to have reproducible builds. > It updates versions without warning, even major ones. > > Since OpenSSL 3.1.0-aplha1 was released we are affected by the problem, > we stopped building with 3.0.x without noticing, and our internal CI for > the stable branches start failing because of that. Majour versions must > never change in the previous branches. > > What I suggest is to stop using "latest" for the "git push" CI, but > using it only in a separate CI (once a day/week I don't know). And only > use fixed version of the libraries on the CI so builds are not broken by > external components. Because in my opinion the "git push" CI is to test > our code, not the libraries. > > What do you guys think? > > -- > William Lallemand >
Reproducible CI build with OpenSSL and "latest" keyword
Hello, We are experiencing difficulties with the way the CI matrix is generated with the SSL libraries. As I already mentionned, I don't really like the "latest" keyword for the OpenSSL version as it prevent us to have reproducible builds. It updates versions without warning, even major ones. Since OpenSSL 3.1.0-aplha1 was released we are affected by the problem, we stopped building with 3.0.x without noticing, and our internal CI for the stable branches start failing because of that. Majour versions must never change in the previous branches. What I suggest is to stop using "latest" for the "git push" CI, but using it only in a separate CI (once a day/week I don't know). And only use fixed version of the libraries on the CI so builds are not broken by external components. Because in my opinion the "git push" CI is to test our code, not the libraries. What do you guys think? -- William Lallemand
RE: RE: [PATCH 0/2] BUG/MINOR: promex: fix haproxy_backend_agg_server_check_status
>Sorry, indeed I mentioned the enum ID and not the promex name. >I proposed to keep "haproxy_bakcned_agg_server_check_status" > altering its description to announce it is deprecated >. And then to add "haproxy_backend_agg_check_status" >aggregate haproxy_server_check_status) and >"haproxy_backend_agg_server_status" > or "haproxy_backend_agg_srv_status" (to aggregate haproxy_server_status). To resume, is it ok for you ? - Deprecated haproxy_backend_agg_server_check_status. ( Modify the metric description ) - create new haproxy_backend_agg_server_status metric, aggregation of haproxy_backend_server_status. ( to replace misnamed haproxy_backend_agg_server_check_status ) - create new haproxy_backend_agg_check_status metric, aggregation of haproxy_backend_server_check_status. this will be merged/backported in 2.5/2.6/2.7/2.8-dev and haproxy_backend_agg_server_check_status will be removed in 2.9 ( or 2.10 )
Re: [PATCH 0/2] BUG/MINOR: promex: fix haproxy_backend_agg_server_check_status
On Tue, Dec 6, 2022 at 3:27 AM Christopher Faulet wrote: > IMHO, we should keep the current metric and its description should be updated > to > mention it is deprecated. This way, we will be able to remove it in the 2.9 or > 3.0. This means we should find new names for the aggregated servers status and > the aggregated check status. "ST_F_AGG_SRV_STATUS" is pretty good for the > first > one. I may propose "ST_F_AGG_CHECK_STATUS" for the second one. > > I guess the two new metrics may be backported. They are not used by the stats > applet. > > What do you think about it? I am aligned with this plan of creating two new metrics Thanks, -- William
Re: RE: [PATCH 0/2] BUG/MINOR: promex: fix haproxy_backend_agg_server_check_status
Le 12/6/22 à 12:26, Cedric Paillet a écrit : Sorry for the delay. We were busy because of the 2.7.0 and then I forgot your patches :) And it was a bit late to add it into the 2.7.0. No problem. Because the metric name is indeed badly chosen but we can't break the compatibility, especially in a LTS version. Ok, I understand that. We need to find new names for the metric IMHO, we should keep the current metric and its description should be updated to mention it is deprecated. This way, we will be able to remove it in the 2.9 or 3.0. This means we should find new names for the aggregated servers status and the aggregated check status. "ST_F_AGG_SRV_STATUS" is pretty good for the first one. I may propose "ST_F_AGG_CHECK_STATUS" for the second one. Not very clear, the value to change to keep the compatibility is the metric name itself ? do you propose to use?: - haproxy_backend_agg_check_status ( to aggregate haproxy_server_check_status ) - haproxy_backend_agg_status ( to aggregate haproxy_server_status ) Or - haproxy_backend_agg_srv_check_status ( to aggregate haproxy_server_check_status - haproxy_backend_agg_srv_status ( to aggregate haproxy_server_status ) For reminder we currently have - haproxy_backend_agg_server_check_status ( to aggregate haproxy_server_status ) ( and we have also have haproxy_backend_status, for the status of the backend itself ) Cedric Paillet Sorry, indeed I mentioned the enum ID and not the promex name. I proposed to keep "haproxy_bakcned_agg_server_check_status" altering its description to announce it is deprecated. And then to add "haproxy_backend_agg_check_status" (aggregate haproxy_server_check_status) and "haproxy_backend_agg_server_status" or "haproxy_backend_agg_srv_status" (to aggregate haproxy_server_status). -- Christopher Faulet
RE: [PATCH 0/2] BUG/MINOR: promex: fix haproxy_backend_agg_server_check_status
> Sorry for the delay. We were busy because of the 2.7.0 and then I forgot your > patches :) And it was a bit late to add it into the 2.7.0. No problem. > Because the metric name is indeed badly chosen but we can't break the > compatibility, especially in a LTS version. Ok, I understand that. We need to find new names for the metric >IMHO, we should keep the current metric and its description should be updated >to mention it is deprecated. This way, we will be able to remove it in the 2.9 >or 3.0. This means we should find new names for the aggregated servers status >and the aggregated check status. "ST_F_AGG_SRV_STATUS" is pretty good for the >first one. I may propose "ST_F_AGG_CHECK_STATUS" for the second one. Not very clear, the value to change to keep the compatibility is the metric name itself ? do you propose to use?: - haproxy_backend_agg_check_status ( to aggregate haproxy_server_check_status ) - haproxy_backend_agg_status ( to aggregate haproxy_server_status ) Or - haproxy_backend_agg_srv_check_status ( to aggregate haproxy_server_check_status - haproxy_backend_agg_srv_status ( to aggregate haproxy_server_status ) For reminder we currently have - haproxy_backend_agg_server_check_status ( to aggregate haproxy_server_status ) ( and we have also have haproxy_backend_status, for the status of the backend itself ) Cedric Paillet
Re: [PATCH 0/2] BUG/MINOR: promex: fix haproxy_backend_agg_server_check_status
Le 11/28/22 à 08:44, Cedric Paillet a écrit : As described in github issue #1312, the first intention of patch 42d7c402d was to aggregate haproxy_server_check_status. But we aggregated haproxy_server_status instead. To fix that, rename haproxy_backend_agg_server_check_status to haproxy_backend_agg_server_status and use right data source for haproxy_backend_agg_server_check_status. Cedric Paillet (2): BUG/MINOR: promex: rename haproxy_backend_agg_server_check_status MINOR: promex: reintroduce haproxy_backend_agg_server_check_status addons/promex/service-prometheus.c | 28 +++- include/haproxy/stats-t.h | 1 + src/stats.c| 4 3 files changed, 32 insertions(+), 1 deletion(-) Hi Cedric, Sorry for the delay. We were busy because of the 2.7.0 and then I forgot your patches :) And it was a bit late to add it into the 2.7.0. So, I'm a bit bother. Because the metric name is indeed badly chosen but we can't break the compatibility, especially in a LTS version. The prometheus exporter is an addon. Thus the backward compatibility is less strict than for core code of HAProxy. However, it is probably the most used addon. IMHO, we should keep the current metric and its description should be updated to mention it is deprecated. This way, we will be able to remove it in the 2.9 or 3.0. This means we should find new names for the aggregated servers status and the aggregated check status. "ST_F_AGG_SRV_STATUS" is pretty good for the first one. I may propose "ST_F_AGG_CHECK_STATUS" for the second one. I guess the two new metrics may be backported. They are not used by the stats applet. What do you think about it? -- Christopher Faulet
