fix: spurious errors/warnings from a pc13v HA deploy (bug-1872..1877)
- bug-1872 (worker): "Failed to find any disk statistics in /proc/diskstats" ERROR every 15 s on an NVMe-only host. The whole-disk filter guessed from the name and skipped anything ending in a digit or containing 'p' -- every nvmeXnY, mmcblkN, even sdp -- so io was never reported. diskstats.h now counts a device only when /sys/block/<name>/device exists (real disks; not partitions, loop, dm, md, zram), and a host with no disk warns once. - bug-1873 (worker): "Failed to get worker pid" WARN on job start. MainPID reads 0 until systemd has forked the new transient unit; xcecd.c already retries, so the miss is DEBUG and the final failure names the unit. - bug-1874 (scheduler): deprecation WARN for the legacy xce_sched_fcfs on every start, though builtin_fcfs is the picker. Registration still wraps a job_elect_fn plugin but no longer warns; selecting one warns once. - bug-1875 (apiserver): "Not running as root -- PAM authentication will not work" with the default provider config_file; now only for provider pam. - bug-1876 (controller): standbys logged "cluster_init_phase2 ... failed (rc=-22)" WARN every 3 s while waiting for the primary's config push, a state the function itself calls normal. It returns -EAGAIN for that wait; callers log it at INFO, other failures still WARN. - bug-1877 (controller): "WAL/snapshot stored unencrypted" warned without looking. persist_path_encrypted() checks fscrypt (statx) and dm-crypt (sysfs dm/uuid CRYPT-, through LVM slaves); the warning stays when the storage is not shown to be encrypted (e.g. a plain /tmp). Tests: worker diskstats_test (2 of 4 RED on the old filter with pc13v's /proc/diskstats), controller test_phase2_cold_boot and test_persist_encryption, scheduler picker tests, apiserver TestRootWarning. Full ASAN build: all 113 suites pass (controller 319/0, harness 141/0). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8cadeeb2d0
commit
44173f74ae
23 changed files with 619 additions and 63 deletions
64
src/worker/include/diskstats.h
Normal file
64
src/worker/include/diskstats.h
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
#pragma once
|
||||
/*
|
||||
* diskstats.h -- sum the sector counters of the physical disks listed in
|
||||
* /proc/diskstats (node_information.c's io metric).
|
||||
*/
|
||||
#include <stdio.h>
|
||||
#include <string.h>
|
||||
#include <sys/stat.h>
|
||||
|
||||
/* A whole physical disk has /sys/block/<name>/device (a link to its
|
||||
* hardware). Partitions are not in /sys/block, and loop, ram, zram, dm-
|
||||
* and md devices have no `device` -- counting dm/md would also count the
|
||||
* same I/O twice, once more through their member disks. */
|
||||
static inline int diskstats_is_physical(const char* sysblock, const char* name)
|
||||
{
|
||||
char path[512];
|
||||
struct stat st;
|
||||
|
||||
if (strchr(name, '/') != NULL || name[0] == '.') {
|
||||
return 0;
|
||||
}
|
||||
snprintf(path, sizeof(path), "%s/%s/device", sysblock, name);
|
||||
return stat(path, &st) == 0;
|
||||
}
|
||||
|
||||
/* Sum sectors read/written over the physical disks in `fp` (a
|
||||
* /proc/diskstats stream). `sysblock` is the /sys/block directory.
|
||||
* Returns the number of disks counted.
|
||||
*
|
||||
* bug-1872: this used to guess from the name (skip names ending in a digit
|
||||
* or containing 'p'), which dropped every NVMe disk (nvme0n1), mmc disks
|
||||
* (mmcblk0) and even sdp. */
|
||||
static inline int diskstats_sum_physical(FILE* fp, const char* sysblock,
|
||||
unsigned long long* sectors_read,
|
||||
unsigned long long* sectors_written)
|
||||
{
|
||||
char line[512];
|
||||
int ndisks = 0;
|
||||
|
||||
*sectors_read = 0;
|
||||
*sectors_written = 0;
|
||||
|
||||
while (fgets(line, sizeof(line), fp)) {
|
||||
int major, minor;
|
||||
char name[64];
|
||||
unsigned long long rc, rm, rd, rt, wc, wm, wr, wt, inprog, iot, wiot;
|
||||
|
||||
if (sscanf(line,
|
||||
"%d %d %63s %llu %llu %llu %llu %llu %llu %llu %llu %llu "
|
||||
"%llu %llu",
|
||||
&major, &minor, name, &rc, &rm, &rd, &rt, &wc, &wm, &wr, &wt,
|
||||
&inprog, &iot, &wiot)
|
||||
< 14) {
|
||||
continue;
|
||||
}
|
||||
if (!diskstats_is_physical(sysblock, name)) {
|
||||
continue;
|
||||
}
|
||||
*sectors_read += rd;
|
||||
*sectors_written += wr;
|
||||
ndisks++;
|
||||
}
|
||||
return ndisks;
|
||||
}
|
||||
|
|
@ -1,4 +1,5 @@
|
|||
#include "node_information.h"
|
||||
#include "diskstats.h"
|
||||
#include "xcelog.h"
|
||||
#include <dirent.h>
|
||||
#include <errno.h>
|
||||
|
|
@ -888,7 +889,6 @@ static int read_disk_io_rate(double* io_rate)
|
|||
static time_t last_read_time = 0;
|
||||
|
||||
FILE* fp;
|
||||
char line[512];
|
||||
unsigned long long total_read_sectors = 0, total_write_sectors = 0;
|
||||
unsigned long long prev_total, current_total, sector_diff;
|
||||
time_t current_time = time(NULL);
|
||||
|
|
@ -906,50 +906,21 @@ static int read_disk_io_rate(double* io_rate)
|
|||
return -1;
|
||||
}
|
||||
|
||||
// Parse /proc/diskstats to accumulate I/O statistics for all physical disks
|
||||
while (fgets(line, sizeof(line), fp)) {
|
||||
int major, minor;
|
||||
char device_name[64];
|
||||
unsigned long long reads_completed, reads_merged, sectors_read,
|
||||
read_time;
|
||||
unsigned long long writes_completed, writes_merged, sectors_written,
|
||||
write_time;
|
||||
unsigned long long ios_in_progress, total_ios_time, weighted_ios_time;
|
||||
|
||||
// Parse diskstats line format
|
||||
int parsed = sscanf(
|
||||
line,
|
||||
"%d %d %s %llu %llu %llu %llu %llu %llu %llu %llu %llu %llu %llu",
|
||||
&major, &minor, device_name, &reads_completed, &reads_merged,
|
||||
§ors_read, &read_time, &writes_completed, &writes_merged,
|
||||
§ors_written, &write_time, &ios_in_progress, &total_ios_time,
|
||||
&weighted_ios_time);
|
||||
|
||||
if (parsed >= 14) {
|
||||
// Only count physical disks (skip partitions and virtual devices)
|
||||
// Physical disks typically have names like sda, sdb, hda, etc. (no
|
||||
// numbers at end) Skip loop devices, ram disks, and partitions
|
||||
if (strncmp(device_name, "loop", 4) == 0
|
||||
|| strncmp(device_name, "ram", 3) == 0
|
||||
|| strncmp(device_name, "dm-", 3) == 0
|
||||
|| strstr(device_name, "p") != NULL
|
||||
|| // nvme partitions (nvme0n1p1)
|
||||
(strlen(device_name) > 3
|
||||
&& device_name[strlen(device_name) - 1] >= '0'
|
||||
&& device_name[strlen(device_name) - 1] <= '9')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
total_read_sectors += sectors_read;
|
||||
total_write_sectors += sectors_written;
|
||||
found_disk = 1;
|
||||
}
|
||||
}
|
||||
/* bug-1872: physical disks are decided by /sys/block, not by name */
|
||||
found_disk = diskstats_sum_physical(fp, "/sys/block", &total_read_sectors,
|
||||
&total_write_sectors);
|
||||
fclose(fp);
|
||||
|
||||
if (!found_disk) {
|
||||
XCECD_LOG(XCE_LOG_ERROR,
|
||||
"Failed to find any disk statistics in /proc/diskstats");
|
||||
/* nothing to measure on this host; say so once, not every 15 s */
|
||||
static int warned = 0;
|
||||
|
||||
if (!warned) {
|
||||
XCECD_LOG(XCE_LOG_WARN,
|
||||
"No physical disk found in /proc/diskstats "
|
||||
"(/sys/block/<disk>/device); io will not be reported");
|
||||
warned = 1;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -156,7 +156,8 @@ void xcecd_process_command(void* arg)
|
|||
}
|
||||
|
||||
if (pid < 0) {
|
||||
XCECD_LOG(XCE_LOG_ERROR, "Failed to get worker pid");
|
||||
XCECD_LOG(XCE_LOG_ERROR, "Failed to get worker pid of %s after %d "
|
||||
"retries", unit_name, retry);
|
||||
|
||||
xcecd_job_start_error(ctx->xce_jobid);
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -136,7 +136,10 @@ pid_t dbus_get_worker_pid(const char* unit_name)
|
|||
|
||||
pid_t pid = get_systemd_job_pid(unit_name);
|
||||
if (pid <= 0) {
|
||||
XCECD_LOG(XCE_LOG_WARN, "Failed to get worker pid");
|
||||
/* bug-1873: MainPID reads 0 until systemd has forked the unit just
|
||||
* started, so a miss here is expected and the caller (xcecd.c)
|
||||
* retries; it logs the real failure once its retries run out. */
|
||||
XCECD_LOG(XCE_LOG_DEBUG, "worker pid of %s not known yet", unit_name);
|
||||
return -1;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -206,3 +206,16 @@ test('worker xcecd_tmpfile test', xcecd_tmpfile_test,
|
|||
suite: 'worker',
|
||||
timeout: 60,
|
||||
env: ['ASAN_OPTIONS=detect_leaks=0'])
|
||||
|
||||
# bug-1872: which /proc/diskstats lines are physical disks (diskstats.h);
|
||||
# NVMe and mmc disks were never counted.
|
||||
diskstats_test = executable('diskstats_test',
|
||||
'test_diskstats.c',
|
||||
include_directories: [worker_test_incdir],
|
||||
dependencies: worker_test_deps,
|
||||
install: false)
|
||||
|
||||
test('worker diskstats test', diskstats_test,
|
||||
suite: 'worker',
|
||||
timeout: 60,
|
||||
env: ['ASAN_OPTIONS=detect_leaks=0'])
|
||||
|
|
|
|||
139
src/worker/testsuite/test_diskstats.c
Normal file
139
src/worker/testsuite/test_diskstats.c
Normal file
|
|
@ -0,0 +1,139 @@
|
|||
/*
|
||||
* test_diskstats.c -- which /proc/diskstats lines count as physical disks
|
||||
* (diskstats.h). The old filter skipped any name ending in a digit or
|
||||
* containing 'p', so a host whose only disks are NVMe (nvme0n1) reported
|
||||
* no disk at all and logged an ERROR every 15 s.
|
||||
*/
|
||||
#include <errno.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <sys/stat.h>
|
||||
#include <unistd.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#include "diskstats.h"
|
||||
|
||||
static char sysblock[256];
|
||||
|
||||
static void make_disk(const char* name, int physical)
|
||||
{
|
||||
char path[512];
|
||||
|
||||
snprintf(path, sizeof(path), "%s/%s", sysblock, name);
|
||||
TEST_ASSERT_EQUAL_INT(0, mkdir(path, 0755));
|
||||
if (physical) {
|
||||
/* a real disk has a `device` link to its hardware; loop, dm-, md,
|
||||
* zram and ram devices have none, and partitions are not listed
|
||||
* in /sys/block at all */
|
||||
snprintf(path, sizeof(path), "%s/%s/device", sysblock, name);
|
||||
TEST_ASSERT_EQUAL_INT(0, mkdir(path, 0755));
|
||||
}
|
||||
}
|
||||
|
||||
void setUp(void)
|
||||
{
|
||||
snprintf(sysblock, sizeof(sysblock), "/tmp/test_diskstats.%d", (int)getpid());
|
||||
TEST_ASSERT_EQUAL_INT(0, mkdir(sysblock, 0755));
|
||||
}
|
||||
|
||||
void tearDown(void)
|
||||
{
|
||||
char cmd[300];
|
||||
|
||||
snprintf(cmd, sizeof(cmd), "rm -rf %s", sysblock);
|
||||
TEST_ASSERT_EQUAL_INT(0, system(cmd));
|
||||
}
|
||||
|
||||
static FILE* diskstats(const char* text)
|
||||
{
|
||||
FILE* fp = fmemopen((void*)text, strlen(text), "r");
|
||||
|
||||
TEST_ASSERT_NOT_NULL(fp);
|
||||
return fp;
|
||||
}
|
||||
|
||||
/* pc13v's /proc/diskstats (2026-09-28): one NVMe disk, its partitions,
|
||||
* and idle loop devices. */
|
||||
static const char PC13V[] =
|
||||
" 259 0 nvme0n1 152421 98489 7165957 15288 250739 83553 21142411 167803 0 37190 206665 0 0 0 0 190349 23574\n"
|
||||
" 259 1 nvme0n1p1 177 2028 14619 16 13 0 11 0 0 10 17 0 0 0 0 0 0\n"
|
||||
" 259 2 nvme0n1p2 152156 96461 7149330 15265 250723 83553 21142400 167802 0 43488 183067 0 0 0 0 0 0\n"
|
||||
" 7 0 loop0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0\n"
|
||||
" 7 1 loop1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0\n";
|
||||
|
||||
static void test_nvme_disk_is_counted_and_its_partitions_are_not(void)
|
||||
{
|
||||
unsigned long long rd = 0, wr = 0;
|
||||
FILE* fp;
|
||||
|
||||
make_disk("nvme0n1", 1);
|
||||
make_disk("loop0", 0);
|
||||
make_disk("loop1", 0);
|
||||
|
||||
fp = diskstats(PC13V);
|
||||
TEST_ASSERT_EQUAL_INT(1, diskstats_sum_physical(fp, sysblock, &rd, &wr));
|
||||
fclose(fp);
|
||||
TEST_ASSERT_EQUAL_UINT64(7165957ULL, rd);
|
||||
TEST_ASSERT_EQUAL_UINT64(21142411ULL, wr);
|
||||
}
|
||||
|
||||
static void test_sd_mmc_and_names_with_p_are_disks_virtual_ones_are_not(void)
|
||||
{
|
||||
static const char text[] =
|
||||
" 8 0 sda 1 0 10 0 1 0 100 0 0 0 0\n"
|
||||
" 8 1 sda1 1 0 5 0 1 0 50 0 0 0 0\n"
|
||||
" 8 240 sdp 1 0 20 0 1 0 200 0 0 0 0\n"
|
||||
" 179 0 mmcblk0 1 0 30 0 1 0 300 0 0 0 0\n"
|
||||
" 179 1 mmcblk0p1 1 0 15 0 1 0 150 0 0 0 0\n"
|
||||
" 253 0 dm-0 1 0 4000 0 1 0 4000 0 0 0 0\n"
|
||||
" 9 0 md0 1 0 5000 0 1 0 5000 0 0 0 0\n";
|
||||
unsigned long long rd = 0, wr = 0;
|
||||
FILE* fp;
|
||||
|
||||
make_disk("sda", 1);
|
||||
make_disk("sdp", 1);
|
||||
make_disk("mmcblk0", 1);
|
||||
make_disk("dm-0", 0);
|
||||
make_disk("md0", 0);
|
||||
|
||||
fp = diskstats(text);
|
||||
TEST_ASSERT_EQUAL_INT(3, diskstats_sum_physical(fp, sysblock, &rd, &wr));
|
||||
fclose(fp);
|
||||
TEST_ASSERT_EQUAL_UINT64(10ULL + 20 + 30, rd);
|
||||
TEST_ASSERT_EQUAL_UINT64(100ULL + 200 + 300, wr);
|
||||
}
|
||||
|
||||
static void test_no_physical_disk_returns_zero(void)
|
||||
{
|
||||
unsigned long long rd = 0, wr = 0;
|
||||
FILE* fp;
|
||||
|
||||
make_disk("loop0", 0);
|
||||
fp = diskstats(" 7 0 loop0 0 0 0 0 0 0 0 0 0 0 0\n");
|
||||
TEST_ASSERT_EQUAL_INT(0, diskstats_sum_physical(fp, sysblock, &rd, &wr));
|
||||
fclose(fp);
|
||||
}
|
||||
|
||||
static void test_short_or_garbage_lines_are_skipped(void)
|
||||
{
|
||||
unsigned long long rd = 0, wr = 0;
|
||||
FILE* fp;
|
||||
|
||||
make_disk("sda", 1);
|
||||
fp = diskstats("garbage\n 8 0 sda 1 2\n 8 0 sda 1 0 10 0 1 0 100 0 0 0 0\n");
|
||||
TEST_ASSERT_EQUAL_INT(1, diskstats_sum_physical(fp, sysblock, &rd, &wr));
|
||||
fclose(fp);
|
||||
TEST_ASSERT_EQUAL_UINT64(10ULL, rd);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
RUN_TEST(test_nvme_disk_is_counted_and_its_partitions_are_not);
|
||||
RUN_TEST(test_sd_mmc_and_names_with_p_are_disks_virtual_ones_are_not);
|
||||
RUN_TEST(test_no_physical_disk_returns_zero);
|
||||
RUN_TEST(test_short_or_garbage_lines_are_skipped);
|
||||
return UNITY_END();
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue