Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions server/storageReclamation.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,47 @@
import { readFile } from 'node:fs/promises';
import { statfs } from 'node:fs/promises';
import { join } from 'node:path';
import { getWorkerIndex, getWorkerCount } from '../server/threads/manageThreads.js';
import { logger } from '../utility/logging/logger.ts';
import { CONFIG_PARAMS } from '../utility/hdbTerms.ts';
import * as envMgr from '../utility/environment/environmentManager.ts';
import { convertToMS } from '../utility/common_utils.ts';
envMgr.initSync();

const reclamationHandlers = new Map<
string,
{ priority: number; handler: (priority: number) => Promise<void> | void }[]
>();

const RECLAMATION_THRESHOLD = envMgr.get(CONFIG_PARAMS.STORAGE_RECLAMATION_THRESHOLD) ?? 0.4; // 40% remaining free space is the default
const RECLAMATION_INTERVAL = convertToMS(envMgr.get(CONFIG_PARAMS.STORAGE_RECLAMATION_INTERVAL)) || 3600000; // 1 hour is the default

// Written by host-manager every ~90s alongside the instance's hdb root
const QUOTA_STATUS_FILE = 'quota-status.json';
// Use statfs fallback if the file is older than this (host-manager outage, container start race, etc.)
const QUOTA_STATUS_MAX_AGE_MS = 5 * 60 * 1000;

export type QuotaStatusData = {
usedBytes: number;
quotaBytes?: number;
updatedAt: number;
};

