Review Culling support

[strawgate]

We should review our stores for future improvements to how we handle expired objects and expired object culling.

For example, should we offer a setting that causes the store to issue a delete if it gets back an expired managedentry?

Should we be triggering culling on stores at some interval? Should that be an option?

[claude]

Claude finished @strawgate's task —— View job

---

Investigation Complete ✓

I've conducted a thorough investigation of culling support for expired objects across all stores in py-key-value.

---

Recommendation

Implement a multi-layered approach to expiration culling:

1. Add "delete-on-read" option for stores without native TTL (Priority: High)
2. Document culling capabilities per store (Priority: High)
3. Add periodic culling wrapper as an optional component (Priority: Medium)
4. Expand CullProtocol support to more stores (Priority: Low)

Findings

Current State of Expiration Handling

py-key-value stores handle expired objects using three different strategies:

1. Native Automatic Culling (Best - Zero Application Overhead)

These stores leverage backend-native TTL features for automatic cleanup:

Store	Mechanism	Cleanup Interval	Implementation
Redis	SETEX command	Immediate (active+passive eviction)	redis/store.py:141
Valkey	ExpirySet with TTL	Immediate (like Redis)	valkey/store.py:127-129
DynamoDB	TTL attribute	~Hourly background process	dynamodb/store.py:178-181
MongoDB	TTL Index (expireAfterSeconds=0)	~60 second background thread	mongodb/store.py:234-235
Memory	cachetools TTU	Immediate on access	memory/store.py:38-42
Disk	diskcache expire	On access	disk/store.py:89-99
Memcached	Native TTL	Immediate (LRU eviction)	memcached/store.py

✅ These stores DON'T need culling - expiration happens automatically at the backend level.

2. Manual Culling via CullProtocol (Good - Requires Manual Trigger)

These stores implement AsyncCullProtocol with a cull() method:

Store	Implementation	How it Works
Elasticsearch	_cull() method	Executes delete_by_query with expires_at < now filter across all indices

Code Reference (elasticsearch/store.py:537-548):

async def _cull(self) -> None:
    ms_epoch = int(now_as_epoch() * 1000)
    _ = await self._client.options(ignore_status=404).delete_by_query(
        index=f"{self._index_prefix}-*",
        body={
            "query": {
                "range": {
                    "expires_at": {"lt": ms_epoch},
                },
            },
        },
    )

⚠️ Requires manual invocation - application must call await store.cull() periodically.

3. Application-Level Expiration Checking Only (Suboptimal - Data Accumulates)

These stores check is_expired on read but never delete expired entries:

Store	Notes
Keyring	Stores TTL in JSON payload, checked at retrieval time (keyring/store.py:67-68)
WindowsRegistry	Stores TTL in JSON payload, explicitly noted "does not currently cull expired entries" (windows_registry/store.py:47-49)
RocksDB	No native TTL support
Simple	Testing/dev store, no culling needed
Null	No-op store

❌ Problem: Expired entries accumulate indefinitely, wasting storage.

How Expiration Currently Works

All stores inherit from BaseStore which implements application-level expiration checking:

base.py:231-232:

if managed_entry.is_expired:
    return None

This prevents reading expired data but does NOT delete it. For stores without native TTL (Category 3), this creates a storage leak.

Detailed Action Plan

Priority 1: Delete-on-Read Option (High Impact, Low Complexity)

Problem Solved: Stores like Keyring, WindowsRegistry, RocksDB accumulate expired entries indefinitely.

Proposal: Add an optional delete_on_expired_read parameter to BaseStore:

class BaseStore(AsyncKeyValueProtocol, ABC):
    _delete_on_expired_read: bool
    
    def __init__(
        self,
        *,
        delete_on_expired_read: bool = False,  # NEW parameter
        serialization_adapter: SerializationAdapter | None = None,
        # ... existing parameters
    ) -> None:
        self._delete_on_expired_read = delete_on_expired_read
        # ... existing init code
    
    @bear_enforce
    @override
    async def get(self, key: str, *, collection: str | None = None) -> dict[str, Any] | None:
        collection = collection or self.default_collection
        await self.setup_collection(collection=collection)
        
        managed_entry: ManagedEntry | None = await self._get_managed_entry(collection=collection, key=key)
        
        if not managed_entry:
            return None
        
        if managed_entry.is_expired:
            # NEW: Optionally delete expired entries on read
            if self._delete_on_expired_read:
                await self._delete_managed_entry(key=key, collection=collection)
            return None
        
        return dict(managed_entry.value)

Benefits:

• ✅ Works for ALL stores (universal solution)
• ✅ Opt-in via parameter (backward compatible)
• ✅ No new protocol needed
• ✅ Prevents indefinite accumulation for stores without native TTL
• ✅ Simple to implement (~10 lines of code change)

Trade-offs:

• ⚠️ Adds latency to reads that encounter expired entries
• ⚠️ Requires reads to trigger cleanup (unused keys never get cleaned up)

Implementation: Modify BaseStore.get(), get_many(), ttl(), ttl_many() methods.

---

Priority 2: Expand CullProtocol Support (Medium Impact, Medium Complexity)

Current State: Only Elasticsearch implements AsyncCullProtocol.

Proposal: Add _cull() implementations for stores that can efficiently bulk-delete expired entries:

RocksDB: Enumerate and Delete

