From 73160d3d0acab80cefa8bd29b73558e9dfaca5cc Mon Sep 17 00:00:00 2001 From: Alexander Chemeris Date: Fri, 8 May 2020 19:10:40 +0300 Subject: stats: Support regular stats flush Reliable monitoring requires regular flush of all stat values, even if they have not changed. Otherwise (1) the monitoring app has to maintain state and (2) can go out of sync if it's restarted while the app is still running. Change-Id: I04f1e7bdf0d6f20e4f15571e94191de61c47ddad --- include/osmocom/core/stats.h | 5 ++++- src/stats.c | 29 +++++++++++++++++++++++++++++ src/vty/stats_vty.c | 26 ++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/include/osmocom/core/stats.h b/include/osmocom/core/stats.h index e01016d4..b9edac2a 100644 --- a/include/osmocom/core/stats.h +++ b/include/osmocom/core/stats.h @@ -73,6 +73,7 @@ struct osmo_stats_reporter { char *bind_addr_str; /*!< local bind IP address */ int dest_port; /*!< destination (UDP) port */ int mtu; /*!< Maximum Transmission Unit */ + unsigned int flush_period; /*!< period between regular flushes */ /*! Maximum class/index to report. FIXME: More details! */ enum osmo_stats_class max_class; @@ -87,7 +88,8 @@ struct osmo_stats_reporter { int fd; /*!< file descriptor of socket */ struct msgb *buffer; /*!< message buffer for log output */ int agg_enabled; /*!< is aggregation enabled? */ - int force_single_flush; + int force_single_flush; /*!< set to 1 to force a flush (send even unchanged stats values) */ + unsigned int flush_period_counter; /*!< count sends between forced flushes */ struct llist_head list; int (*open)(struct osmo_stats_reporter *srep); @@ -129,6 +131,7 @@ int osmo_stats_reporter_set_max_class(struct osmo_stats_reporter *srep, int osmo_stats_reporter_set_name_prefix(struct osmo_stats_reporter *srep, const char *prefix); int osmo_stats_reporter_enable(struct osmo_stats_reporter *srep); int osmo_stats_reporter_disable(struct osmo_stats_reporter *srep); +int osmo_stats_reporter_set_flush_period(struct osmo_stats_reporter *srep, unsigned int period); /* reporter creation */ struct osmo_stats_reporter *osmo_stats_reporter_create_log(const char *name); diff --git a/src/stats.c b/src/stats.c index 61b7df87..5954167a 100644 --- a/src/stats.c +++ b/src/stats.c @@ -341,6 +341,25 @@ int osmo_stats_set_interval(int interval) return 0; } +/*! Set the regular flush period for a given stats_reporter + * + * Send all stats even if they have not changed (i.e. force the flush) + * every N-th reporting interval. Set to 0 to disable regular flush, + * set to 1 to flush every time, set to 2 to flush every 2nd time, etc. + * \param[in] srep stats_reporter to set flush period for + * \param[in] period Reporting interval in seconds + * \returns 0 on success; negative on error */ +int osmo_stats_reporter_set_flush_period(struct osmo_stats_reporter *srep, unsigned int period) +{ + srep->flush_period = period; + srep->flush_period_counter = 0; + /* force the flush now if it's not disabled by period=0 */ + if (period > 0) + srep->force_single_flush = 1; + + return 0; +} + /*! Set the name prefix of a given stats_reporter. * \param[in] srep stats_reporter whose name prefix is to be set * \param[in] prefix NAme perfix to pre-pend for any reported value @@ -706,7 +725,17 @@ static void flush_all_reporters() continue; osmo_stats_reporter_send_buffer(srep); + + /* reset force_single_flush first */ srep->force_single_flush = 0; + /* and schedule a new flush if it's time for it */ + if (srep->flush_period > 0) { + srep->flush_period_counter++; + if (srep->flush_period_counter >= srep->flush_period) { + srep->force_single_flush = 1; + srep->flush_period_counter = 0; + } + } } } diff --git a/src/vty/stats_vty.c b/src/vty/stats_vty.c index a512703f..46282817 100644 --- a/src/vty/stats_vty.c +++ b/src/vty/stats_vty.c @@ -245,6 +245,27 @@ DEFUN(cfg_stats_reporter_disable, cfg_stats_reporter_disable_cmd, return CMD_SUCCESS; } +DEFUN(cfg_stats_reporter_flush_period, cfg_stats_reporter_flush_period_cmd, + "flush-period <0-65535>", + CFG_STATS_STR "Send all stats even if they have not changed (i.e. force the flush)" + "every N-th reporting interval. Set to 0 to disable regular flush (default).\n" + "0 to disable regular flush (default), 1 to flush every time, 2 to flush every 2nd time, etc\n") +{ + int rc; + unsigned int period = atoi(argv[0]); + struct osmo_stats_reporter *srep = osmo_stats_vty2srep(vty); + OSMO_ASSERT(srep); + + rc = osmo_stats_reporter_set_flush_period(srep, period); + if (rc < 0) { + vty_out(vty, "%% Unable to set force flush period: %s%s", + strerror(-rc), VTY_NEWLINE); + return CMD_WARNING; + } + + return CMD_SUCCESS; +} + DEFUN(cfg_stats_reporter_statsd, cfg_stats_reporter_statsd_cmd, "stats reporter statsd", CFG_STATS_STR CFG_REPORTER_STR "Report to a STATSD server\n") @@ -588,6 +609,10 @@ static int config_write_stats_reporter(struct vty *vty, struct osmo_stats_report else vty_out(vty, " no prefix%s", VTY_NEWLINE); + if (srep->flush_period > 0) + vty_out(vty, " flush-period %d%s", + srep->flush_period, VTY_NEWLINE); + if (srep->enabled) vty_out(vty, " enable%s", VTY_NEWLINE); @@ -637,6 +662,7 @@ void osmo_stats_vty_add_cmds() install_element(CFG_STATS_NODE, &cfg_stats_reporter_level_cmd); install_element(CFG_STATS_NODE, &cfg_stats_reporter_enable_cmd); install_element(CFG_STATS_NODE, &cfg_stats_reporter_disable_cmd); + install_element(CFG_STATS_NODE, &cfg_stats_reporter_flush_period_cmd); install_element_ve(&show_stats_asciidoc_table_cmd); install_element_ve(&show_rate_counters_cmd); -- cgit v1.2.3 From e3f785fbf179bb385ae4bfea28bed90159e57789 Mon Sep 17 00:00:00 2001 From: Alexander Chemeris Date: Sat, 9 May 2020 01:57:16 +0300 Subject: select: Fix typo in a comment Osmcoom->Osmocom Change-Id: I6fb4d20d149abc724d477420b5eba482a0b63259 --- src/select.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/select.c b/src/select.c index 8e312054..774056a9 100644 --- a/src/select.c +++ b/src/select.c @@ -334,7 +334,7 @@ int osmo_timerfd_disable(struct osmo_fd *ofd) return timerfd_settime(ofd->fd, 0, &its_null, NULL); } -/*! schedule the osmcoom-wrapped timerfd to occur first at \a first, then periodically at \a interval +/*! schedule the osmocom-wrapped timerfd to occur first at \a first, then periodically at \a interval * \param[in] ofd Osmocom wrapped timerfd * \param[in] first Relative time at which the timer should first execute (NULL = \a interval) * \param[in] interval Time interval at which subsequent timer shall fire -- cgit v1.2.3 From 4b3e4dc86674af172e6be24fce843902db77756d Mon Sep 17 00:00:00 2001 From: Alexander Chemeris Date: Sat, 9 May 2020 01:57:51 +0300 Subject: stats: Change timer to timerfd to make it a true interval timer. Previously the interval between stats flushes would slowly increase which would lead to reporting time jitter and confuse a timescale database. Change-Id: I23d8b5157ef8a9833ba16a81d9b28a126f303c30 --- src/stats.c | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/stats.c b/src/stats.c index 5954167a..8cccf67b 100644 --- a/src/stats.c +++ b/src/stats.c @@ -85,7 +85,7 @@ #include #include #include -#include +#include #include #include @@ -102,7 +102,7 @@ static struct osmo_stats_config s_stats_config = { }; struct osmo_stats_config *osmo_stats_config = &s_stats_config; -static struct osmo_timer_list osmo_stats_timer; +static struct osmo_fd osmo_stats_timer = {.fd=-1}; static int osmo_stats_reporter_log_send_counter(struct osmo_stats_reporter *srep, const struct rate_ctr_group *ctrg, @@ -140,23 +140,48 @@ static int update_srep_config(struct osmo_stats_reporter *srep) return rc; } -static void osmo_stats_timer_cb(void *data) +static int osmo_stats_timer_cb(struct osmo_fd *ofd, unsigned int what) { - int interval = osmo_stats_config->interval; + uint64_t expire_count; + int rc; + + /* check that the timer has actually expired */ + if (!(what & BSC_FD_READ)) + return 0; + + /* read from timerfd: number of expirations of periodic timer */ + rc = read(ofd->fd, (void *) &expire_count, sizeof(expire_count)); + if (rc < 0 && errno == EAGAIN) + return 0; + OSMO_ASSERT(rc == sizeof(expire_count)); if (!llist_empty(&osmo_stats_reporter_list)) osmo_stats_report(); - osmo_timer_schedule(&osmo_stats_timer, interval, 0); + return 0; } static int start_timer() { + int rc; + int interval = osmo_stats_config->interval; + if (!is_initialised) return -ESRCH; - osmo_timer_setup(&osmo_stats_timer, osmo_stats_timer_cb, NULL); - osmo_timer_schedule(&osmo_stats_timer, 0, 1); + struct timespec ts_first = {.tv_sec=0, .tv_nsec=1000}; + struct timespec ts_interval = {.tv_sec=interval, .tv_nsec=0}; + + rc = osmo_timerfd_setup(&osmo_stats_timer, osmo_stats_timer_cb, NULL); + if (rc < 0) + LOGP(DLSTATS, LOGL_ERROR, "Failed to setup the timer with error code %d (fd=%d)\n", + rc, osmo_stats_timer.fd); + rc = osmo_timerfd_schedule(&osmo_stats_timer, &ts_first, &ts_interval); + if (rc < 0) + LOGP(DLSTATS, LOGL_ERROR, "Failed to schedule the timer with error code %d (fd=%d, interval %d sec)\n", + rc, osmo_stats_timer.fd, interval); + + LOGP(DLSTATS, LOGL_INFO, "Stats timer started with interval %d sec\n", interval); return 0; } -- cgit v1.2.3