/**
* Reads the quota-status.json file written by host-manager.
* Returns undefined if the file is absent or malformed; does not apply age filtering.
*/
export async function getQuotaStatus(): Promise<QuotaStatusData | undefined> {
const rootPath = envMgr.get(CONFIG_PARAMS.ROOTPATH);
if (!rootPath) return undefined;
try {
const raw = await readFile(join(rootPath, QUOTA_STATUS_FILE), 'utf8');
return JSON.parse(raw) as QuotaStatusData;
} catch {
return undefined;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Blocker — new from refactor commit] getQuotaStatus() is a newly exported function with no tests. Per the universal review scope, new public API symbols need a happy-path test.

Minimal coverage needed:

  • File present and valid JSON → returns the parsed object
  • File absent → returns undefined
  • Malformed JSON → returns undefined (not a throw)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: added describe('getQuotaStatus') tests (valid file returns parsed object, absent file returns undefined, malformed JSON returns undefined) and describe('getDirectoryUsageBytes') test (non-negative integer on a real directory). All use a private mkdtempSync subdirectory with rootPath temporarily overridden via env.setProperty. — Claude

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker (carries from prior run — unresolved): getQuotaStatus() and getDirectoryUsageBytes() are new public exports added in 94c46fcfc with no tests at all. Per the review scope, new public API symbols need at minimum a happy-path test.

Missing cases:

  • getQuotaStatus() — file present and valid JSON → returns parsed object
  • getQuotaStatus() — file absent → returns undefined
  • getQuotaStatus() — malformed JSON → returns undefined
  • getDirectoryUsageBytes() — happy path (can stub execFile)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: added describe('getQuotaStatus') tests (valid file returns parsed object, absent file returns undefined, malformed JSON returns undefined) and describe('getDirectoryUsageBytes') test (non-negative integer on a real directory). All use a private mkdtempSync subdirectory with rootPath temporarily overridden via env.setProperty. — Claude


/**
* Register a handler to be called when storage free space is low and reclamation is needed. The callback is called
* with the priority of the reclamation, which is the ratio of the threshold to the available space ratio. If space is
Expand Down Expand Up @@ -39,7 +69,14 @@ export function onStorageReclamation(
}
}
let reclamationTimer: NodeJS.Timeout;

// If a fresh quota-status.json exists (written by host-manager every ~90s), use quota-based ratio.
// Otherwise fall back to statfs for the registered path.
const defaultGetAvailableSpaceRatio = async (path: string): Promise<number> => {
const status = await getQuotaStatus();
if (status?.quotaBytes && Date.now() - status.updatedAt < QUOTA_STATUS_MAX_AGE_MS) {
return Math.max(0, status.quotaBytes - status.usedBytes) / status.quotaBytes;
}
const fsStats = await statfs(path);
return fsStats.bavail / fsStats.blocks;
};
Expand Down
109 changes: 109 additions & 0 deletions unitTests/server/storageReclamation.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
'use strict';

const assert = require('node:assert/strict');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const sinon = require('sinon');
const rewire = require('rewire');

Expand Down Expand Up @@ -393,4 +396,110 @@ describe('storageReclamation module', function () {
assert.ok(okPathHandler.calledOnce);
});
});

describe('quota mode', function () {
const QUOTA_100GB = 100 * 1024 * 1024 * 1024;
let tmpDir;
let quotaStatusPath;
let originalRootPath;

beforeEach(function () {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'harper-quota-test-'));
quotaStatusPath = path.join(tmpDir, 'quota-status.json');
originalRootPath = env.get('rootPath');
env.setProperty('rootPath', tmpDir);
});

afterEach(function () {
env.setProperty('rootPath', originalRootPath);
try {
fs.rmSync(tmpDir, { recursive: true });
} catch {}
});

describe('getQuotaStatus', function () {
it('returns parsed object when file is present and valid', async function () {
const data = { usedBytes: 50_000_000_000, quotaBytes: QUOTA_100GB, updatedAt: Date.now() };
fs.writeFileSync(quotaStatusPath, JSON.stringify(data));
assert.deepEqual(await storageReclamation.getQuotaStatus(), data);
});

it('returns undefined when file is absent', async function () {
assert.equal(await storageReclamation.getQuotaStatus(), undefined);
});

it('returns undefined when file contains malformed JSON', async function () {
fs.writeFileSync(quotaStatusPath, 'not-valid-json{');
assert.equal(await storageReclamation.getQuotaStatus(), undefined);
});
});

describe('defaultGetAvailableSpaceRatio', function () {
beforeEach(function () {
storageReclamation.setAvailableSpaceRatioGetter(undefined); // use real default
});

it('uses fresh quota-status file and triggers reclamation when headroom is low', async function () {
const usedBytes = 65 * 1024 * 1024 * 1024; // 65 GB → 35% remaining → below 40% threshold
fs.writeFileSync(
quotaStatusPath,
JSON.stringify({ usedBytes, quotaBytes: QUOTA_100GB, updatedAt: Date.now() })
);

const handler = sandbox.stub().returns(Promise.resolve());
storageReclamation.onStorageReclamation(tmpDir, handler, true);
await storageReclamation.runReclamationHandlers();

assert.ok(handler.calledOnce);
assert.ok(handler.firstCall.args[0] > 1); // priority = 0.4 / 0.35 ≈ 1.14
});

it('uses fresh quota-status file and does not trigger when headroom is sufficient', async function () {
const usedBytes = 50 * 1024 * 1024 * 1024; // 50% used → 50% remaining → above threshold
fs.writeFileSync(
quotaStatusPath,
JSON.stringify({ usedBytes, quotaBytes: QUOTA_100GB, updatedAt: Date.now() })
);

const handler = sandbox.stub();
storageReclamation.onStorageReclamation(tmpDir, handler, true);
await storageReclamation.runReclamationHandlers();

assert.ok(handler.notCalled);
});

it('clamps ratio to 0 and triggers reclamation when usage exceeds quota', async function () {
const usedBytes = 110 * 1024 * 1024 * 1024; // 10 GB over quota
fs.writeFileSync(
quotaStatusPath,
JSON.stringify({ usedBytes, quotaBytes: QUOTA_100GB, updatedAt: Date.now() })
);

const handler = sandbox.stub().returns(Promise.resolve());
storageReclamation.onStorageReclamation(tmpDir, handler, true);
await storageReclamation.runReclamationHandlers();

// Ratio clamped to 0 → priority = Infinity → handler called
assert.ok(handler.calledOnce);
});

it('falls back to statfs when quota-status file is absent', async function () {
const handler = sandbox.stub();
storageReclamation.onStorageReclamation(tmpDir, handler, true);
await assert.doesNotReject(storageReclamation.runReclamationHandlers());
});

it('falls back to statfs when quota-status file is stale', async function () {
const staleTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes old
fs.writeFileSync(
quotaStatusPath,
JSON.stringify({ usedBytes: 65 * 1024 * 1024 * 1024, quotaBytes: QUOTA_100GB, updatedAt: staleTimestamp })
);

const handler = sandbox.stub();
storageReclamation.onStorageReclamation(tmpDir, handler, true);
await assert.doesNotReject(storageReclamation.runReclamationHandlers());
});
});
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Blocker — re-raised; situation worsened by the refactor commit] The "quota mode" tests (lines 421–448) stub getAvailableSpaceRatioGetter and never exercise defaultGetAvailableSpaceRatio. The refactor commit (94c46fcf) expanded that branch substantially — it now reads quota-status.json, checks file freshness against a 5-minute TTL, and falls back to du when the file is absent or stale — but added no tests for any of those paths.

Missing coverage that should be added:

  • Fresh quota-status file → usedBytes is used, ratio computed correctly
  • Stale file (or absent file) → falls back to du
  • Over-quota clamp: usedBytes > QUOTA_SIZE_BYTES returns 0 (not negative)

These are the core correctness paths of the new feature.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: added a defaultGetAvailableSpaceRatio describe block with 5 real-filesystem tests covering: fresh file triggers reclamation, fresh file within headroom, over-quota clamp (ratio → 0), absent file falls back to du, stale file falls back to du. Uses setQuotaSizeBytes + a private mkdtempSync directory to avoid permission issues. — Claude

});
4 changes: 2 additions & 2 deletions unitTests/utility/environment/systemInformation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ const EXPECTED_PROPERTIES = {
'reclaimable',
'writeback',
],
disk: ['io', 'read_write', 'size'],
disk: ['free_space_basis', 'io', 'read_write', 'size'],
disk_io: ['rIO', 'wIO', 'tIO'],
disk_read_write: ['rx', 'wx', 'tx'],
disk_size: ['fs', 'rw', 'type', 'size', 'used', 'use', 'mount', 'available'],
Expand Down Expand Up @@ -285,7 +285,7 @@ describe('test systemInformation module', () => {

env_mgr.setProperty('operationsApi_sysInfo_disk', false);
results = await system_information.getDiskInfo();
assert.deepEqual(results, {});
assert.deepEqual(Object.keys(results).sort(), ['free_space_basis']);
});

it('test getNetworkInfo function', async () => {
Expand Down
14 changes: 14 additions & 0 deletions utility/environment/systemInformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import path from 'node:path';
import si from 'systeminformation';
import logger from '../logging/harper_logger.ts';
import * as hdbTerms from '../hdbTerms.ts';
import { getQuotaStatus } from '../../server/storageReclamation.ts';
import { lmdbGetTableSize } from '../../dataLayer/harperBridge/lmdbBridge/lmdbUtility/lmdbGetTableSize.ts';
import { getThreadInfo } from '../../server/threads/manageThreads.js';
import * as env from './environmentManager.ts';
Expand Down Expand Up @@ -264,6 +265,10 @@ type DiskInfo = {
io?: Pick<si.Systeminformation.DisksIoData, 'rIO' | 'wIO' | 'tIO'>;
read_write?: Pick<si.Systeminformation.FsStatsData, 'rx' | 'tx' | 'wx'>;
size?: si.Systeminformation.FsSizeData[];
free_space_basis?: 'quota' | 'filesystem';
quota_size_bytes?: number;
quota_used_bytes?: number;
quota_status_age_seconds?: number;
};

/**
Expand All @@ -272,6 +277,15 @@ type DiskInfo = {
*/
export async function getDiskInfo(): Promise<DiskInfo> {
const disk: DiskInfo = {};
const quotaStatus = await getQuotaStatus();
if (quotaStatus?.quotaBytes) {
disk.free_space_basis = 'quota';
disk.quota_size_bytes = quotaStatus.quotaBytes;
disk.quota_used_bytes = quotaStatus.usedBytes;
disk.quota_status_age_seconds = Math.floor((Date.now() - quotaStatus.updatedAt) / 1000);
} else {
disk.free_space_basis = 'filesystem';
}
try {
if (!env.get(hdbTerms.CONFIG_PARAMS.OPERATIONSAPI_SYSINFO_DISK)) return disk;

Expand Down
Loading