MEDIUM: counters: mostly revert da813ae4d7cb77137ed

Contrarily to what was previously believed, there are corner cases where
the counters may not be allocated, and we may want to make them optional
at a later date, so we have to check if those counters are there.
However, just checking that shared.tg is non-NULL is enough, we can then
assume that shared.tg[tgid - 1] has properly been allocated too.
Also modify the various COUNTER_SHARED_* macros to make sure they check
for that too.

diff --git a/include/haproxy/backend.h b/include/haproxy/backend.h
index 47ecfff3373543e666b0cd9fc952049f13e4f129..bdb457e33560c16b1b04044d3219db2bf4143bc0 100644 (file)
--- a/include/haproxy/backend.h
+++ b/include/haproxy/backend.h
@@ -88,7 +88,8 @@ static inline int be_usable_srv(struct proxy *be)
 /* set the time of last session on the backend */
 static inline void be_set_sess_last(struct proxy *be)
 {
-       HA_ATOMIC_STORE(&be->be_counters.shared.tg[tgid - 1]->last_sess, ns_to_sec(now_ns));
+       if (be->be_counters.shared.tg)
+               HA_ATOMIC_STORE(&be->be_counters.shared.tg[tgid - 1]->last_sess, ns_to_sec(now_ns));
 }
 
 /* This function returns non-zero if the designated server will be

diff --git a/include/haproxy/counters.h b/include/haproxy/counters.h
index 89855ce1ce371b6b1724e89b016e7d26cd094d71..a6a68623d80850811da6ccd5f7149c267388b47a 100644 (file)
--- a/include/haproxy/counters.h
+++ b/include/haproxy/counters.h
@@ -43,11 +43,13 @@ void counters_be_shared_drop(struct be_counters_shared *counters);
  */
 #define COUNTERS_SHARED_LAST_OFFSET(scounters, type, offset)                  \
 ({                                                                            \
-       unsigned long last = HA_ATOMIC_LOAD((type *)((char *)scounters[0] + offset));\
+       unsigned long last = 0;                                               \
        unsigned long now_seconds = ns_to_sec(now_ns);                        \
        int it;                                                               \
                                                                               \
-       for (it = 1; (it < global.nbtgroups && scounters[it]); it++) {        \
+       if (scounters)                                                        \
+               last = HA_ATOMIC_LOAD((type *)((char *)scounters[0] + offset));\
+       for (it = 1; (it < global.nbtgroups && scounters); it++) {            \
                unsigned long cur = HA_ATOMIC_LOAD((type *)((char *)scounters[it] + offset));\
                if ((now_seconds - cur) < (now_seconds - last))               \
                        last = cur;                                           \
@@ -74,7 +76,7 @@ void counters_be_shared_drop(struct be_counters_shared *counters);
        uint64_t __ret = 0;                                                   \
         int it;                                                               \
                                                                               \
-       for (it = 0; (it < global.nbtgroups && scounters[it]); it++)          \
+       for (it = 0; (it < global.nbtgroups && scounters); it++)              \
                __ret += rfunc((type *)((char *)scounters[it] + offset));     \
        __ret;                                                                \
 })
@@ -94,7 +96,7 @@ void counters_be_shared_drop(struct be_counters_shared *counters);
        uint64_t __ret = 0;                                                   \
        int it;                                                               \
                                                                               \
-       for (it = 0; (it < global.nbtgroups && scounters[it]); it++)          \
+       for (it = 0; (it < global.nbtgroups && scounters); it++)              \
                __ret += rfunc(&scounters[it]->elem, arg1, arg2);             \
        __ret;                                                                \
 })

diff --git a/include/haproxy/proxy.h b/include/haproxy/proxy.h
index 34274756b8350acb056e7d5e37e76ac9ed0f6e88..a4ba4be5840d055cba86d8005b2d5dd320b628ac 100644 (file)
--- a/include/haproxy/proxy.h
+++ b/include/haproxy/proxy.h
@@ -166,10 +166,11 @@ static inline int proxy_abrt_close(const struct proxy *px)
 /* increase the number of cumulated connections received on the designated frontend */
 static inline void proxy_inc_fe_conn_ctr(struct listener *l, struct proxy *fe)
 {
-       _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_conn);
-       update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->conn_per_sec, 1);
-
-       if (l && l->counters)
+       if (fe->fe_counters.shared.tg) {
+               _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_conn);
+               update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->conn_per_sec, 1);
+       }
+       if (l && l->counters && l->counters->shared.tg)
                _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->cum_conn);
        HA_ATOMIC_UPDATE_MAX(&fe->fe_counters.cps_max,
                             update_freq_ctr(&fe->fe_counters._conn_per_sec, 1));
