From f2d4d2fae18bb69411bcafd07ee10b5faa939682 Mon Sep 17 00:00:00 2001 From: Gmer4Lfe Date: Sun, 9 Aug 2026 19:46:17 -0400 Subject: [PATCH] Stop reparsing both confs, and forking, on every conf lookup vv_conf_vars() read and regex-parsed ~1,900 lines per call and reached a shell fork through vv_detect_host(); the repair sweep asks it for a key per log line. Keyed on a hash of the contents rather than mtime and size, which missed a same-second rewrite to the same length. --- Plugin/unraid/include/config.php | 87 ++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 9 deletions(-) diff --git a/Plugin/unraid/include/config.php b/Plugin/unraid/include/config.php index 9316a45..b343bcb 100644 --- a/Plugin/unraid/include/config.php +++ b/Plugin/unraid/include/config.php @@ -300,8 +300,13 @@ function vv_push_master_conf(): array { return $results; } -function vv_get_hostname(): string { - return trim(shell_exec('hostname -s') ?: ''); +// Cached: this forks a shell, and it is reached from vv_conf_vars() by way of vv_detect_host(), +// which the repair sweep calls per log line. A machine does not rename itself mid-request. +function vv_get_hostname(bool $flush = false): string { + static $name = null; + if ($flush) { $name = null; return ''; } + if ($name === null) $name = trim(shell_exec('hostname -s') ?: ''); + return $name; } // Mirror of common.sh resolve_tailscale_ip(): tries `tailscale ip -4` first (Tailscale manages @@ -328,7 +333,19 @@ function vv_resolve_tailscale_ip(string $hostname): string { return count($matches) === 1 ? $matches[0] : ''; } -function vv_detect_host(): string { +// Cached alongside the others: this reads master.conf in full and is called by vv_conf_vars() on +// every lookup, so leaving it uncached would mean the conf is still read once per key even with +// the parsed values cached. +function vv_detect_host(bool $flush = false): string { + static $host = null; + if ($flush) { $host = null; return ''; } + if ($host !== null) return $host; + + $host = _vv_detect_host_uncached(); + return $host; +} + +function _vv_detect_host_uncached(): string { // Reads master.conf for HOST1="name" (or HOST1_NAME="name") and matches running hostname. // Returns 'host1', 'host2', 'host3', ... or 'unknown'. Works for any number of hosts. $master = vv_read_conf_raw('master.conf'); @@ -397,7 +414,13 @@ function vv_write_conf_raw(string $filename, string $content): bool { if ($content !== '' && !str_ends_with($content, "\n")) $content .= "\n"; if (file_put_contents($tmp, $content) === false) return false; - return rename($tmp, $path); + if (!rename($tmp, $path)) return false; + + // Every conf write in the plugin lands here, so this is the one place the parsed-conf cache + // has to be dropped. Doing it in the callers instead would mean a new writer inheriting a + // stale cache and no obvious reason why. + vv_conf_vars_flush(); + return true; } function vv_get_conf_files(): array { @@ -420,15 +443,59 @@ function vv_get_conf_files(): array { return $files; } -// Parse conf into key=>value map for $VAR substitution in docs -function vv_conf_vars(): array { - $vars = []; +// Drops the parsed-conf cache. Called by vv_write_conf_raw() — the single point at which a conf +// changes on disk — so no writer has to remember to do it. +// +// Necessary on top of the mtime check below, not instead of it: filesystem mtimes have one-second +// resolution, and a write followed by a read inside the same second is exactly what the conf +// writer does when it verifies a value it just wrote. Without this, that read could be served the +// value from before the write and the verification would compare a value against itself. +function vv_conf_vars_flush(): void { + vv_conf_vars(true); + // Host identity is derived from master.conf too, so a write that changes HOST1 has to + // invalidate it as well — first-run setup does exactly that, then asks which host this is. + vv_detect_host(true); + vv_get_hostname(true); +} + +// Parse conf into key=>value map for $VAR substitution in docs. +// +// Cached, because this is not the cheap function its callers assume. It reads and regex-parses +// both conf files on every call — around 1,900 lines — and the repair sweep asks it for a key per +// log line. The measured cost of exactly this pattern is on record: the 30 minutes play_state_sync +// spent in 2026-07 was parse and fork overhead, not the API it was blamed on. +// +// Keyed on a hash of the contents, so an edit made outside this process — the raw editor in +// another tab, conf_upgrade from bash, a hand edit over SSH — is still picked up. +// +// Content rather than mtime and size, which was the first attempt and was wrong. Both conf files +// were rewritten within the same second and to the same byte count — "true" and "false" trading +// places across two keys — and the stale values were served straight back. mtime has one-second +// resolution and a same-size edit is not a rare shape in a file of booleans. +// +// Reading both files every call is not what costs anything here; parsing them is. The read is +// tens of microseconds against a parse of ~1,900 lines followed by two resolution passes. +function vv_conf_vars(bool $flush = false): array { + static $cache = null; + static $stamp = null; + + if ($flush) { $cache = null; $stamp = null; return []; } + $files = ['master.conf']; $host = vv_detect_host(); if (preg_match('/^host\d+$/', $host)) $files[] = $host . '.conf'; + $raws = []; + $sig = ''; foreach ($files as $f) { - $raw = vv_read_conf_raw($f); + $raws[$f] = vv_read_conf_raw($f); + $sig .= md5($raws[$f]); + } + if ($cache !== null && $stamp === $sig) return $cache; + + $vars = []; + foreach ($files as $f) { + $raw = $raws[$f]; // Match: VAR_NAME="value" or VAR_NAME=value (no quotes) preg_match_all('/^\s*([A-Z0-9_]+)\s*=\s*["\']?([^"\'#\n]*?)["\']?\s*(?:#.*)?$/m', $raw, $m); foreach ($m[1] as $i => $key) { @@ -449,7 +516,9 @@ function vv_conf_vars(): array { } } unset($v); - return $vars; + + $stamp = $sig; + return $cache = $vars; } // Query the Unraid GraphQL API for a given host.