MT#55283 simplify/clarify min/max stats gathering

Rename structs and variables to make it clear that these min/max values
are min/max per-sec rate values.

Carry mins and maxes separately from averages. This changes the meaning
of $command_ps_avg away from an "average of averages" to an actual
average, which is more accurate.

Calculate this average based on per-interval differences and interval
duration (stats_rate_min_max_avg_sample).

Side effect: As rtpe_latest_graphite_interval_start is now set in
print_graphite_data instead of in graphite_loop_run, the test now
reports a different "interval calls duration".

Change-Id: I67b1118c18ca2464a48c4836fca3cfdb4d53c898

daemon/call.c

@@ -570,11 +570,13 @@ void call_timer(void *ptr) {
 
 	stats_counters_calc_rate(&rtpe_stats, run_diff_us, &rtpe_stats_intv, &rtpe_stats_rate);
 
-	stats_counters_min_max(&rtpe_stats_graphite_min_max, &rtpe_stats_rate);
+	// TODO: should be moved into a separate thread/timer
+	stats_rate_min_max(&rtpe_rate_graphite_min_max, &rtpe_stats_rate);
 
 	// stats derived while iterating calls
 	RTPE_GAUGE_SET(transcoded_media, hlp.transcoded_media);
 
+	// TODO: eliminate/split out most of what this single central timer does
 	i = hlp.count ? kernel_list() : NULL;
 	while (i) {
 		ke = i->data;

daemon/graphite.c

@@ -40,8 +40,8 @@ static struct global_stats_counter rtpe_stats_graphite_intv;	// copied out when
 struct global_stats_gauge_min_max rtpe_stats_gauge_graphite_min_max;
 struct global_stats_gauge_min_max rtpe_stats_gauge_graphite_min_max_interval;
 
-struct global_stats_min_max rtpe_stats_graphite_min_max;
-struct global_stats_min_max rtpe_stats_graphite_min_max_interval;
+struct global_rate_min_max rtpe_rate_graphite_min_max;
+struct global_rate_min_max_avg rtpe_rate_graphite_min_max_avg_sampled;
 
 
 void set_graphite_interval_tv(struct timeval *tv) {
@@ -89,8 +89,11 @@ static int connect_to_graphite_server(const endpoint_t *graphite_ep) {
 
 GString *print_graphite_data(void) {
 
+	long long time_diff_us = timeval_diff(&rtpe_now, &rtpe_latest_graphite_interval_start);
+	rtpe_latest_graphite_interval_start = rtpe_now;
+
 	stats_counters_calc_diff(&rtpe_stats, &rtpe_stats_graphite_intv, &rtpe_stats_graphite_diff);
-	stats_counters_min_max_reset(&rtpe_stats_graphite_min_max, &rtpe_stats_graphite_min_max_interval);
+	stats_rate_min_max_avg_sample(&rtpe_rate_graphite_min_max, &rtpe_rate_graphite_min_max_avg_sampled, time_diff_us, &rtpe_stats_graphite_diff);
 	stats_gauge_calc_avg_reset(&rtpe_stats_gauge_graphite_min_max_interval, &rtpe_stats_gauge_graphite_min_max);
 
 	GString *graph_str = g_string_new("");
@@ -108,9 +111,9 @@ GString *print_graphite_data(void) {
 		GPF("%s_time_avg %.6f", ng_command_strings[i],
 				(double) atomic64_get(&rtpe_stats_gauge_graphite_min_max_interval.avg.ng_command_times[i]) / 1000000.0);
 
-		GPF("%ss_ps_min " UINT64F, ng_command_strings[i], atomic64_get(&rtpe_stats_graphite_min_max_interval.min.ng_commands[i]));
-		GPF("%ss_ps_max " UINT64F, ng_command_strings[i], atomic64_get(&rtpe_stats_graphite_min_max_interval.max.ng_commands[i]));
-		GPF("%ss_ps_avg " UINT64F, ng_command_strings[i], atomic64_get(&rtpe_stats_graphite_min_max_interval.avg.ng_commands[i]));
+		GPF("%ss_ps_min " UINT64F, ng_command_strings[i], atomic64_get(&rtpe_rate_graphite_min_max_avg_sampled.min.ng_commands[i]));
+		GPF("%ss_ps_max " UINT64F, ng_command_strings[i], atomic64_get(&rtpe_rate_graphite_min_max_avg_sampled.max.ng_commands[i]));
+		GPF("%ss_ps_avg " UINT64F, ng_command_strings[i], atomic64_get(&rtpe_rate_graphite_min_max_avg_sampled.avg.ng_commands[i]));
 
 		ilog(LOG_DEBUG, "Min/Max/Avg %s processing delay: %.6f/%.6f/%.6f sec",
 			ng_command_strings[i],
@@ -310,7 +313,6 @@ static void graphite_loop_run(endpoint_t *graphite_ep, int seconds) {
 
 		gettimeofday(&rtpe_now, NULL);
 		rc = send_graphite_data();
-		rtpe_latest_graphite_interval_start = rtpe_now;
 		if (rc < 0) {
 			ilog(LOG_ERROR,"Sending graphite data failed.");
 			close_socket(&graphite_sock);

daemon/statistics.c

@@ -485,9 +485,9 @@ GQueue *statistics_gather_metrics(void) {
 	}
 
 	for (int i = 0; i < NGC_COUNT; i++) {
-		uint64_t min = atomic64_get(&rtpe_stats_graphite_min_max_interval.min.ng_commands[i]);
-		uint64_t max = atomic64_get(&rtpe_stats_graphite_min_max_interval.max.ng_commands[i]);
-		uint64_t avg = atomic64_get(&rtpe_stats_graphite_min_max_interval.avg.ng_commands[i]);
+		uint64_t min = atomic64_get(&rtpe_rate_graphite_min_max_avg_sampled.min.ng_commands[i]);
+		uint64_t max = atomic64_get(&rtpe_rate_graphite_min_max_avg_sampled.max.ng_commands[i]);
+		uint64_t avg = atomic64_get(&rtpe_rate_graphite_min_max_avg_sampled.avg.ng_commands[i]);
 		AUTO_CLEANUP(char *min_label, free_gbuf) = g_strdup_printf("min%srequestrate", ng_command_strings[i]);
 		AUTO_CLEANUP(char *max_label, free_gbuf) = g_strdup_printf("max%srequestrate", ng_command_strings[i]);
 		AUTO_CLEANUP(char *avg_label, free_gbuf) = g_strdup_printf("avg%srequestrate", ng_command_strings[i]);

include/graphite.h

@@ -21,6 +21,10 @@ extern struct timeval rtpe_latest_graphite_interval_start;
 extern struct global_stats_gauge_min_max rtpe_stats_gauge_graphite_min_max;
 extern struct global_stats_gauge_min_max rtpe_stats_gauge_graphite_min_max_interval;
 
+extern struct global_stats_counter rtpe_stats_graphite_diff;	// per-interval increases
+extern struct global_rate_min_max rtpe_rate_graphite_min_max;	// running min/max, reset when graphite runs
+extern struct global_rate_min_max_avg rtpe_rate_graphite_min_max_avg_sampled; // updated once per graphite run
+
 
 void set_prefix(char* prefix);
 void free_prefix(void);

include/statistics.h

@@ -58,11 +58,14 @@ struct global_stats_counter {
 #undef FA
 };
 
-struct global_stats_min_max {
+struct global_rate_min_max {
 	struct global_stats_counter min;
 	struct global_stats_counter max;
-	struct global_stats_counter avg; // sum while accumulation is running
-	atomic64 count;
+};
+struct global_rate_min_max_avg {
+	struct global_stats_counter min;
+	struct global_stats_counter max;
+	struct global_stats_counter avg;
 };
 
 
@@ -136,9 +139,6 @@ extern struct global_stats_gauge_min_max rtpe_stats_gauge_cumulative; // lifetim
 
 extern struct global_stats_counter rtpe_stats;			// total, cumulative, master
 extern struct global_stats_counter rtpe_stats_rate;		// per-second, calculated once per timer run
-extern struct global_stats_counter rtpe_stats_graphite_diff;	// per-interval increases
-extern struct global_stats_min_max rtpe_stats_graphite_min_max; // running min/max
-extern struct global_stats_min_max rtpe_stats_graphite_min_max_interval; // updated once per graphite run
 
 #define RTPE_STATS_ADD(field, num) atomic64_add(&rtpe_stats.field, num)
 #define RTPE_STATS_INC(field) RTPE_STATS_ADD(field, 1)
@@ -175,25 +175,22 @@ INLINE void stats_counters_calc_diff(const struct global_stats_counter *stats,
 #undef F
 }
 
-INLINE void stats_counters_min_max(struct global_stats_min_max *mm, struct global_stats_counter *inp) {
+// update the running min/max counter `mm` with the newly calculated per-sec rate values `inp`
+INLINE void stats_rate_min_max(struct global_rate_min_max *mm, struct global_stats_counter *inp) {
 #define F(x) \
 	atomic64_mina(&mm->min.x, &inp->x); \
-	atomic64_maxa(&mm->max.x, &inp->x); \
-	atomic64_add(&mm->avg.x, atomic64_get(&inp->x));
+	atomic64_maxa(&mm->max.x, &inp->x);
 #include "counter_stats_fields.inc"
 #undef F
-	atomic64_inc(&mm->count);
 }
-INLINE void stats_counters_min_max_reset(struct global_stats_min_max *mm, struct global_stats_min_max *loc) {
-	uint64_t count = atomic64_get_set(&mm->count, 0);
-
+// sample running min/max from `mm` into `loc` and reset `mm` to zero.
+// calculate average values in `loc` from `counter_diff` and `time_diff_us`
+INLINE void stats_rate_min_max_avg_sample(struct global_rate_min_max *mm, struct global_rate_min_max_avg *loc,
+		long long run_diff_us, const struct global_stats_counter *counter_diff) {
 #define F(x) \
 	atomic64_set(&loc->min.x, atomic64_get_set(&mm->min.x, 0)); \
 	atomic64_set(&loc->max.x, atomic64_get_set(&mm->max.x, 0)); \
-	if (count) \
-		atomic64_set(&loc->avg.x, atomic64_get_set(&mm->avg.x, 0) / count); \
-	else \
-		atomic64_set(&loc->avg.x, 0);
+	atomic64_set(&loc->avg.x, run_diff_us ? atomic64_get(&counter_diff->x) * 1000000LL / run_diff_us : 0);
 #include "counter_stats_fields.inc"
 #undef F
 }

t/test-payload-tracker.c

@@ -13,8 +13,8 @@ struct global_stats_gauge_min_max rtpe_stats_gauge_graphite_min_max_interval;
 struct global_stats_counter rtpe_stats;
 struct global_stats_counter rtpe_stats_rate;
 struct global_stats_counter rtpe_stats_graphite_diff;
-struct global_stats_min_max rtpe_stats_graphite_min_max;
-struct global_stats_min_max rtpe_stats_graphite_min_max_interval;
+struct global_rate_min_max rtpe_rate_graphite_min_max;
+struct global_rate_min_max_avg rtpe_rate_graphite_min_max_avg_sampled;
 
 
 static void most_cmp(struct payload_tracker *t, const char *cmp, const char *file, int line) {

t/test-stats.c

@@ -3254,7 +3254,7 @@ int main(void) {
 			"offer_time_avg 0.000000 157\n"
 			"offers_ps_min 40 157\n"
 			"offers_ps_max 60 157\n"
-			"offers_ps_avg 33 157\n"
+			"offers_ps_avg 45 157\n"
 			"offer_count 322 157\n"
 			"answer_time_min 0.000000 157\n"
 			"answer_time_max 0.000000 157\n"
@@ -3868,13 +3868,13 @@ int main(void) {
 			"avgpingrequestrate\n"
 			"0\n"
 			"Min/Max/Avg offer requests per second\n"
-			"40/60/33 per sec\n"
+			"40/60/45 per sec\n"
 			"minofferrequestrate\n"
 			"40\n"
 			"maxofferrequestrate\n"
 			"60\n"
 			"avgofferrequestrate\n"
-			"33\n"
+			"45\n"
 			"Min/Max/Avg answer requests per second\n"
 			"0/0/0 per sec\n"
 			"minanswerrequestrate\n"
@@ -5528,7 +5528,7 @@ int main(void) {
 			"unsubscribes_ps_max 0 200\n"
 			"unsubscribes_ps_avg 0 200\n"
 			"unsubscribe_count 0 200\n"
-			"call_dur 186.000000 200\n"
+			"call_dur 143.000000 200\n"
 			"average_call_dur 0.000000 200\n"
 			"forced_term_sess 0 200\n"
 			"managed_sess 0 200\n"
@@ -5759,8 +5759,8 @@ int main(void) {
 			"{\n"
 			"Total calls duration\n"
 			"totalcallsduration\n"
-			"186.000000 seconds\n"
-			"186.000000\n"
+			"143.000000 seconds\n"
+			"143.000000\n"
 			"\n"
 			"Min managed sessions\n"
 			"minmanagedsessions\n"