Feature/mmap store backend#828
Open
UlysseCorbeil wants to merge 18 commits into
Open
Conversation
Resolve save-path conflict by porting the lazy streaming marshal (#822) to the KVImpl backends: replace the eager MarshalStreamIter with MarshalStreamSnapshot over a pull-based KVImpl.Snapshot() iterator. mmap snapshots read in bounded batches (short View tx per batch) and block writes for the snapshot lifetime via RWMutex.
Stages.Close (leak fix in 25a7ed2) also closed the FullKV stores that FinalStoreMap hands to the linear pipeline, which then failed writes with 'database not open'. Transfer ownership by detaching cachedStore, and close the pipeline store map at request end on tier1. Also close the throwaway FullKV created by estimateStoreSizeBytes.
Unlink each bbolt store file right after opening it: on unix the inode and its mmap stay valid for the open handle's lifetime, but the directory entry is gone, so no process death (SIGKILL, OOM, shutdown deadline outliving a multi-minute Load/Save) can leave an orphan behind. Also make Close remove the file even when db.Close errors. Separately, a load aborted by context cancellation was wrapped as ErrInvalidFullKVFile, which made callers delete the valid remote .kv file and forced reprocessing. Load now returns the cancellation, and the corrupt-store delete sites additionally guard on a live context. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
A canceled tier2 request kept reading the entire (multi-GB) store file into the heap before returning, then held it via a fire-and-forget SetMetadata goroutine capturing the whole store and a no-op Close. - Load now checks the request context while streaming entries and bails on cancel. - memoryKVImpl.Close drops its backing map. - the post-load SetMetadata goroutine captures only what it needs and runs under a bounded timeout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors Substreams store state management to support a mmap/bbolt-backed KV backend (to reduce heap pressure/OOM risk), improves store lifecycle cleanup to avoid leaked resources, and adds observability around store backend usage and mmap operations/sizes.
Changes:
- Introduces a
KVImplabstraction with a default mmap/bbolt implementation plus an in-memory fallback, and migrates store read/write paths to use it. - Reworks store load/save/marshal paths to stream entries (iterator-based) and hardens cancellation/error handling to avoid misclassifying canceled loads as corruption.
- Adds explicit store backend/scratch-space configuration plumbing across tier1/tier2/services and introduces new store backend metrics; adds targeted mmap regression tests/benchmarks.
Reviewed changes
Copilot reviewed 63 out of 65 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wasm/call_test.go | Updates store config constructor call to include new backend parameters. |
| tools/module.go | Passes new store backend/scratch-space args into store config construction. |
| tools/decode.go | Updates store config construction calls for new backend parameters. |
| tools/check.go | Updates store config construction call for new backend parameters. |
| tools/analytics_store_stats.go | Updates store config construction call for new backend parameters. |
| tests_e2e/utils_test.go | Adds tier2 scratch-space plumbing and switches container bind config to Mounts. |
| tests_e2e/mmap_test.go | Adds end-to-end tests covering mmap vs memory backends and comparison scenarios. |
| tests_e2e/go.sum | Updates test module dependency checksums. |
| tests_e2e/go.mod | Updates test module dependencies (removes docker/docker direct dep, adds bbolt indirect). |
| storage/store/value_get.go | Switches key lookups to kvImpl.Get to support non-map backends. |
| storage/store/value_delete.go | Makes prefix deletes scan-safe for mmap (avoid deadlocks) and prevents OldValue aliasing. |
| storage/store/value_delete_mmap_test.go | Adds regression test ensuring delete deltas don’t retain bbolt-aliased OldValue slices. |
| storage/store/store_sum_test.go | Migrates tests from direct map mutation to kvImpl operations. |
| storage/store/store_setsum_test.go | Migrates tests from direct map mutation to kvImpl operations. |
| storage/store/store_min_test.go | Updates test initialization to reset/load via kvImpl. |
| storage/store/store_max_test.go | Updates test initialization to reset/load via kvImpl. |
| storage/store/partial_kv.go | Streams partial store load/save via iterators and snapshot streaming; clears via kvImpl.Clear. |
| storage/store/partial_kv_test.go | Updates partial KV tests to validate kvImpl presence instead of raw map. |
| storage/store/merge.go | Replaces map-based merge with batched, transaction-efficient merge using Iter, GetMany, and BatchSet. |
| storage/store/merge_test.go | Updates merge tests for kvImpl-based storage and snapshot comparisons. |
| storage/store/merge_mmap_test.go | Adds mmap-vs-memory merge oracle test for ADD/bigdecimal scenario. |
| storage/store/marshaller/vtproto.go | Adds iterator-based streaming unmarshal and snapshot-based lazy marshal. |
| storage/store/marshaller/interface.go | Extends marshaller interfaces to support iterator-based load/save and snapshot streaming. |
| storage/store/kv_impl.go | Introduces the KVImpl interface for pluggable KV backends. |
| storage/store/kv_impl_snapshot_test.go | Adds snapshot streaming round-trip and snapshot write-gate tests. |
| storage/store/kv_impl_mmap.go | Implements mmap/bbolt KV backend with batching, snapshot gating, and metrics hooks. |
| storage/store/kv_impl_mmap_roundtrip_test.go | Adds byte-exact mmap snapshot/load round-trip regression test. |
| storage/store/kv_impl_mmap_cancel_test.go | Adds test ensuring mmap load cancellation doesn’t leave orphaned bbolt files. |
| storage/store/kv_impl_merge_micro_bench_test.go | Adds micro-benchmarks isolating merge performance costs on mmap backend. |
| storage/store/kv_impl_memory.go | Implements in-memory KV backend and ensures Close drops backing map to release memory. |
| storage/store/kv_impl_integration_test.go | Adds integration tests across mmap/memory backends, fallback behavior, snapshot/load behavior. |
| storage/store/kv_impl_config.go | Adds backend selection/config resolution (config arg vs env var) for KVImpl creation. |
| storage/store/kv_impl_bench_test.go | Adds benchmarks for KV operations and serialization/merge paths. |
| storage/store/iterable.go | Routes store iteration/length through kvImpl instead of direct map access. |
| storage/store/iter_alias_mmap_test.go | Adds regression test ensuring retained Iter values are copied (no bbolt aliasing). |
| storage/store/init_test.go | Updates base store test helper to initialize kvImpl and streaming marshaller. |
| storage/store/full_kv.go | Streams full store load/save/quicksave via snapshot streaming and iterator load; adds cancellation classification fix. |
| storage/store/full_kv_test.go | Updates tests for kvImpl and adds canceled-load-not-corrupt regression test. |
| storage/store/configmap.go | Adds scratch-space/backend args to config map creation. |
| storage/store/config.go | Adds store backend/scratch-space config, KVImpl initialization, and forces partial stores to use memory backend. |
| storage/store/common.go | Adds unmarshalIterInto + cancellation-aware iterator wrapper for streaming loads. |
| storage/store/base_store.go | Replaces raw KV map with kvImpl, switches marshaller to streaming interface, and adds Close(). |
| service/tier2.go | Plumbs backend/scratch-space into store config map; ensures stores are closed after processing. |
| service/tier1.go | Plumbs backend/scratch-space into store config map; ensures stores are closed at request end. |
| service/tier1_test.go | Updates store config helper for new backend parameters. |
| service/options.go | Adds service options for store scratch space/backend (tier2 + partial tier1 support). |
| service/config/runtimeconfig.go | Adds runtime config fields for store scratch space/backend and wires into tier1 runtime config creation. |
| pipeline/stores.go | Adds Stores.Close() to close all stores and release backend resources. |
| pipeline/snapshot.go | Copies iterated values to avoid bbolt aliasing when accumulating snapshot deltas. |
| pipeline/pipeline.go | Adds store cleanup on quickload/setup failures; fixes corrupt-file deletion logic for canceled contexts; avoids goroutine pinning full stores in metadata updates. |
| pipeline/pipeline_test.go | Updates store config construction for new backend parameters. |
| orchestrator/stage/stages.go | Adds Stages.Close() to close cached store module states; detaches ownership on final store map handoff. |
| orchestrator/stage/squash.go | Uses cache-independent full store load helper; ensures old full store is closed when replaced; adds merge size log. |
| orchestrator/stage/modstate.go | Ensures loaded full stores are closed on error/replacement; adds module-state Close(); adds tryLoadFullKV. |
| orchestrator/stage/fetchstorage_test.go | Updates store config helper for new backend parameters. |
| orchestrator/parallelprocessor.go | Ensures stages are closed after run to release store resources. |
| metrics/metrics.go | Adds new store backend and mmap metrics declarations. |
| go.work.sum | Updates workspace sums for dependency changes. |
| go.sum | Adds bbolt checksums. |
| go.mod | Adds bbolt dependency. |
| docs/release-notes/change-log.md | Documents store load cancellation behavior and memory pinning fixes. |
| app/tier2.go | Adds Tier2 config fields and wires new service options for scratch space/backend selection. |
| app/tier1.go | Adds Tier1 config fields and passes scratch space/backend into tier1 service construction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
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
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.
Adds an mmap (bbolt-backed) store backend as an alternative to the existing in-memory (Go heap) store, selectable via
--substreams-stores-backend=mmap. The goal is to address production OOMs where large or concurrent FullKV stores exhaust a pod's heap: mmap keeps store data in a memory-mapped bbolt file so cold pages are reclaimable by the kernel under pressure, instead of pinning everything on the non-reclaimable Go heap. The in-memory backend stays the default; mmap is opt-in per deployment.Store backend management and configuration
StoresScratchSpaceandStoresBackendconfig fields to Tier1 and Tier2, wired through to the services.Store correctness (mmap)
Resource cleanup and error handling
Closemethods onStoreModuleStateandStages, plus cleanup in pipelines/orchestrators).Store load and metadata handling
SetMetadatagoroutine no longer captures the whole store and runs under a bounded timeout.tryLoadFullKVfor context-aware, cache-independent loading.Metrics and observability