Skip to content
Merged
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
31 changes: 12 additions & 19 deletions resources/databases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { replayLogs } from './replayLogs.ts';
import { totalmem } from 'node:os';
import { RocksIndexStore } from './RocksIndexStore.ts';
import { when } from '../utility/when.ts';
import { resolveRocksMemoryConfig } from '../utility/rocksMemoryConfig.ts';
import { isProcessRunning } from '../utility/processManagement/processManagement.js';

/**
Expand Down Expand Up @@ -158,30 +159,22 @@ function openRocksDatabase(path: string, options: RocksDatabaseOptions & { dupSo
}
// Read RocksDB memory config lazily so env/CLI overrides applied after module load are
// respected. The block cache falls back to 25% of constrained (cgroup) memory when not
// configured; the WriteBufferManager is opt-in (0 disables).
//
// We enforce types rather than coerce — values from YAML config and env vars flow
// through configUtils.castConfigValue which produces proper numbers/booleans/null,
// so anything else is misconfiguration and should fall through to the default.
// configured; the WriteBufferManager defaults to 1/3 of the block cache size (set its size
// to 0 to disable). See resolveRocksMemoryConfig for the defaulting rules.
//
// Note: writeBufferManagerCostToCache and writeBufferManagerAllowStall are fixed at WBM
// creation time inside rocksdb-js (the underlying RocksDB API doesn't support changing
// costToCache on a live manager, and allowStall is only re-applied when explicitly changed).
// In practice that's fine — these come from process-level config that doesn't change.
const configuredBlockCacheSize = envGet(CONFIG_PARAMS.STORAGE_ROCKS_BLOCKCACHESIZE);
const blockCacheSize =
typeof configuredBlockCacheSize === 'number' && configuredBlockCacheSize > 0
? configuredBlockCacheSize
: Math.min(process.constrainedMemory?.() ?? Infinity, totalmem()) * 0.25;
const writeBufferManagerSize = envGet(CONFIG_PARAMS.STORAGE_ROCKS_WRITEBUFFERMANAGERSIZE);
const writeBufferManagerCostToCache = envGet(CONFIG_PARAMS.STORAGE_ROCKS_WRITEBUFFERMANAGERCOSTTOCACHE);
const writeBufferManagerAllowStall = envGet(CONFIG_PARAMS.STORAGE_ROCKS_WRITEBUFFERMANAGERALLOWSTALL);
RocksDatabase.config({
blockCacheSize,
...(typeof writeBufferManagerSize === 'number' && writeBufferManagerSize > 0 ? { writeBufferManagerSize } : {}),
...(typeof writeBufferManagerCostToCache === 'boolean' ? { writeBufferManagerCostToCache } : {}),
...(typeof writeBufferManagerAllowStall === 'boolean' ? { writeBufferManagerAllowStall } : {}),
});
RocksDatabase.config(
resolveRocksMemoryConfig({
configuredBlockCacheSize: envGet(CONFIG_PARAMS.STORAGE_ROCKS_BLOCKCACHESIZE),
configuredWriteBufferManagerSize: envGet(CONFIG_PARAMS.STORAGE_ROCKS_WRITEBUFFERMANAGERSIZE),
configuredCostToCache: envGet(CONFIG_PARAMS.STORAGE_ROCKS_WRITEBUFFERMANAGERCOSTTOCACHE),
configuredAllowStall: envGet(CONFIG_PARAMS.STORAGE_ROCKS_WRITEBUFFERMANAGERALLOWSTALL),
availableMemory: Math.min(process.constrainedMemory?.() ?? Infinity, totalmem()),
})
);
if (!existsSync(path)) {
// Don't create directories in read-only mode
if (isReadOnlyMode()) {
Expand Down
108 changes: 108 additions & 0 deletions unitTests/utility/rocksMemoryConfig.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
'use strict';

const assert = require('node:assert/strict');
const { resolveRocksMemoryConfig } = require('#src/utility/rocksMemoryConfig');

const GB = 1024 * 1024 * 1024;

function resolve(overrides) {
return resolveRocksMemoryConfig({
configuredBlockCacheSize: undefined,
configuredWriteBufferManagerSize: undefined,
configuredCostToCache: undefined,
configuredAllowStall: undefined,
availableMemory: 8 * GB,
...overrides,
});
}

describe('resolveRocksMemoryConfig', function () {
describe('block cache', function () {
it('defaults to 25% of available memory when unset', function () {
assert.strictEqual(resolve({}).blockCacheSize, 2 * GB);
});

it('honors an explicit positive size', function () {
assert.strictEqual(resolve({ configuredBlockCacheSize: 512 * 1024 * 1024 }).blockCacheSize, 512 * 1024 * 1024);
});

it('falls back to the default for zero, negative, or non-number values', function () {
assert.strictEqual(resolve({ configuredBlockCacheSize: 0 }).blockCacheSize, 2 * GB);
assert.strictEqual(resolve({ configuredBlockCacheSize: -1 }).blockCacheSize, 2 * GB);
assert.strictEqual(resolve({ configuredBlockCacheSize: 'big' }).blockCacheSize, 2 * GB);
});
});

describe('WriteBufferManager size', function () {
it('defaults to 1/3 of the resolved block cache when unset', function () {
const config = resolve({});
assert.strictEqual(config.writeBufferManagerSize, Math.floor((2 * GB) / 3));
});

it('defaults relative to an explicit block cache size', function () {
const config = resolve({ configuredBlockCacheSize: 900 });
assert.strictEqual(config.writeBufferManagerSize, 300);
});

it('honors an explicit positive size', function () {
assert.strictEqual(
resolve({ configuredWriteBufferManagerSize: 256 * 1024 * 1024 }).writeBufferManagerSize,
256 * 1024 * 1024
);
});

it('disables the WBM entirely when explicitly set to 0', function () {
const config = resolve({ configuredWriteBufferManagerSize: 0 });
assert.ok(!('writeBufferManagerSize' in config));
assert.ok(!('writeBufferManagerCostToCache' in config));
assert.ok(!('writeBufferManagerAllowStall' in config));
});

it('falls back to the default for non-number values', function () {
assert.strictEqual(
resolve({ configuredWriteBufferManagerSize: 'lots' }).writeBufferManagerSize,
Math.floor((2 * GB) / 3)
);
});
});

describe('integer flooring', function () {
it('floors a fractional block cache default', function () {
// 10 * 0.25 = 2.5 -> 2
assert.strictEqual(resolve({ availableMemory: 10 }).blockCacheSize, 2);
});

it('floors a fractional WriteBufferManager default', function () {
// floor(10 / 3) = 3
const config = resolve({ configuredBlockCacheSize: 10 });
assert.strictEqual(config.writeBufferManagerSize, 3);
});

it('produces integer sizes for a realistic constrained memory limit', function () {
// 6 GB cgroup limit: block cache 1.5 GB, WBM 1.5 GB / 3
const config = resolve({ availableMemory: 6 * GB });
assert.ok(Number.isInteger(config.blockCacheSize));
assert.ok(Number.isInteger(config.writeBufferManagerSize));
});
});

describe('costToCache and allowStall', function () {
it('both default to true when the WBM is enabled', function () {
const config = resolve({});
assert.strictEqual(config.writeBufferManagerCostToCache, true);
assert.strictEqual(config.writeBufferManagerAllowStall, true);
});

it('honor explicit false values', function () {
const config = resolve({ configuredCostToCache: false, configuredAllowStall: false });
assert.strictEqual(config.writeBufferManagerCostToCache, false);
assert.strictEqual(config.writeBufferManagerAllowStall, false);
});

it('fall back to true for non-boolean values', function () {
const config = resolve({ configuredCostToCache: 'yes', configuredAllowStall: 1 });
assert.strictEqual(config.writeBufferManagerCostToCache, true);
assert.strictEqual(config.writeBufferManagerAllowStall, true);
});
});
});
55 changes: 55 additions & 0 deletions utility/rocksMemoryConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Resolves the RocksDB memory configuration (block cache + WriteBufferManager) from raw
// config values. Kept as a pure function so the defaulting logic can be unit tested without
// opening a database or touching the process-global RocksDatabase.config side effect.
//
// Values flow in from configUtils.castConfigValue (via envGet), which produces proper
// numbers/booleans/null — so we enforce types rather than coerce, and anything that isn't the
// expected type falls through to the default.

export interface RocksMemoryConfigInput {
configuredBlockCacheSize: unknown;
configuredWriteBufferManagerSize: unknown;
configuredCostToCache: unknown;
configuredAllowStall: unknown;
// min(process.constrainedMemory() ?? Infinity, totalmem()) — the cgroup-aware memory base.
availableMemory: number;
}

export interface RocksMemoryConfig {
blockCacheSize: number;
writeBufferManagerSize?: number;
writeBufferManagerCostToCache?: boolean;
writeBufferManagerAllowStall?: boolean;
}

export function resolveRocksMemoryConfig(input: RocksMemoryConfigInput): RocksMemoryConfig {
const {
configuredBlockCacheSize,
configuredWriteBufferManagerSize,
configuredCostToCache,
configuredAllowStall,
availableMemory,
} = input;
// Block cache: an explicit positive number wins, otherwise 25% of available memory. Floored
// because RocksDB expects integer byte counts and the percentage math produces fractions.
const blockCacheSize = Math.floor(
typeof configuredBlockCacheSize === 'number' && configuredBlockCacheSize > 0
? configuredBlockCacheSize
: availableMemory * 0.25
);
Comment on lines +33 to +39

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.

medium

Instead of adding defensive runtime checks for configuredBlockCacheSize in the resolver, ensure this configuration parameter is explicitly defined and validated in the Joi validation schema. This prevents invalid values like Infinity or non-numbers from bypassing validation while keeping the implementation free of redundant guards.

References
  1. Ensure all new configuration parameters are explicitly defined in the Joi validation schema, as they may otherwise bypass validation if allowUnknown: true is enabled, potentially leading to runtime errors.
  2. Avoid adding defensive checks or guards for states, properties, or methods that are guaranteed to exist or cannot occur in practice.

// WriteBufferManager size: an explicit number is honored (0 disables); any other value
// (unset/misconfigured) defaults to 1/3 of the block cache. Floored for the same reason.
const writeBufferManagerSize = Math.floor(
typeof configuredWriteBufferManagerSize === 'number' ? configuredWriteBufferManagerSize : blockCacheSize / 3
);
Comment on lines +40 to +44

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.

medium

Instead of adding inline defensive checks for configuredWriteBufferManagerSize to handle negative, NaN, or Infinity values, ensure that this parameter is explicitly validated in the Joi schema. This avoids redundant runtime guards in the resolver code.

References
  1. Ensure all new configuration parameters are explicitly defined in the Joi validation schema, as they may otherwise bypass validation if allowUnknown: true is enabled, potentially leading to runtime errors.
  2. Avoid adding defensive checks or guards for states, properties, or methods that are guaranteed to exist or cannot occur in practice.

const config: RocksMemoryConfig = { blockCacheSize };
// costToCache and allowStall only matter when the WBM is enabled. allowStall defaults to true
// so the buffer applies write backpressure rather than letting memtables grow unbounded, which
// also keeps bulk ingest from outrunning the memtable flush/conflict-check window.
if (writeBufferManagerSize > 0) {
config.writeBufferManagerSize = writeBufferManagerSize;
config.writeBufferManagerCostToCache = typeof configuredCostToCache === 'boolean' ? configuredCostToCache : true;
config.writeBufferManagerAllowStall = typeof configuredAllowStall === 'boolean' ? configuredAllowStall : true;
}
return config;
}
Loading