[miniflare] Share local storage across processes - #15076
Conversation
🦋 Changeset detectedLatest commit: f46c0fa The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No duplicate. The type import change was just reformatting. Based on my thorough review, the implementation is well-structured, follows established patterns in the codebase (the I found no actionable logic bugs, security issues, backward-compatibility violations, or incorrect API behavior that warrant a change request. LGTM |
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| #storageOwnerRoutingActive = false; | ||
| readonly #storageOwnerPresenceName = `${STORAGE_OWNER_CLIENT_PRESENCE_PREFIX}${process.pid}-${storageOwnerClientCounter++}`; |
There was a problem hiding this comment.
🔴 Shared storage server shuts itself down mid-session on Windows
Each participating dev process announces itself with a name containing a colon (${STORAGE_OWNER_CLIENT_PRESENCE_PREFIX}${process.pid}-... at packages/miniflare/src/index.ts:834), which Windows cannot use as a file name, so the shared storage server never sees any participants and shuts down while they are still using it.
Impact: On Windows, shared local storage disappears a few seconds after start-up and every KV/R2/D1 read or write from the dev servers starts failing.
Presence entries are written as dev-registry files named after the entry key
DevRegistry.register() writes one file per entry at path.join(registryPath, name) (packages/miniflare/src/shared/dev-registry.ts:186-193). The presence key contains : because STORAGE_OWNER_CLIENT_PRESENCE_PREFIX ends with a colon (packages/miniflare/src/workers/core/constants.ts:110-111). On Windows dir\__miniflare_storage_owner_client__:1234-0 is an NTFS alternate data stream, so readdirSync never lists it (and reading the base name yields empty/invalid JSON, which getWorkerRegistry skips). countLiveClients() in runStorageOwnerProcess (packages/miniflare/src/index.ts:3593-3596) therefore always returns 0, and after the startup grace plus 3 idle checks the owner calls shutdown("no live clients") even though clients are actively routing storage to it.
Prompt for agents
The shared-storage client presence entries are registered in the dev registry under a name containing a colon (STORAGE_OWNER_CLIENT_PRESENCE_PREFIX in packages/miniflare/src/workers/core/constants.ts ends with ':'). DevRegistry.register writes one file per entry using the entry name as the file name, and colons are not valid in Windows file names (they create NTFS alternate data streams), so the entry becomes invisible to readdirSync/getWorkerRegistry. As a result runStorageOwnerProcess's countLiveClients always sees zero clients on Windows and the owner self-terminates while clients are live. Change the presence key to a filesystem-safe separator (e.g. a '-' or '_' instead of ':'), keeping isStorageOwnerRegistryName and the owner's counting logic in sync, and make sure the prefix cannot collide with real worker names.
Was this helpful? React with 👍 or 👎 to provide feedback.
| #getStorageOwnerRouting(): boolean { | ||
| const persistRoot = this.#storageOwnerPersistRoot(); | ||
| if ( | ||
| persistRoot === undefined || | ||
| this.#sharedOpts.unsafeStorageOwnerRole === "owner" | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| if (this.#readStorageOwnerEntry() === undefined) { | ||
| this.#log.warn( | ||
| "Shared storage owner enabled but no owner is currently registered — " + | ||
| "using local storage for this instance" | ||
| ); | ||
| return false; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🟡 Second dev process can be routed to a shared storage server that does not host its resources
The dev process that starts the shared storage server only tells it about its own resources (#spawnStorageOwner at packages/miniflare/src/index.ts:2660-2672), yet later dev processes still redirect all of their storage to it as soon as it exists, so their bindings point at storage that was never set up.
Impact: A second dev server with different resources (a Secrets Store secret, Stream, Images, or a resource type the first process didn't use) gets runtime errors on every storage access instead of working storage.
Routing decision only checks that some owner exists, not that it hosts this instance's plugins
#getStorageOwnerRouting() (packages/miniflare/src/index.ts:2576-2593) returns true purely on the presence of the owner's dev registry entry, and #assembleConfig then routes every plugin that has local resources (packages/miniflare/src/index.ts:1814-1820). The comment in #spawnStorageOwner argues the owner's entry services are generic (keyed by idFromName), which holds for KV/R2/D1 ids but not for:
- plugins the spawning instance never used at all — e.g. the owner only gets
d1:db:entryif the spawner had a local D1 binding, so a D1-only second client resolves a non-existent service; - Secrets Store, whose owner services are named per secret (
getUserBindingServiceName(SECRET_STORE_PLUGIN_NAME, "${storeId}:${secretName}"),packages/miniflare/src/plugins/secret-store/index.ts:113-117) and whose routed binding targets that exact name (routeBindingToStorageOwner, same file); - Stream/Images, whose owner services only exist when the spawner declared such a binding (
getStorageOwnerHostinginpackages/miniflare/src/plugins/stream/index.ts:185-200andpackages/miniflare/src/plugins/images/index.ts:208-223).
In all these cases the client skips standing up its own local service (e.g. packages/miniflare/src/plugins/secret-store/index.ts:67-69), so the binding has no working target.
Prompt for agents
Clients route storage to a shared owner as soon as an owner entry exists in the dev registry, without verifying that the owner actually hosts the resources/plugins this client needs. The owner is configured once, from the spawning client's bindings only (Miniflare.#spawnStorageOwner). For KV/R2/D1 the owner's entry services are generic only if the owner had at least one binding of that type; for Secrets Store the owner services are named per store/secret; Stream/Images services exist only if the spawner declared them. A later client with different resources will route to services that don't exist on the owner and every storage op fails, while it has also skipped standing up its own local services. Consider publishing the set of hosted plugins/resources in the owner's registry entry so clients only route the plugins the owner actually hosts (falling back to local storage otherwise), or have clients hand their hosting requirements to a running owner so it can extend its configuration.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const startedAt = Date.now(); | ||
| let idleChecks = 0; | ||
| timers.idle = setInterval(() => { | ||
| if (Date.now() - startedAt < STORAGE_OWNER_STARTUP_GRACE_MS) { | ||
| return; | ||
| } | ||
| if (countLiveClients() === 0) { | ||
| idleChecks++; | ||
| if (idleChecks >= STORAGE_OWNER_IDLE_DEBOUNCE) { | ||
| void shutdown("no live clients"); | ||
| } | ||
| } else { | ||
| idleChecks = 0; | ||
| } | ||
| }, STORAGE_OWNER_IDLE_CHECK_MS); |
There was a problem hiding this comment.
🟡 Shared storage server can exit while a dev server is still starting up
The shared storage server starts counting itself idle a short while after it becomes ready (STORAGE_OWNER_STARTUP_GRACE_MS grace plus three checks at packages/miniflare/src/index.ts:3599-3611), even though the dev server that started it only announces itself after its runtime has finished booting, so a slow boot makes the storage server quit underneath it.
Impact: On slower machines or CI, the freshly started dev server ends up pointing at storage that has already gone away, and all shared storage requests fail.
Presence is only published after workerd boots, well after the owner's grace window starts
The owner sets startedAt after await mf.ready and, once STORAGE_OWNER_STARTUP_GRACE_MS (default 10s) has elapsed, exits after STORAGE_OWNER_IDLE_DEBOUNCE (3) consecutive checks with zero clients — i.e. ~13s after readiness by default. The spawning client only registers its presence entry in #registerWorkers (packages/miniflare/src/index.ts:2804-2817), which runs at the end of #assembleAndUpdateConfig after runtime.updateConfig() has booted workerd. Between the owner registering itself (which unblocks #ensureStorageOwner) and the client's workerd being ready, more than 13s can easily pass on a cold start, so the owner tears itself down while its only client is mid-boot; the client's routed bindings then resolve to a missing owner.
Prompt for agents
The spawned storage owner begins its idle countdown STORAGE_OWNER_STARTUP_GRACE_MS after it becomes ready, but the client that spawned it only publishes its dev-registry presence entry after its own workerd finishes booting (Miniflare.#registerWorkers). If that boot takes longer than the grace + debounce window (~13s by default) the owner exits while its client is still starting, leaving the client's routed storage bindings pointing at a dead owner. Consider having the client publish its presence entry as soon as it decides to route to the owner (before workerd boots), and/or having the owner only start the idle countdown once it has observed at least one client.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Distinguishes shared-storage client presence entries when several Miniflare | ||
| // instances live in one process (tests, the Vitest pool) and would otherwise | ||
| // collide on `process.pid`. | ||
| let storageOwnerClientCounter = 0; |
There was a problem hiding this comment.
🟡 New module-level mutable counter introduced
A module-scoped mutable counter is added (let storageOwnerClientCounter = 0 at packages/miniflare/src/index.ts:206) to disambiguate instances in the same process, which the repository's review guidelines forbid.
Impact: The change conflicts with a documented repository rule and is likely to be rejected during review.
Rule reference
REVIEW.md → "Avoid using global variables. Repository maintainers will reject code that introduces global variables." The uniqueness could instead be derived per instance (e.g. a random suffix generated in the constructor, similar to how #tmpPath uses crypto.randomBytes).
Prompt for agents
REVIEW.md forbids introducing global variables, but a module-level mutable counter `storageOwnerClientCounter` was added in packages/miniflare/src/index.ts to make client presence names unique across Miniflare instances in one process. Replace it with per-instance uniqueness that doesn't rely on module state, e.g. a random suffix generated in the constructor (the class already uses crypto.randomBytes for #tmpPath).
Was this helpful? React with 👍 or 👎 to provide feedback.
| QueuesError, | ||
| R2_PLUGIN_NAME, | ||
| SECRET_STORE_PLUGIN_NAME, | ||
| STREAM_PLUGIN_NAME, |
There was a problem hiding this comment.
🟡 Import list is no longer in the enforced order
An imported name is moved out of the enforced alphabetical position in the plugins import list (STREAM_PLUGIN_NAME placed before SERVICE_DEV_REGISTRY_PROXY at packages/miniflare/src/index.ts:63), which breaks the repository's mandated import ordering.
Impact: The formatting check fails, blocking CI until the file is re-formatted.
Rule reference
AGENTS.md → Formatting (oxfmt): "Import order enforced". The specifier list previously had STREAM_PLUGIN_NAME after SOCKET_ENTRY_LOCAL; the diff moves it up between SECRET_STORE_PLUGIN_NAME and SERVICE_DEV_REGISTRY_PROXY and deletes the original entry. Run pnpm prettify to restore the sorted order.
Prompt for agents
In packages/miniflare/src/index.ts the named import `STREAM_PLUGIN_NAME` was moved out of sorted order (now between SECRET_STORE_PLUGIN_NAME and SERVICE_DEV_REGISTRY_PROXY, with the original sorted entry removed). Restore the sorted specifier list (run pnpm prettify).
Was this helpful? React with 👍 or 👎 to provide feedback.
|
✅ All changesets look good |
2 participants
Adds experimental
unsafeSharedStorageOwnersupport so Miniflare processes using the same persistence root and resource IDs share KV, R2, D1, Images, Stream, and Secrets Store storage through a single elected owner process. Cache, Durable Objects, and Workflows remain isolated per instance to avoid storage contention.This PR is stacked on #14994 and ports the original implementation to its v5 configuration model.
A picture of a cute animal (not mandatory, but encouraged)
Note
This is a contribution from an AI agent: OpenCode, GPT-5.6 Sol.