Guys,
These two patches address it for me, and I could verify that they apply
on top of 2.2.11 and work there as well. This time I tested with two
counters at different periods 500 and 2000ms.
Thanks,
Willy
>From 6064b34be0e761de881a5bfca287d01f69122602 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Tue, 23 Mar 2021 08:45:42 +0100
Subject: MINOR: time: also provide a global, monotonic global_now_ms timer
The period-based freq counters need the global date in milliseconds,
so better calculate it and expose it rather than letting all call
places incorrectly retrieve it.
Here what we do is that we maintain a new globally monotonic timer,
global_now_ms, which ought to be very close to the global_now one,
but maintains the monotonic approach of now_ms between all threads
in that global_now_ms is always ahead of any now_ms.
This patch is made simple to ease backporting (it will be needed for
a subsequent fix), but it also opens the way to some simplifications
on the time handling: instead of computing the local time and trying
to force it to the global one, we should soon be able to proceed in
the opposite way, that is computing the new global time an making the
local one just the latest snapshot of it. This will bring the benefit
of making sure that the global time is always ahead of the local one.
---
include/haproxy/time.h | 1 +
src/time.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/include/haproxy/time.h b/include/haproxy/time.h
index 87bc3655a..93dd84080 100644
--- a/include/haproxy/time.h
+++ b/include/haproxy/time.h
@@ -61,6 +61,7 @@ extern struct timeval start_date; /* the process's
start date */
extern THREAD_LOCAL struct timeval before_poll; /* system date before
calling poll() */
extern THREAD_LOCAL struct timeval after_poll; /* system date after
leaving poll() */
extern volatile unsigned long long global_now;
+extern volatile unsigned int global_now_ms;
/**** exported functions *************************************************/
diff --git a/src/time.c b/src/time.c
index d5344d26f..0cfc9bf3c 100644
--- a/src/time.c
+++ b/src/time.c
@@ -15,6 +15,7 @@
#include <haproxy/api.h>
#include <haproxy/time.h>
+#include <haproxy/ticks.h>
#include <haproxy/tools.h>
THREAD_LOCAL unsigned int ms_left_scaled; /* milliseconds left for current
second (0..2^32-1) */
@@ -29,6 +30,7 @@ THREAD_LOCAL struct timeval after_poll; /* system date
after leaving poll()
static THREAD_LOCAL struct timeval tv_offset; /* per-thread time ofsset
relative to global time */
volatile unsigned long long global_now; /* common date between all
threads (32:32) */
+volatile unsigned int global_now_ms; /* common date in milliseconds
(may wrap) */
static THREAD_LOCAL unsigned int iso_time_sec; /* last iso time value for
this thread */
static THREAD_LOCAL char iso_time_str[34]; /* ISO time representation
of gettimeofday() */
@@ -177,6 +179,7 @@ void tv_update_date(int max_wait, int interrupted)
{
struct timeval adjusted, deadline, tmp_now, tmp_adj;
unsigned int curr_sec_ms; /* millisecond of current second
(0..999) */
+ unsigned int old_now_ms, new_now_ms;
unsigned long long old_now;
unsigned long long new_now;
@@ -260,6 +263,15 @@ void tv_update_date(int max_wait, int interrupted)
*/
ms_left_scaled = (999U - curr_sec_ms) * 4294967U;
now_ms = now.tv_sec * 1000 + curr_sec_ms;
+
+ /* update the global current millisecond */
+ old_now_ms = global_now_ms;
+ do {
+ new_now_ms = old_now_ms;
+ if (tick_is_lt(new_now_ms, now_ms))
+ new_now_ms = now_ms;
+ } while (!_HA_ATOMIC_CAS(&global_now_ms, &old_now_ms, new_now_ms));
+
return;
}
--
2.28.0
>From 8cc586c73fefd96f4be1f7820e38a1263f6252ca Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Tue, 23 Mar 2021 08:58:22 +0100
Subject: BUG/MEDIUM: freq_ctr/threads: use the global_now_ms variable
In commit a1ecbca0a ("BUG/MINOR: freq_ctr/threads: make use of the last
updated global time"), for period-based counters, the millisecond part
of the global_now variable was used as the date for the new period. But
it's wrong, it only works with sub-second periods as it wraps every
second, and for other periods the counters never rotate anymore.
Let's make use of the newly introduced global_now_ms variable instead,
which contains the global monotonic time expressed in milliseconds.
This patch needs to be backported wherever the patch above is backported.
It depends on previous commit "MINOR: time: also provide a global,
monotonic global_now_ms timer".
---
include/haproxy/freq_ctr.h | 2 +-
src/freq_ctr.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/haproxy/freq_ctr.h b/include/haproxy/freq_ctr.h
index d5cea414e..b1db43ef6 100644
--- a/include/haproxy/freq_ctr.h
+++ b/include/haproxy/freq_ctr.h
@@ -88,7 +88,7 @@ static inline unsigned int update_freq_ctr_period(struct
freq_ctr_period *ctr,
curr_tick = ctr->curr_tick;
do {
- now_ms_tmp = (uint32_t)global_now / 1000;
+ now_ms_tmp = global_now_ms;
if (now_ms_tmp - curr_tick < period)
return _HA_ATOMIC_ADD(&ctr->curr_ctr, inc);
diff --git a/src/freq_ctr.c b/src/freq_ctr.c
index 8fd0c90f0..42fbc1cea 100644
--- a/src/freq_ctr.c
+++ b/src/freq_ctr.c
@@ -206,7 +206,7 @@ unsigned int read_freq_ctr_period(struct freq_ctr_period
*ctr, unsigned int peri
break;
};
- remain = curr_tick + period - (uint32_t)global_now / 1000;
+ remain = curr_tick + period - global_now_ms;
if (unlikely((int)remain < 0)) {
/* We're past the first period, check if we can still report a
* part of last period or if we're too far away.
@@ -253,7 +253,7 @@ unsigned int freq_ctr_remain_period(struct freq_ctr_period
*ctr, unsigned int pe
break;
};
- remain = curr_tick + period - (uint32_t)global_now / 1000;
+ remain = curr_tick + period - global_now_ms;
if (likely((int)remain < 0)) {
/* We're past the first period, check if we can still report a
* part of last period or if we're too far away.
--
2.28.0
