fix: bound _serial_cache lifetime to prevent unbounded growth (#62)#64
Merged
Conversation
The conflict-resolution cache was a plain dict with no eviction or clearing. It grew monotonically for the life of the storage instance, driving memory pressure on long-running pods (estimated ~3.8 GB leaked per pod after 24 hours of moderate traffic at 5 threads). Two complementary fixes: - History-preserving: swap for _NoopSerialCache. object_history already serves _do_loadSerial, so the cache is redundant. Zero memory cost. - History-free: clear on afterCompletion. Base versions needed by tryToResolveConflict are consumed during tpc_vote within the same transaction, so bounding the cache lifetime to the enclosing tx is safe. Applies to both PGJsonbStorageInstance and main PGJsonbStorage. No new config, no API change. Co-Authored-By: Claude Opus 4.7 (1M context) <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
1 participant
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.
Closes #62.
Summary
The
_serial_cacheon bothPGJsonbStorageInstanceand the mainPGJsonbStoragewas a plaindictwith no eviction and no clearing — it grew monotonically for the life of the storage instance. On long-running pods (5 threads × moderate traffic) the leak was estimated at ~3.8 GB after 24 hours in #62, matching observed drift toward pod memory limits.Fix
Two complementary fixes, both keep the existing conflict-resolution behaviour intact:
_serial_cache = {}for a new_NoopSerialCache(drop-in dict interface that silently drops writes and always misses on reads)._do_loadSerialretrieves old revisions fromobject_historydirectly in this mode, so the cache is redundant.afterCompletion. The base versions needed bytryToResolveConflictare consumed duringtpc_votewithin the same transaction, so bounding the cache lifetime to the enclosing tx is safe.Applied symmetrically to both
PGJsonbStorageInstanceandPGJsonbStorage. No new config knob, no API change.Test plan
New
tests/test_serial_cache.pywith 4 tests covering both invariants:_serial_cachestays at size 0 after arbitrary loadsafterCompletionRisk
Minimal. The only behavioural change is that cross-transaction conflict resolution in history-free mode no longer finds its base version in a stale in-memory cache — but that path was already broken (the row has been overwritten in PG by the time it matters), so this is documentation of existing behaviour, not a new limitation.
🤖 Generated with Claude Code