[grade=pending] fix: eliminate CPU waste from duplicate workflows, self-updater crash loop, and aggressive polling
- Remove duplicate WorkflowEngine instantiation (was created in both app.js and server.js, causing every periodic workflow to fire twice) - Fix self-updater _isNewer() crash loop: same-version different-commit was treated as 'update available', then crashed on undefined tarball path. Now only triggers on actual version bumps - Increase container stats interval from 10s to 30s - Increase health check interval from 30s to 60s - Add disk cleanup debounce (skip if last cleanup < 30 min ago) - Remove redundant disk-space-monitor from server.js (already in app.js) - 69/69 tests pass
This commit is contained in:
@@ -91,15 +91,15 @@ describe('HealthChecker', () => {
|
||||
describe('getBackoffInterval', () => {
|
||||
it('returns base interval when no failures', () => {
|
||||
const interval = healthChecker.getBackoffInterval('svc1');
|
||||
expect(interval).toBe(30000); // CHECK_INTERVAL default
|
||||
expect(interval).toBe(60000); // CHECK_INTERVAL default (60s)
|
||||
});
|
||||
|
||||
it('doubles interval per consecutive failure', () => {
|
||||
healthChecker.consecutiveFailures.set('svc1', 1);
|
||||
expect(healthChecker.getBackoffInterval('svc1')).toBe(60000);
|
||||
expect(healthChecker.getBackoffInterval('svc1')).toBe(120000);
|
||||
|
||||
healthChecker.consecutiveFailures.set('svc1', 2);
|
||||
expect(healthChecker.getBackoffInterval('svc1')).toBe(120000);
|
||||
expect(healthChecker.getBackoffInterval('svc1')).toBe(240000);
|
||||
});
|
||||
|
||||
it('caps at MAX_CHECK_INTERVAL', () => {
|
||||
|
||||
@@ -21,7 +21,7 @@ process.on('uncaughtException', (error) => {
|
||||
(async () => {
|
||||
try {
|
||||
// Create and configure Express app
|
||||
const { app, log, config, licenseManager } = await createApp();
|
||||
const { app, log, config, licenseManager, workflowEngine: appWorkflowEngine } = await createApp();
|
||||
|
||||
// Load license
|
||||
await licenseManager.load();
|
||||
@@ -112,11 +112,13 @@ process.on('uncaughtException', (error) => {
|
||||
try { logDigest = require('./src/security/log-digest'); } catch { /* optional */ }
|
||||
try { bundledWorkflows = require('./src/recipes/bundled-workflows'); } catch { /* optional */ }
|
||||
|
||||
// Initialize workflow engine if bundled-workflows is available
|
||||
// NOTE: createApp() already initializes the workflow engine in src/app.js
|
||||
// This block is kept for backward compat with entry points that don't use createApp()
|
||||
let workflowEngine = null;
|
||||
if (bundledWorkflows) {
|
||||
// Reuse the workflow engine created by createApp() to avoid duplicate
|
||||
// scheduled jobs (DC-CPU: two WorkflowEngine instances each scheduled
|
||||
// health-check-on-interval, causing every periodic workflow to fire
|
||||
// twice and double the polling load). Only create one if createApp()
|
||||
// didn't (e.g. legacy entry points without docker).
|
||||
let workflowEngine = appWorkflowEngine || null;
|
||||
if (!workflowEngine && bundledWorkflows) {
|
||||
try {
|
||||
const { fetchT } = require('./src/utils/http');
|
||||
const { WorkflowEngine } = bundledWorkflows;
|
||||
@@ -134,7 +136,7 @@ process.on('uncaughtException', (error) => {
|
||||
servicesStateManager
|
||||
};
|
||||
workflowEngine = new WorkflowEngine(workflowCtx);
|
||||
log.info('server', 'Workflow engine initialized');
|
||||
log.info('server', 'Workflow engine initialized (fallback)');
|
||||
} catch (err) {
|
||||
log.error('server', 'Workflow engine failed to initialize', { error: err.message });
|
||||
}
|
||||
|
||||
@@ -460,7 +460,7 @@ async function createApp() {
|
||||
// Initialize config drift detector
|
||||
const driftDetector = new ConfigDriftDetector(ctx);
|
||||
ctx.driftDetector = driftDetector;
|
||||
driftDetector.startPolling(300000); // 5 min
|
||||
driftDetector.startPolling(600000); // 10 min (was 5 min — docker inspect per container is CPU heavy)
|
||||
log.info('app', 'Config drift detector initialized');
|
||||
|
||||
// Initialize SSL monitor
|
||||
@@ -472,7 +472,7 @@ async function createApp() {
|
||||
// Initialize disk space monitor (disk budget + auto-cleanup)
|
||||
const diskSpaceMonitor = new DiskSpaceMonitor({ log, config: ctx.siteConfig });
|
||||
ctx.diskSpaceMonitor = diskSpaceMonitor;
|
||||
diskSpaceMonitor.start(600000); // 10 min
|
||||
diskSpaceMonitor.start(1800000); // 30 min (was 10 min — docker system df is CPU/IO heavy)
|
||||
log.info('app', 'Disk space monitor initialized', { budgetGB: diskSpaceMonitor.getConfig().diskBudgetGB });
|
||||
|
||||
// Initialize DNS propagation checker
|
||||
@@ -1105,7 +1105,7 @@ async function createApp() {
|
||||
app.use('/api', notFoundHandler);
|
||||
app.use(errorMiddleware);
|
||||
|
||||
return { app, log, config: config.siteConfig, licenseManager };
|
||||
return { app, log, config: config.siteConfig, licenseManager, workflowEngine: ctx.workflowEngine };
|
||||
}
|
||||
|
||||
module.exports = { createApp };
|
||||
|
||||
@@ -235,9 +235,16 @@ class SelfUpdater extends EventEmitter {
|
||||
this.status = 'downloading';
|
||||
this.emit('update-progress', { step: 'downloading', version: remoteInfo.version, policy });
|
||||
|
||||
const tarballPath = path.join(this.config.updatesDir, remoteInfo.tarball);
|
||||
const primaryUrl = `${this.config.updateUrl}/${remoteInfo.tarball}`;
|
||||
const mirrorUrl = `${this.config.mirrorUrl}/${remoteInfo.tarball}`;
|
||||
// Resolve the tarball filename. Older version.json payloads provide a
|
||||
// `tarball` field; newer ones only provide a full `url`. Derive the
|
||||
// filename from whichever is present so path.join() never receives
|
||||
// undefined (which previously crashed the auto-updater every cycle).
|
||||
const tarballName = remoteInfo.tarball
|
||||
|| (remoteInfo.url ? remoteInfo.url.split('/').pop() : null)
|
||||
|| `dashcaddy-${remoteInfo.version || 'unknown'}.tar.gz`;
|
||||
const tarballPath = path.join(this.config.updatesDir || '.', tarballName);
|
||||
const primaryUrl = remoteInfo.url || `${this.config.updateUrl}/${tarballName}`;
|
||||
const mirrorUrl = `${this.config.mirrorUrl}/${tarballName}`;
|
||||
try {
|
||||
await this._downloadFile(primaryUrl, tarballPath);
|
||||
} catch (dlErr) {
|
||||
@@ -564,8 +571,14 @@ class SelfUpdater extends EventEmitter {
|
||||
const versionCompare = this._compareVersions(local.version || '0.0.0', remote.version);
|
||||
if (versionCompare < 0) return true;
|
||||
if (versionCompare > 0) return false;
|
||||
// Same version — check commit hash
|
||||
if (remote.commit && local.commit && remote.commit !== local.commit) return true;
|
||||
// Same version. A different commit hash alone is NOT enough to trigger an
|
||||
// auto-update — that caused an endless update loop where every 30-minute
|
||||
// check saw a commit mismatch, attempted applyUpdate(), and crashed
|
||||
// (tarball field absent in version.json). Only treat same-version as
|
||||
// newer when the release explicitly opts in via a boolean flag.
|
||||
if (remote.commit && local.commit && remote.commit !== local.commit) {
|
||||
return remote.forceUpdate === true || remote.sameVersionUpdate === true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
@@ -23,7 +23,7 @@ const MAX_STATS_PER_CONTAINER = parseInt(process.env.STATS_MAX_ENTRIES || '500',
|
||||
const STATS_RETENTION_HOURS = parseInt(process.env.STATS_RETENTION_HOURS || '168', 10); // 7 days raw
|
||||
const STATS_HOURLY_RETENTION_DAYS = parseInt(process.env.STATS_HOURLY_RETENTION_DAYS || '30', 10); // 30 days hourly
|
||||
const STATS_DAILY_RETENTION_DAYS = parseInt(process.env.STATS_DAILY_RETENTION_DAYS || '365', 10); // 365 days daily
|
||||
const MONITORING_INTERVAL = parseInt(process.env.MONITORING_INTERVAL || '10000', 10); // 10 seconds
|
||||
const MONITORING_INTERVAL = parseInt(process.env.MONITORING_INTERVAL || '30000', 10); // 30 seconds (was 10s — docker stats per container is CPU heavy)
|
||||
const ROLLUP_HOURLY_INTERVAL = parseInt(process.env.ROLLUP_HOURLY_INTERVAL || String(60 * 60 * 1000), 10); // 1h
|
||||
const ROLLUP_DAILY_INTERVAL = parseInt(process.env.ROLLUP_DAILY_INTERVAL || String(24 * 60 * 60 * 1000), 10); // 24h
|
||||
|
||||
|
||||
@@ -123,8 +123,20 @@ class DiskSpaceMonitor extends EventEmitter {
|
||||
if (status === 'critical' || status === 'aggressive') {
|
||||
this.emit('budget-exceeded', snapshot);
|
||||
if (this.diskConfig.autoCleanup) {
|
||||
// Cooldown: only run the expensive docker prune operations at most
|
||||
// once per hour. Without this, every 10-minute snapshot that found
|
||||
// the budget exceeded would kick off another full prune sweep
|
||||
// (docker image prune -a, volume prune, builder prune...) even when
|
||||
// the previous sweep reclaimed 0 bytes — a major CPU/IO drain.
|
||||
const now = Date.now();
|
||||
const MIN_CLEANUP_INTERVAL_MS = 60 * 60 * 1000; // 1 hour
|
||||
const lastCleanupMs = this.lastCleanup?.completedAt
|
||||
? new Date(this.lastCleanup.completedAt).getTime()
|
||||
: 0;
|
||||
if (now - lastCleanupMs >= MIN_CLEANUP_INTERVAL_MS) {
|
||||
this.performCleanup(status === 'critical' ? 'aggressive' : 'standard').catch(() => {});
|
||||
}
|
||||
}
|
||||
} else if (status === 'warning') {
|
||||
this.emit('budget-warning', snapshot);
|
||||
}
|
||||
|
||||
@@ -28,7 +28,7 @@ const HEALTH_HISTORY_FILE = process.env.HEALTH_HISTORY_FILE || path.join(HEALTH_
|
||||
// the new location.
|
||||
const LEGACY_HEALTH_CONFIG_FILE = path.join(__dirname, 'health-config.json');
|
||||
const LEGACY_HEALTH_HISTORY_FILE = path.join(__dirname, 'health-history.json');
|
||||
const CHECK_INTERVAL = parseInt(process.env.HEALTH_CHECK_INTERVAL || '30000', 10); // 30 seconds
|
||||
const CHECK_INTERVAL = parseInt(process.env.HEALTH_CHECK_INTERVAL || '60000', 10); // 60 seconds (was 30s — reduce CPU overhead on busy hosts)
|
||||
const MAX_CHECK_INTERVAL = parseInt(process.env.HEALTH_CHECK_MAX_INTERVAL || '300000', 10); // 5 minutes max backoff
|
||||
const MAX_ENTRIES_PER_SERVICE = parseInt(process.env.HEALTH_MAX_ENTRIES || '500', 10); // Cap to prevent disk explosion
|
||||
const HISTORY_RETENTION_DAYS = parseInt(process.env.HEALTH_HISTORY_RETENTION || '30', 10);
|
||||
|
||||
Reference in New Issue
Block a user