@@ -178,10 +179,11 @@ static inline void proxy_inc_fe_conn_ctr(struct listener *l, struct proxy *fe)
 /* increase the number of cumulated connections accepted by the designated frontend */
 static inline void proxy_inc_fe_sess_ctr(struct listener *l, struct proxy *fe)
 {
-       _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess);
-       update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->sess_per_sec, 1);
-
-       if (l && l->counters)
+       if (fe->fe_counters.shared.tg) {
+               _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess);
+               update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->sess_per_sec, 1);
+       }
+       if (l && l->counters && l->counters->shared.tg)
                _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->cum_sess);
        HA_ATOMIC_UPDATE_MAX(&fe->fe_counters.sps_max,
                             update_freq_ctr(&fe->fe_counters._sess_per_sec, 1));
@@ -197,17 +199,19 @@ static inline void proxy_inc_fe_cum_sess_ver_ctr(struct listener *l, struct prox
            http_ver > sizeof(fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver) / sizeof(*fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver))
            return;
 
-       _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver[http_ver - 1]);
-       if (l && l->counters)
+       if (fe->fe_counters.shared.tg)
+               _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->cum_sess_ver[http_ver - 1]);
+       if (l && l->counters && l->counters->shared.tg && l->counters->shared.tg[tgid - 1])
                _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->cum_sess_ver[http_ver - 1]);
 }
 
 /* increase the number of cumulated streams on the designated backend */
 static inline void proxy_inc_be_ctr(struct proxy *be)
 {
-       _HA_ATOMIC_INC(&be->be_counters.shared.tg[tgid - 1]->cum_sess);
-       update_freq_ctr(&be->be_counters.shared.tg[tgid - 1]->sess_per_sec, 1);
-
+       if (be->be_counters.shared.tg) {
+               _HA_ATOMIC_INC(&be->be_counters.shared.tg[tgid - 1]->cum_sess);
+               update_freq_ctr(&be->be_counters.shared.tg[tgid - 1]->sess_per_sec, 1);
+       }
        HA_ATOMIC_UPDATE_MAX(&be->be_counters.sps_max,
                             update_freq_ctr(&be->be_counters._sess_per_sec, 1));
 }
@@ -222,10 +226,11 @@ static inline void proxy_inc_fe_req_ctr(struct listener *l, struct proxy *fe,
        if (http_ver >= sizeof(fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req) / sizeof(*fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req))
            return;
 
-       _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req[http_ver]);
-       update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->req_per_sec, 1);
-
-       if (l && l->counters)
+       if (fe->fe_counters.shared.tg) {
+               _HA_ATOMIC_INC(&fe->fe_counters.shared.tg[tgid - 1]->p.http.cum_req[http_ver]);
+               update_freq_ctr(&fe->fe_counters.shared.tg[tgid - 1]->req_per_sec, 1);
+       }
+       if (l && l->counters && l->counters->shared.tg)
                _HA_ATOMIC_INC(&l->counters->shared.tg[tgid - 1]->p.http.cum_req[http_ver]);
        HA_ATOMIC_UPDATE_MAX(&fe->fe_counters.p.http.rps_max,
                             update_freq_ctr(&fe->fe_counters.p.http._req_per_sec, 1));

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 3dd438dd77a4652ae805800c4e193b2cf55765dc..05446e98f5e61aa37d8cce152c854c0c99a16ad1 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -207,9 +207,10 @@ static inline void server_index_id(struct proxy *px, struct server *srv)
 /* increase the number of cumulated streams on the designated server */
 static inline void srv_inc_sess_ctr(struct server *s)
 {
-       _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->cum_sess);
-       update_freq_ctr(&s->counters.shared.tg[tgid - 1]->sess_per_sec, 1);
-
+       if (s->counters.shared.tg) {
+               _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->cum_sess);
+               update_freq_ctr(&s->counters.shared.tg[tgid - 1]->sess_per_sec, 1);
+       }
        HA_ATOMIC_UPDATE_MAX(&s->counters.sps_max,
                             update_freq_ctr(&s->counters._sess_per_sec, 1));
 }
@@ -217,7 +218,8 @@ static inline void srv_inc_sess_ctr(struct server *s)
 /* set the time of last session on the designated server */
 static inline void srv_set_sess_last(struct server *s)
 {
-       HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_sess,  ns_to_sec(now_ns));
+       if (s->counters.shared.tg)
+               HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_sess,  ns_to_sec(now_ns));
 }
 
 /* returns the current server throttle rate between 0 and 100% */

diff --git a/src/backend.c b/src/backend.c
index 54787f5ed32b390541343bc16b5c1aaf5a2a2c97..3f00e1d194a919e3e345a73523a189a9f96a7cd1 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -823,7 +823,8 @@ int assign_server(struct stream *s)
                else if (srv != prev_srv) {
                        if (s->be_tgcounters)
                                _HA_ATOMIC_INC(&s->be_tgcounters->cum_lbconn);
-                       _HA_ATOMIC_INC(&srv->counters.shared.tg[tgid - 1]->cum_lbconn);
+                       if (srv->counters.shared.tg)
+                               _HA_ATOMIC_INC(&srv->counters.shared.tg[tgid - 1]->cum_lbconn);
                }
                stream_set_srv_target(s, srv);
        }