class RocksDBStore(BaseCullStore, ...):
    async def _cull(self) -> None:
        # RocksDB doesn't support TTL queries, must scan
        keys_to_delete: list[str] = []
        
        for key in self._db.keys():  # Scan all keys
            entry = await self._get_managed_entry_from_raw_key(key)
            if entry and entry.is_expired:
                keys_to_delete.append(key)
        
        # Batch delete
        for key in keys_to_delete:
            self._db.delete(key)

WindowsRegistry: Enumerate and Delete

class WindowsRegistryStore(BaseCullStore, ...):
    async def _cull(self) -> None:
        # Similar to RocksDB: enumerate, check expiration, delete
        for collection in self._enumerate_collections():
            for key in self._enumerate_keys(collection):
                entry = await self._get_managed_entry(collection=collection, key=key)
                if entry and entry.is_expired:
                    await self._delete_managed_entry(collection=collection, key=key)

Benefits:

• ✅ Provides explicit cleanup mechanism
• ✅ Can be triggered manually or via scheduled task
• ✅ Follows existing CullProtocol pattern

Trade-offs:

• ⚠️ RocksDB/Registry must scan ALL keys (expensive for large datasets)
• ⚠️ Still requires manual invocation

---

Priority 3: Periodic Culling Wrapper (Low Impact, Medium Complexity)

Problem Solved: Applications that need automatic periodic culling.

Proposal: Create a PeriodicCullingWrapper that schedules background culling:

class PeriodicCullingWrapper(BaseWrapper):
    """Wrapper that periodically calls cull() on stores that support it."""
    
    def __init__(
        self,
        key_value: AsyncKeyValue,
        cull_interval_seconds: float = 3600,  # Default: hourly
        auto_start: bool = True,
    ) -> None:
        super().__init__(key_value=key_value)
        self._cull_interval = cull_interval_seconds
        self._task: asyncio.Task | None = None
        
        if auto_start:
            self.start_culling()
    
    def start_culling(self) -> None:
        """Start background culling task."""
        if isinstance(self.key_value, AsyncCullProtocol):
            self._task = asyncio.create_task(self._cull_loop())
    
    async def _cull_loop(self) -> None:
        """Background task that periodically calls cull()."""
        while True:
            await asyncio.sleep(self._cull_interval)
            if isinstance(self.key_value, AsyncCullProtocol):
                await self.key_value.cull()
    
    async def stop_culling(self) -> None:
        """Stop background culling task."""
        if self._task:
            self._task.cancel()
            try:
                await self._task
            except asyncio.CancelledError:
                pass

Benefits:

• ✅ Automatic periodic cleanup
• ✅ Opt-in via wrapper
• ✅ Configurable interval
• ✅ Works with any CullProtocol-implementing store

Trade-offs:

• ⚠️ Adds background task overhead
• ⚠️ Requires proper lifecycle management (start/stop)

---

Priority 4: Documentation (High Impact, Low Complexity)

Proposal: Add a comprehensive "TTL and Expiration" documentation page covering:

1. How each store handles TTL:

   • Which stores have native automatic culling (no action needed)
   • Which stores support manual culling via cull()
   • Which stores benefit from delete_on_expired_read=True

2. Best Practices:

   • Use native-TTL stores (Redis, DynamoDB, MongoDB) for production when possible
   • Enable delete_on_expired_read for Keyring/WindowsRegistry/RocksDB
   • Schedule periodic cull() calls for Elasticsearch
   • Consider PeriodicCullingWrapper for automatic cleanup

3. Store Comparison Table (like the one in Findings above)

Location: docs/expiration.md

Related Issues and Pull Requests

Repository	Issue or PR	Relevance
strawgate/py-key-value	Review Table Schemas #148	Added MongoDB TTL index for automatic expiration. Demonstrates native TTL approach.
strawgate/py-key-value	Review mongodb for inclusion as stable store #90	Discusses MongoDB TTL index implementation.
strawgate/py-key-value	Review DynamoStore #168	Mentions DynamoDB's native TTL support via TimeToLiveSpecification.
strawgate/py-key-value	Memory Store Expiration and Windows Registry Store Issues #160	Fixed TTL bugs in MemoryStore. Shows TTL handling in memory stores.
strawgate/py-key-value	Review ManagedEntry #165	Discusses is_expired property and how expiration checking works.
strawgate/py-key-value	Store values directly for stores that natively support ttl #88	Proposes leveraging native TTL in stores like MongoDB, DynamoDB.

Related Files

Repository	File	Relevance	Sections
strawgate/py-key-value	aio/stores/base.py	BaseStore implements application-level expiration checking	231-232, 504-520
strawgate/py-key-value	aio/protocols/key_value.py	AsyncCullProtocol definition	128-134
strawgate/py-key-value	aio/stores/elasticsearch/store.py	Only current implementation of culling	537-548
strawgate/py-key-value	aio/stores/mongodb/store.py	TTL index for automatic expiration	234-235
strawgate/py-key-value	aio/stores/dynamodb/store.py	Native DynamoDB TTL configuration	175-181
strawgate/py-key-value	aio/stores/redis/store.py	Redis SETEX for native TTL	138-143
strawgate/py-key-value	aio/stores/windows_registry/store.py	Explicitly notes lack of culling	47-49
strawgate/py-key-value	shared/utils/managed_entry.py	ManagedEntry.is_expired property	29-32

---

Problems Encountered During Investigation

None - all investigation tools and resources were accessible. The codebase is well-organized with clear patterns for TTL handling across different store implementations.

---