This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
osmith gerrit-no-reply at lists.osmocom.orgosmith has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/25206 ) Change subject: stats: send real last value if no new values come ...................................................................... stats: send real last value if no new values come Background: * Individual values can be added to osmo_stat_item.values at any time. * Stats are reported at a fixed interval (see vty 'stats interval'), e.g. every 10 seconds. * In order to report a new stat value, we use the maximum of all osmo_stat_item.values added since the last report. * By default, we do not send new stat values if they did not change (see vty 'config-stats' -> 'flush-period' default of 0). Fix the following bug: * If 'flush-period' is 0, and no new osmo_stat_item.values are coming in, the last value that gets reported is not necessarily the last entry in osmo_stat_item.values. * For attached reporters (statsd), it could then be that the given stat stays at the wrong value for a long stretch of time (think of several hours/days/forever). Explanation of how the test shows that it is fixed: * stats get reported (value is irrelevant) * osmo_stat_item gets a new value: 20 * osmo_stat_item gets a new value: 10 * stats get reported (value: 20, the maximum of both new values) * osmo_stat_item gets no new values * stats get reported (value: 10, this is new because of the bug fix, the real last value in osmo_stat_item, different from the 20 sent earlier, without the fix it would not send anything here and the last sent value would be 20) * osmo_stat_item gets no new values * stats get reported (nothing gets sent, since the real last value was already sent and 'flush-period' is 0) Fixes: OS#5215 Change-Id: Ibeefd0e3d1dbe4be454ff05a21df4848b2abfabe --- M TODO-RELEASE M include/osmocom/core/stat_item.h M src/stats.c M tests/stats/stats_test.c M tests/stats/stats_test.err 5 files changed, 15 insertions(+), 1 deletion(-) Approvals: neels: Looks good to me, approved daniel: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/TODO-RELEASE b/TODO-RELEASE index 260e1d4..c8966cd 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -15,3 +15,4 @@ libosmosim osim_card_{reset,close} New API libosmocore struct rate_ctr_group, osmo_stat_item_group_desc ABI breakage due to new struct members libosmgsm kdf functions New API +libosmocore osmo_stat_item ABI + API breakage due to new struct members diff --git a/include/osmocom/core/stat_item.h b/include/osmocom/core/stat_item.h index fbe0433..332bbe9 100644 --- a/include/osmocom/core/stat_item.h +++ b/include/osmocom/core/stat_item.h @@ -28,6 +28,10 @@ * be read from the FIFO. If accessing osmo_stat_item directly, without * the stats API, store this value elsewhere. */ int32_t stats_next_id; + /* internal use by stats API: indicate if the last value sent to + * reporters was actually the last value in the FIFO. This may not be + * the case, as always a max of 1 or more values gets sent (OS#5215) */ + bool stats_last_sent_was_max; /*! the index of the last value written to the FIFO */ int16_t last_offs; /*! value FIFO */ diff --git a/src/stats.c b/src/stats.c index 411ecff..f06515d 100644 --- a/src/stats.c +++ b/src/stats.c @@ -715,6 +715,11 @@ if (!have_value) { /* Send the last value in case a flush is requested */ value = osmo_stat_item_get_last(item); + + /* Also send it in case a different max value was sent + * previously (OS#5215) */ + if (!item->stats_last_sent_was_max) + have_value = 1; } else { int32_t next_val; /* If we have multiple values only send the max */ @@ -722,6 +727,8 @@ value = OSMO_MAX(value, next_val); } + item->stats_last_sent_was_max = (osmo_stat_item_get_last(item) == value); + llist_for_each_entry(srep, &osmo_stats_reporter_list, list) { if (!srep->running) continue; diff --git a/tests/stats/stats_test.c b/tests/stats/stats_test.c index 6505e66..2f2c6ec 100644 --- a/tests/stats/stats_test.c +++ b/tests/stats/stats_test.c @@ -445,7 +445,7 @@ fprintf(stderr, "report (group 1, item 1 no update, send last item (!= last max), OS#5215):\n"); send_count = 0; osmo_stats_report(); - OSMO_ASSERT(send_count == 0); /* BUG: should be 2! */ + OSMO_ASSERT(send_count == 2); fprintf(stderr, "report (group 1, item 1 no update, nothing to send):\n"); send_count = 0; diff --git a/tests/stats/stats_test.err b/tests/stats/stats_test.err index 08c2cbc..daa3e5c 100644 --- a/tests/stats/stats_test.err +++ b/tests/stats/stats_test.err @@ -116,6 +116,8 @@ test2: item p= g=test.one i=1 n=item.a v=20 u=ma test1: item p= g=test.one i=1 n=item.a v=20 u=ma report (group 1, item 1 no update, send last item (!= last max), OS#5215): + test2: item p= g=test.one i=1 n=item.a v=10 u=ma + test1: item p= g=test.one i=1 n=item.a v=10 u=ma report (group 1, item 1 no update, nothing to send): report (remove statg1, ctrg1): test2: counter p= g=ctr-test:one_dot i=3 n=ctr:a v=0 d=0 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/25206 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibeefd0e3d1dbe4be454ff05a21df4848b2abfabe Gerrit-Change-Number: 25206 Gerrit-PatchSet: 5 Gerrit-Owner: osmith <osmith at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann at sysmocom.de> Gerrit-Reviewer: dexter <pmaier at sysmocom.de> Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: osmith <osmith at sysmocom.de> Gerrit-MessageType: merged -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210820/fb465e7f/attachment.htm>