fix: replace setitimer with getrusage for CPU metrics (#828)

The setitimer-based CPU measurement used SIGVTALRM and SIGPROF
signals, which killed processes during long operations like
metadata loading. The fix added in #544 and #573 caught all
timer signals (including SIGALRM), which silently broke
SignalLoopWatchdog across the master.

Replace the countdown-timer trick with getrusage ru_utime and
ru_stime deltas, eliminating SIGVTALRM and SIGPROF entirely.
Remove the now-unnecessary signal interception from main.cc,
restoring SignalLoopWatchdog as sole SIGALRM owner.

Signed-off-by: Crash <crash@leil.io>
Signed-off-by: Dave <dave@leil.io>
Signed-off-by: guillex <guillex@leil.io>

Author: lgsilva3087

config.h.in

@@ -59,6 +59,7 @@
 #cmakedefine SAUNAFS_HAVE_STRING_H
 #cmakedefine SAUNAFS_HAVE_SYS_MMAN_H
 #cmakedefine SAUNAFS_HAVE_SYS_RESOURCE_H
+#cmakedefine SAUNAFS_HAVE_SYS_RUSAGE_H
 #cmakedefine SAUNAFS_HAVE_SYS_SOCKET_H
 #cmakedefine SAUNAFS_HAVE_SYS_STATVFS_H
 #cmakedefine SAUNAFS_HAVE_SYS_TIME_H

src/chunkserver/chartsdata.cc

@@ -23,13 +23,10 @@
 #include "chunkserver/chartsdata.h"
 
 #include <fcntl.h>
-#include <sys/resource.h>
-#include <sys/time.h>
 #include <syslog.h>
 #include <unistd.h>
 #include <algorithm>
 #include <cerrno>
-#include <csignal>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
@@ -41,8 +38,27 @@
 #include "chunkserver/network_stats.h"
 #include "common/charts.h"
 #include "common/event_loop.h"
+#include "common/time_utils.h"
 #include "slogger/slogger.h"
 
+#if defined(SAUNAFS_HAVE_GETRUSAGE)
+#include <sys/types.h>
+#ifdef SAUNAFS_HAVE_SYS_RESOURCE_H
+#include <sys/resource.h>
+#endif
+#ifdef SAUNAFS_HAVE_SYS_RUSAGE_H
+#include <sys/rusage.h>
+#endif
+#ifndef RUSAGE_SELF
+#define RUSAGE_SELF 0
+#endif
+#define CPU_USAGE 1
+#endif
+
+#if defined(CPU_USAGE) && defined(SAUNAFS_HAVE_STRUCT_RUSAGE_RU_MAXRSS)
+#define MEMORY_USAGE 1
+#endif
+
 #define CHARTS_FILENAME "csstats.sfs"
 
 #define CHARTS_UCPU 0
@@ -82,8 +98,6 @@
 
 #define CHARTS_NUMBER 34
 
-const unsigned long kLinuxMaxrssSize = 1024UL;
-
 /* name , join mode , percent , scale , multiplier , divisor */
 #define STATDEFS { \
 	{"ucpu"             ,CHARTS_MODE_ADD,1,CHARTS_SCALE_MICRO, 100,60}, \
@@ -140,32 +154,26 @@ const unsigned long kLinuxMaxrssSize = 1024UL;
 	{CHARTS_NONE                            ,CHARTS_NONE                            ,CHARTS_NONE           ,0              ,0,0                 ,   0, 0}  \
 };
 
-static const uint32_t calcdefs[]=CALCDEFS
-static const statdef statdefs[]=STATDEFS
-static const estatdef estatdefs[]=ESTATDEFS
+static const uint32_t calcdefs[] = CALCDEFS;
+static const statdef statdefs[] = STATDEFS;
+static const estatdef estatdefs[] = ESTATDEFS;
 
-static struct itimerval it_set;
-
-inline uint32_t toMicroSeconds(struct itimerval &itimer) {
-    return itimer.it_value.tv_sec * 1000000 + itimer.it_value.tv_usec;
-}
+#ifdef CPU_USAGE
+static struct rusage prev_rusage;
+static bool prevRusageValid = false;
+#endif
 
 // NOLINTNEXTLINE(misc-use-anonymous-namespace)
