Make RocksDB memory config configurable (block cache + WriteBufferManager)#780
Merged
Conversation
…heSize Adds STORAGE_ROCKS_BLOCKCACHESIZE config param (env var or YAML). When set, it overrides the default 25%-of-constrained-memory heuristic. Reading is done lazily inside openRocksDatabase() so env/CLI overrides applied after module load are always respected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Reviewed; no blockers found. |
cb1kenobi
reviewed
May 26, 2026
Adds three new config params for the process-wide RocksDB WriteBufferManager exposed by @harperfast/rocksdb-js#584: - storage.rocks.writeBufferManagerSize — bytes the WBM caps across all DBs in the process (0 disables). The structural cap on total memtable + maintain-window memory. - storage.rocks.writeBufferManagerCostToCache — when true, memtable charges show up in block-cache-usage as pinned entries (single observability metric for read cache + memtable footprint). - storage.rocks.writeBufferManagerAllowStall — when true, writes stall once the cap is reached instead of allowing memtables to briefly exceed it. Values are read inside openRocksDatabase() and passed via spread, so unset values don't surface unknown options to older bindings of rocksdb-js where the WBM isn't yet wired up. This makes the change forward-compatible: it's a no-op until the env var is set AND the underlying binding supports it. Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
kriszyp
changed the title
envGet may return raw strings from process.env (configUtils doesn't cast every code path), so RocksDatabase.config previously could receive "12345" instead of 12345 for the size params and "false" (truthy in JS) instead of false for the booleans. - Numeric params (blockCacheSize, writeBufferManagerSize): wrap with Number(). Falsy results (NaN, 0) fall through the > 0 guard. - Boolean params (writeBufferManagerCostToCache, allowStall): inline toBool() that recognizes booleans, 'true'/'false' strings, and generic truthy/falsy fallback. Returns undefined when unset so the spread skips the property. Per PR review from @cb1kenobi (PR #780, line 169). Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
Reverts 064501b's Number()/toBool() coercion in favor of strict type checks. Config values flow through configUtils.castConfigValue which produces proper numbers/booleans from YAML, env vars, and CLI args — anything else arriving here is misconfiguration that should fall through to the default rather than be silently rescued. - blockCacheSize / writeBufferManagerSize: only honored when `typeof === 'number' && value > 0`. Non-numbers (including unparseable strings) fall back to the 25% default for the cache and disable the WBM. - writeBufferManagerCostToCache / allowStall: only honored when `typeof === 'boolean'`. The 'false' string is no longer accepted — it's a YAML-quoting mistake the operator should fix. Per feedback on PR #780 review thread. Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
cb1kenobi
approved these changes
May 26, 2026
kriszyp
added a commit
that referenced
this pull request
May 26, 2026
envGet may return raw strings from process.env (configUtils doesn't cast every code path), so RocksDatabase.config previously could receive "12345" instead of 12345 for the size params and "false" (truthy in JS) instead of false for the booleans. - Numeric params (blockCacheSize, writeBufferManagerSize): wrap with Number(). Falsy results (NaN, 0) fall through the > 0 guard. - Boolean params (writeBufferManagerCostToCache, allowStall): inline toBool() that recognizes booleans, 'true'/'false' strings, and generic truthy/falsy fallback. Returns undefined when unset so the spread skips the property. Per PR review from @cb1kenobi (PR #780, line 169). Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exposes RocksDB memory tuning knobs as Harper config params so deployments can override the hardcoded defaults. Adds four new params, all under
storage.rocks.*:storage.rocks.blockCacheSizestorage.rocks.writeBufferManagerSizestorage.rocks.writeBufferManagerCostToCacherocksdb.block-cache-usagemetric.storage.rocks.writeBufferManagerAllowStallWhy: On shared Fabric hosts, each container's RocksDB memory footprint accumulates. For a canopy-style customer with 14 tables, the per-CF maintain window alone holds ~1.8 GB of anonymous memory by default. Today there's no way to bound this without rebuilding harper-pro. These params let host-manager (and other operators) configure both subsystems without code changes.
Implementation: Both reads happen inside
openRocksDatabase()so env/CLI overrides applied after module load are respected. Values are passed toRocksDatabase.config()via conditional spread — unset env vars don't surface unknown options to older bindings of@harperfast/rocksdb-js(forward-compatibility).Dependency: The WBM options are no-ops until HarperFast/rocksdb-js#584 lands and harper-pro updates its dependency. The
blockCacheSizeparam works today.Cross-model review findings addressed:
openRocksDatabase()so config changes after module load are picked up.> 0guard onblockCacheSizehandles0/negative/NaN.'true'/'false'→ boolean,'123'→ number).Generated by Claude. — Claude Sonnet 4.7