Skip to content

feat(store-memory): implement in-process memory store#1710

Merged
jfallows merged 21 commits intodevelopfrom
feature/1667-store-memory
Apr 8, 2026
Merged

feat(store-memory): implement in-process memory store#1710
jfallows merged 21 commits intodevelopfrom
feature/1667-store-memory

Conversation

@jfallows
Copy link
Copy Markdown
Contributor

@jfallows jfallows commented Apr 7, 2026

Summary

Closes #1667

Implements the memory store type in a new runtime/store-memory module. This is the default store for single-instance deployments and development environments.

  • MemoryStoreFactorySpi — registers the memory type name
  • MemoryStore — shared ConcurrentHashMap-backed state, created once per store instance
  • MemoryStoreContext — per-worker context supplying MemoryStoreHandler
  • MemoryStoreHandler — delegates all operations inline to MemoryStore (synchronous, no thread switch)
  • specs/store-memory.spec/ — schema patch registering memory as a valid store type
  • MemoryStoreHandlerTest — unit tests covering all 5 operations and TTL expiry

Behaviour

  • All StoreHandler operations invoke completion synchronously on the calling thread
  • putIfAbsent uses ConcurrentHashMap.putIfAbsent — atomic
  • getAndDelete uses ConcurrentHashMap.remove — atomic
  • Entries carry an expiry timestamp; get returns null for expired entries (lazy expiry)
  • Intentionally unbounded — no eviction; entries are retained until TTL expiry or explicit removal

Test plan

  • ./mvnw install -DskipITs -pl runtime/store-memory -am compiles without errors
  • ./mvnw test -pl runtime/store-memory — all unit tests pass
  • A zilla.yaml with stores: type: memory parses without schema error

@jfallows jfallows force-pushed the feature/1667-store-memory branch 2 times, most recently from 070dfbc to 48a77d7 Compare April 7, 2026 22:07
Comment on lines +83 to +91
if (existing == null || existing.expired())
{
if (existing != null && existing.expired())
{
entries.replace(key, existing, newEntry);
}
return null;
}
return existing.value;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
if (existing == null || existing.expired())
{
if (existing != null && existing.expired())
{
entries.replace(key, existing, newEntry);
}
return null;
}
return existing.value;
if (existing != null && existing.expired())
{
entries.replace(key, existing, newEntry);
existing.value = null;
}
return existing != null ? existing.value : null;

*/
package io.aklivity.zilla.runtime.store.memory.internal;

final class MemoryEntry
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggest using record instead.

@jfallows jfallows force-pushed the feature/1667-store-memory branch 8 times, most recently from 46e4140 to 27f5438 Compare April 7, 2026 23:37
claude added 16 commits April 7, 2026 23:44
Adds runtime/store-memory module implementing the `memory` store type
backed by a shared ConcurrentHashMap with lazy TTL expiry.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
…ests

- Create specs/store-memory.spec/ with memory.schema.patch.json
- Fix MemoryStore.type() path: "internal/..." -> "schema/..." (matches vault-filesystem convention)
- Add maven-dependency-plugin unpack to copy schema from spec JAR into classes
- Remove orphaned MemoryStore.close() (Store interface has no close())
- Add MemoryStoreHandlerTest covering get/put/putIfAbsent/delete/getAndDelete/expiry
- Raise jacoco coverage threshold from 0% to 50%
- Add store-memory.spec to specs/pom.xml module list and dependencyManagement

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
… MemoryStore

- specs/store-memory.spec was missing SchemaTest causing jacoco 100% coverage
  requirement to fail with no Java classes to measure
- Add SchemaTest validating store.yaml config with memory type
- Add store.yaml config fixture for schema validation
- Remove trailing blank line before closing brace in MemoryStore

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
The moditect-maven-plugin requires a module-info.java descriptor in
src/main/moditect/ — without it the module system wiring fails at build
time. Added open module declaration mirroring the vault-filesystem.spec
pattern.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
…gestions

- Add missing COPYRIGHT, LICENSE, NOTICE, NOTICE.template, mvnw, mvnw.cmd
  to specs/store-memory.spec/ — maven-notice-plugin validate check requires
  a NOTICE file to be present in each module
- Convert MemoryEntry class to record as suggested by @jfallows
- Simplify MemoryStore#putIfAbsent to remove redundant nested conditional
  per reviewer suggestion; adapt for record (immutable components)

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
…y test

- Add missing COPYRIGHT, LICENSE, NOTICE, NOTICE.template, mvnw, mvnw.cmd
  to runtime/store-memory/ — maven-notice-plugin validate check requires
  these in every module directory