-static uint64_t GetMemUsage() {
-	struct rusage resUse{};
-	int err = getrusage(RUSAGE_SELF, &resUse);
-	if (err != -1) {
+#ifdef MEMORY_USAGE
+static uint64_t GetMemUsage(const struct rusage &resUse) {
 #ifdef __APPLE__
-		return resUse.ru_maxrss;
+	return resUse.ru_maxrss;
 #else
-		// NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access)
-		return resUse.ru_maxrss * kLinuxMaxrssSize;
+	// NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access)
+	return static_cast<uint64_t>(resUse.ru_maxrss) * UINT64_C(1024);
 #endif
-	} else {
-		safs::log_error_code(errno, "could not get memory usage for chartsdata");
-		return 0;
-	}
 }
+#endif
 
 void chartsdata_refresh(void) {
 	uint64_t data[CHARTS_NUMBER];
@@ -175,54 +183,40 @@ void chartsdata_refresh(void) {
 	uint32_t opsDupTrunc, opsTest, opsGCPurge;
 	uint32_t maxChunkServerJobsCount, maxMasterJobsCount;
 	int64_t usedSpaceDelta;
+#ifdef MEMORY_USAGE
+	struct rusage ru{};
+	bool haveRusage = false;
+#endif
 
-	// Timer runs only when the process is executing.
-	struct itimerval userTime;
-
-	// Timer runs when the process is executing and when
-	// the system is executing on behalf of the process.
-	struct itimerval procTime;
-
-	uint32_t userTimeMicroSeconds, procTimeMicroSeconds;
-
-	for (auto i = 0; i < CHARTS_NUMBER; ++i) {
-		data[i] = 0;
-	}
-
-	setitimer(ITIMER_VIRTUAL, &it_set, &userTime); // user time
-	setitimer(ITIMER_PROF, &it_set, &procTime);    // user time + system time
-
-	// on fucken linux timers can go backward !!!
-	if (userTime.it_value.tv_sec <= 999) {
-		userTime.it_value.tv_sec = 999 - userTime.it_value.tv_sec;
-		userTime.it_value.tv_usec = 999999 - userTime.it_value.tv_usec;
-	} else {
-		userTime.it_value.tv_sec = 0;
-		userTime.it_value.tv_usec = 0;
-	}
+	for (auto i = 0; i < CHARTS_NUMBER; ++i) { data[i] = CHARTS_NODATA; }
 
-	// as abowe - who the hell has invented this stupid os !!!
-	if (procTime.it_value.tv_sec <= 999) {
-		procTime.it_value.tv_sec = 999 - procTime.it_value.tv_sec;
-		procTime.it_value.tv_usec = 999999 - procTime.it_value.tv_usec;
+#ifdef CPU_USAGE
+	// CPU usage via getrusage deltas
+	struct rusage cur{};
+	if (getrusage(RUSAGE_SELF, &cur) == -1) {
+		safs::log_error_code(errno, "could not get cpu usage for chartsdata");
 	} else {
-		procTime.it_value.tv_sec = 0;
-		procTime.it_value.tv_usec = 0;
+		if (prevRusageValid) {
+			data[CHARTS_UCPU] = timeDiffUsec(cur.ru_utime, prev_rusage.ru_utime);
+			data[CHARTS_SCPU] = timeDiffUsec(cur.ru_stime, prev_rusage.ru_stime);
+		}
+		prev_rusage = cur;
+		prevRusageValid = true;
+
+#ifdef MEMORY_USAGE
+		ru = cur;
+		haveRusage = true;
+#endif
 	}
+#endif
 
-	userTimeMicroSeconds = toMicroSeconds(userTime);
-	procTimeMicroSeconds = toMicroSeconds(procTime);
-
-	if (procTimeMicroSeconds > userTimeMicroSeconds) {
-		procTimeMicroSeconds -= userTimeMicroSeconds;
+#ifdef MEMORY_USAGE
+	if (!haveRusage && getrusage(RUSAGE_SELF, &ru) == -1) {
+		safs::log_error_code(errno, "could not get memory usage for chartsdata");
 	} else {
-		procTimeMicroSeconds = 0;
+		data[CHARTS_MEMORY] = GetMemUsage(ru);
 	}
-
-	data[CHARTS_MEMORY] = GetMemUsage();
-
-	data[CHARTS_UCPU] = userTimeMicroSeconds;
-	data[CHARTS_SCPU] = procTimeMicroSeconds;
+#endif
 
 	masterconn_stats(&bytesIn, &bytesOut, &maxMasterJobsCount);
 	data[CHARTS_MASTERIN] = bytesIn;
@@ -283,15 +277,13 @@ void chartsdata_store(void) {
 }
 
 int chartsdata_init() {
-	struct itimerval userTime, procTime;
-
-	it_set.it_interval.tv_sec = 0;
-	it_set.it_interval.tv_usec = 0;
-	it_set.it_value.tv_sec = 999;
-	it_set.it_value.tv_usec = 999999;
-
-	setitimer(ITIMER_VIRTUAL, &it_set, &userTime); // user time
-	setitimer(ITIMER_PROF, &it_set, &procTime);    // user time + system time
+#ifdef CPU_USAGE
+	if (getrusage(RUSAGE_SELF, &prev_rusage) == -1) {
+		safs::log_error_code(errno, "could not initialize cpu usage for chartsdata");
+	} else {
+		prevRusageValid = true;
+	}
+#endif
 
 	eventloop_timeregister(TIMEMODE_RUN_LATE, SECONDS_IN_ONE_MINUTE, 0, chartsdata_refresh);
 	eventloop_timeregister(TIMEMODE_RUN_LATE, SECONDS_IN_ONE_HOUR, 0, chartsdata_store);

src/common/time_utils.cc

@@ -21,6 +21,7 @@
 #include "common/platform.h"
 #include "common/time_utils.h"
 
+#include <sys/time.h>
 #include <chrono>
 #include <ratio>
 
@@ -46,6 +47,25 @@ static int64_t duration_int64_cast(Dur duration) {
 	return std::chrono::duration_cast<std::chrono::duration<int64_t, Ratio2>>(duration).count();
 }
 
+namespace {
+constexpr int64_t kMicrosecondsInSecond = 1000000;
+}
+
+uint64_t timeDiffUsec(const struct timeval &current, const struct timeval &previous) {
+	int64_t sec = static_cast<int64_t>(current.tv_sec) - static_cast<int64_t>(previous.tv_sec);
+	int64_t usec = static_cast<int64_t>(current.tv_usec) - static_cast<int64_t>(previous.tv_usec);
+
+	if (usec < 0) {
+		--sec;
+		usec += kMicrosecondsInSecond;
+	}
+
+	if (sec < 0) { return 0; }
+
+	return (static_cast<uint64_t>(sec) * static_cast<uint64_t>(kMicrosecondsInSecond)) +
+	       static_cast<uint64_t>(usec);
+}
+
 // Timer implementation
 
 Timer::Timer() : startTime_(now()) {

src/common/time_utils.h

@@ -26,6 +26,21 @@
 #include <cstdint>
 #include <ratio>
 
+struct timeval;
+
+/*!
+ * @brief Returns elapsed microseconds between two timeval snapshots.
+ *
+ * Handles microsecond borrow and clamps negative deltas to zero.
+ * Use this helper instead of raw unsigned subtraction to avoid wraparound
+ * when @p current.tv_usec is smaller than @p previous.tv_usec.
+ *
+ * @param current Newer timeval snapshot.
+ * @param previous Older timeval snapshot.
+ * @return Elapsed time in microseconds, clamped to zero when negative.
+ */
+uint64_t timeDiffUsec(const struct timeval &current, const struct timeval &previous);
+
 // SteadyClock is an alias for std::chrono::steady_clock or compatible class.
 // SteadyTimePoint is a matching time_point, SteadyDuration - matching duration.
 

src/common/time_utils_unittest.cc

@@ -21,6 +21,7 @@
 #include "common/platform.h"
 #include "common/time_utils.h"
 
+#include <sys/time.h>
 #include <unistd.h>
 #include <chrono>
 #include <thread>
@@ -90,3 +91,17 @@ TEST(TimeUtilsTests, InfiniteTimeout) {
 	EXPECT_EQ(timeout.remaining_ns(), -1);
 	EXPECT_EQ(timeout.remaining_s(), -1);
 }
+
+TEST(TimeUtilsTests, TimeDiffUsecHandlesMicrosecondBorrow) {
+	const timeval current{.tv_sec = 10, .tv_usec = 100};
+	const timeval previous{.tv_sec = 8, .tv_usec = 900000};
+
+	EXPECT_EQ(timeDiffUsec(current, previous), 1100100U);
+}
+
+TEST(TimeUtilsTests, TimeDiffUsecClampsNegativeDeltaToZero) {
+	const timeval current{.tv_sec = 5, .tv_usec = 0};
+	const timeval previous{.tv_sec = 6, .tv_usec = 0};
+
+	EXPECT_EQ(timeDiffUsec(current, previous), 0U);
+}

src/main/main.cc

@@ -132,13 +132,6 @@ static void signal_pipe_serv(const std::vector<pollfd> &pdesc) {
 			} else if (sigid=='\003') {
 				safs_pretty_syslog(LOG_NOTICE, "Received SIGUSR1, killing gently...");
 				exit(SAUNAFS_EXIT_STATUS_GENTLY_KILL);
-			} else if (sigid == '\004') {
-				safs_pretty_syslog(LOG_NOTICE, "Received SIGPROF signal");
-			} else if (sigid == '\005') {
-				safs_pretty_syslog(LOG_NOTICE, "Received SIGVTALRM signal");
-			} else if (sigid == '\006') {
-				// Disabling logging for SIGALRM to avoid bunch of logs triggered by
-				// SignalLoopWatchdog class
 			}
 		}
 	}
@@ -257,21 +250,6 @@ static int ignoresignal[]={
 	-1
 };
 
-static int timerSignalCodes[]={
-#ifndef GPERFTOOLS
-#ifdef SIGALRM
-	SIGALRM,
-#endif
-#ifdef SIGVTALRM
-	SIGVTALRM,
-#endif
-#ifdef SIGPROF
-	SIGPROF,
-#endif
-#endif
-	-1
-};
-
 static int exitsignal[]={
 #if defined(SIGUSR1) && defined(ENABLE_EXIT_ON_USR1)
 	SIGUSR1,
@@ -299,29 +277,6 @@ void exithandle(int signo) {
 	(void)signo;
 }
 
-void alarmSignalsHandler(int signo) {
-#ifndef GPERFTOOLS
-	ssize_t bytesWritten = 0;
-#ifdef SIGPROF
-	if (signo == SIGPROF) {
-		bytesWritten = write(signalpipe[1], "\004", 1);  // see above
-	}
-#endif
-#ifdef SIGVTALRM
-	if (signo == SIGVTALRM) {
-		bytesWritten = write(signalpipe[1], "\005", 1);  // see above
-	}
-#endif
-#ifdef SIGALRM
-	if (signo == SIGALRM) {
-		bytesWritten = write(signalpipe[1], "\006", 1);  // see above
-	}
-#endif
-	(void)bytesWritten;  // Prevent compiler warning
-#endif
-	(void)signo;  // Prevent compiler warning
-}
-
 void set_signal_handlers(int daemonflag) {
 	struct sigaction sa;
 	uint32_t i;
@@ -351,10 +306,6 @@ void set_signal_handlers(int daemonflag) {
 	for (i=0 ; ignoresignal[i]>0 ; i++) {
 		sigaction(ignoresignal[i],&sa,(struct sigaction *)0);
 	}
-	sa.sa_handler = alarmSignalsHandler;
-	for (i = 0; timerSignalCodes[i] > 0; i++) {
-		sigaction(timerSignalCodes[i], &sa, (struct sigaction *)0);
-	}
 	sa.sa_handler = daemonflag?SIG_IGN:termhandle;
 	for (i=0 ; daemonignoresignal[i]>0 ; i++) {
 		sigaction(daemonignoresignal[i],&sa,(struct sigaction *)0);