@@ -997,11 +998,13 @@ int assign_server_and_queue(struct stream *s)
                                        s->txn->flags |= TX_CK_DOWN;
                                }
                                s->flags |= SF_REDISP;
-                               _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->redispatches);
+                               if (prev_srv->counters.shared.tg)
+                                       _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->redispatches);
                                if (s->be_tgcounters)
                                        _HA_ATOMIC_INC(&s->be_tgcounters->redispatches);
                        } else {
-                               _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->retries);
+                               if (prev_srv->counters.shared.tg)
+                                       _HA_ATOMIC_INC(&prev_srv->counters.shared.tg[tgid - 1]->retries);
                                if (s->be_tgcounters)
                                        _HA_ATOMIC_INC(&s->be_tgcounters->retries);
                        }

diff --git a/src/cache.c b/src/cache.c
index 67edf1889b34f19f71b301dc2bc27fd79cbef183..5fd2540595b989ad61e59b63b1c6551e653a7db3 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -2133,10 +2133,12 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p
                return ACT_RET_CONT;
 
        if (px == strm_fe(s)) {
-               _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_lookups);
+               if (px->fe_counters.shared.tg)
+                       _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_lookups);
        }
        else {
-               _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_lookups);
+               if (px->be_counters.shared.tg)
+                       _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_lookups);
        }
 
        cache_tree = get_cache_tree_from_hash(cache, read_u32(s->txn->cache_hash));
@@ -2224,10 +2226,12 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p
                                 should_send_notmodified_response(cache, htxbuf(&s->req.buf), res);
 
                        if (px == strm_fe(s)) {
-                               _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_hits);
+                               if (px->fe_counters.shared.tg)
+                                       _HA_ATOMIC_INC(&px->fe_counters.shared.tg[tgid - 1]->p.http.cache_hits);
                        }
                        else {
-                               _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_hits);
+                               if (px->be_counters.shared.tg)
+                                       _HA_ATOMIC_INC(&px->be_counters.shared.tg[tgid - 1]->p.http.cache_hits);
                        }
                        return ACT_RET_CONT;
                } else {

diff --git a/src/check.c b/src/check.c
index 0102f0c36e3c70e4e88358f5aed4f5556964e30a..574700b2b366905146875d267344c42f42e922f4 100644 (file)
--- a/src/check.c
+++ b/src/check.c
@@ -513,7 +513,8 @@ void set_server_check_status(struct check *check, short status, const char *desc
                if ((!(check->state & CHK_ST_AGENT) ||
                    (check->status >= HCHK_STATUS_L57DATA)) &&
                    (check->health > 0)) {
-                       _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_checks);
+                       if (s->counters.shared.tg)
+                               _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_checks);
                        report = 1;
                        check->health--;
                        if (check->health < check->rise)
@@ -740,7 +741,8 @@ void __health_adjust(struct server *s, short status)
        HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
 
        HA_ATOMIC_STORE(&s->consecutive_errors, 0);
-       _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_hana);
+       if (s->counters.shared.tg)
+               _HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->failed_hana);
 
        if (s->check.fastinter) {
                /* timer might need to be advanced, it might also already be

diff --git a/src/server.c b/src/server.c
index 9a2c3b8520f19673f6477b01914442b53abaf4f3..024b032444d88dc06bac6ef38d70b3bd48cb8900 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -7159,7 +7159,8 @@ static void srv_update_status(struct server *s, int type, int cause)
                }
                else if (s->cur_state == SRV_ST_STOPPED) {
                        /* server was up and is currently down */
-                       HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->down_trans);
+                       if (s->counters.shared.tg)
+                               HA_ATOMIC_INC(&s->counters.shared.tg[tgid - 1]->down_trans);
                        _srv_event_hdl_publish(EVENT_HDL_SUB_SERVER_DOWN, cb_data.common, s);
                }
 
@@ -7173,7 +7174,8 @@ static void srv_update_status(struct server *s, int type, int cause)
                }
 
                s->last_change = ns_to_sec(now_ns);
-               HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_state_change, s->last_change);
+               if (s->counters.shared.tg)
+                       HA_ATOMIC_STORE(&s->counters.shared.tg[tgid - 1]->last_state_change, s->last_change);
 
                /* publish the state change */
                _srv_event_hdl_prepare_state(&cb_data.state,
@@ -7193,7 +7195,8 @@ static void srv_update_status(struct server *s, int type, int cause)
                if (last_change < ns_to_sec(now_ns))         // ignore negative times
                        s->proxy->down_time += ns_to_sec(now_ns) - last_change;
                s->proxy->last_change = ns_to_sec(now_ns);
-               HA_ATOMIC_STORE(&s->proxy->be_counters.shared.tg[tgid - 1]->last_state_change, s->proxy->last_change);
+               if (s->proxy->be_counters.shared.tg)
+                       HA_ATOMIC_STORE(&s->proxy->be_counters.shared.tg[tgid - 1]->last_state_change, s->proxy->last_change);
        }
 }