- Fix shouldExpireEntry: ttl=0 sets expiresAt=currentTimeMillis() so expired()
  (strict >) can return false when called in the same millisecond; use ttl=-1
  so expiresAt=currentTimeMillis()-1 which is always already expired

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
…pendencies

The maven-notice-plugin generates expected NOTICE from compile-scope deps.
Copied vault-filesystem.spec NOTICE had stale entries (ANTLR 4, JUEL, k3po);
regenerated with notice:generate to match actual transitive deps of engine.spec.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
…age test

Per CLAUDE.md convention, every runtime component needs an XxxConfiguration
class (even as a placeholder) so that config properties can be added later
without structural changes. Includes MemoryStoreConfigurationTest to satisfy
class coverage requirements.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
…tore

Per CLAUDE.md convention, factory SPI must construct the component-specific
Configuration subclass from the raw config and pass it — not the raw
Configuration — into collaborators. Updated MemoryStore constructor to accept
MemoryStoreConfiguration; factory and test wired accordingly.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
Copy LICENSE, COPYRIGHT, NOTICE.template from top-level community
files; regenerate NOTICE; update pom.xml license declaration;
apply community license headers to all source files via license:format.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
…n, use in tests

Per CLAUDE.md: XxxConfiguration should have both a no-args constructor
(super(XXX_CONFIG, new Configuration())) and a Configuration-parameter
constructor; prefer no-args in unit tests.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
@jfallows jfallows force-pushed the feature/1667-store-memory branch 2 times, most recently from ed8f843 to ce352a0 Compare April 7, 2026 23:57
MemoryStore maps StoreConfig.id -> ConcurrentMap via computeIfAbsent
so all cores with the same id rendezvous on the same map while
different ids remain fully isolated. MemoryStoreHandler now holds
the entries map directly and owns all key operations.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
public StoreContext supply(
EngineContext context)
{
return new MemoryStoreContext(this);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Instead pass this::supplyEntries and rename attach to supplyEntries, also make it private.
Then MemoryStoreContext can have a parameter of type LongFunction<ConcurrentMap<String, MemoryEntry>>.

{
static final String NAME = "memory";

private final ConcurrentMap<Long, ConcurrentMap<String, MemoryEntry>> entries;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggest renaming this as storage instead of entries.

@Before
public void setUp()
{
handler = new MemoryStoreHandler(new MemoryStore(new MemoryStoreConfiguration()).attach(0L));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Use the normal pattern to create the handler, via MemoryStore supplyContext(...), attach(StoreConfig).

@Test
public void shouldReturnNullForMissingKey()
{
final AtomicReference<String> result = new AtomicReference<>();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do we need AtomicReference<String> in these tests, or is MutableReference<String> sufficient?

claude added 2 commits April 8, 2026 00:40
- Rename MemoryStore.entries field to storage
- Rename MemoryStore.attach to supplyEntries (private), pass as method ref
- Replace MemoryStore field in MemoryStoreContext with LongFunction<ConcurrentMap>
- Change test handler field type to StoreHandler, use supply()/attach() pattern
- Replace AtomicReference with MutableReference in test methods

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
Add removeEntries method to MemoryStore and pass as LongConsumer to
MemoryStoreContext so detach removes the per-store-id entry map from
storage, preventing unbounded memory growth.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
private void removeEntries(
long storeId)
{
storage.remove(storeId);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There can be multiple contexts per store, one context per engine worker, so removing the entries from storage on the first call to detach is not correct.

Instead we need a reference count on the storage, and only remove when it reaches zero.

Perhaps the lambdas should be called acquireEntries and releaseEntries instead?

…afe detach

Introduce MemoryStorage inner class holding AtomicInteger refs and
ConcurrentMap<String, MemoryEntry> entries. Rename supplyEntries/removeEntries
to acquireEntries/releaseEntries to reflect lifecycle semantics.

acquireEntries increments the ref count; releaseEntries decrements it and
removes the MemoryStorage from the outer map only when refs reaches zero,
ensuring that multiple contexts (one per engine worker) sharing the same
store id do not prematurely evict each other's entries.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
@jfallows jfallows merged commit d234bd6 into develop Apr 8, 2026
39 of 40 checks passed
jfallows pushed a commit that referenced this pull request Apr 8, 2026
Resolves conflicts: take develop version for all store-memory files
(final merged implementation) and .gitignore; take develop CLAUDE.md
content for the engine concept test implementation guidance section.

https://claude.ai/code/session_0174raBeXFTgt98bp4DTyRDm
@jfallows jfallows deleted the feature/1667-store-memory branch April 9, 2026 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

store-memory: implement in-process memory store

2 participants