Repository navigation
feat(plugins): limit store namespace size - #774
Merged
Merged
Conversation
Add plugins.defaults.store.max_namespace_size (per plugin in plugins.available.<id>.store; default 16 MiB): each entry counts its key and value in UTF-8 (counters 8 bytes) plus 64 bytes. Writes that would exceed the limit raise StoreFullError, a StoreError; replacing an entry by one that isn't larger, incrementing existing counters and deleting always work. The memory backend counts exactly and removes the namespace's expired entries when it is full. The MongoDB backend keeps the sizes in the collection plugin_store_sizes and each entry's size in a field "size", and counts a namespace again before a process's first write, every 5 minutes and when a write doesn't fit (at most every 10 s), so that entries removed by the TTL index free their space; entries of earlier versions get their size then. The library chart maps maxNamespaceSize (version 0.6.0). Documented in the plugin and configuration docs and the 0.23.0 migration guide. Closes #552 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Counting a namespace again no longer fails writes: one aggregation (entries of earlier versions count 64 bytes, without a backfill), within half the write's time, by one thread per process; failures are logged and retried after the interval. The memory backend sweeps a full namespace at most every 10 s. Docs: a section on how sizes are counted, the rights on plugin_store_sizes, and the breaking change for custom store backends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review fixes. If counting a namespace in the call fails (e.g., a large namespace and a short timeout), a thread of the process counts it in the background (60 s), one at a time per namespace, so that the size doesn't drift up until every write fails. Docs: the examples catch StoreFullError when caching; plugins of a distribution share the limit, so caches keyed by client input can keep new counters out; the limit counts bytes, not memory or storage; writes that failed after their size was added are miscounted until the next count. Release notes of the library chart 0.6.0. Tests of the MongoDB size accounting run on a MongoDB server too, and mongomock tests start from an empty database. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round-2 review fixes. A count corrects the stored size by the difference to the size it read before ($inc instead of $set), so that sizes added while it runs (also by the counting process, during a background count of up to 60 s) aren't lost. Background counts run in a context of their own, so that the call's deadline never applies, and a thread that can't be started is logged instead of failing the write. Docs: reflowed paragraphs on how sizes are counted, the single background attempt, and a note on failing increments in the example. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideSchema and migrationsPersist each MongoDB entry’s counted size and track aggregate namespace sizes in the new size collection so writes can enforce the limit.
Data structuresAdd the namespace limit to store settings and backend contracts, plus the public StoreFullError and entry-size accounting model used across implementations.
APIExpose StoreFullError publicly and pass the effective per-plugin namespace limit into each Store instance’s set and increment calls.
ImplementationEnforce exact in-memory namespace accounting, sweep expired entries when full, and maintain sizes through replacements, deletions, and counter increments.
ConfigurationBump the common Helm library chart and update the gateway’s chart dependencies to expose the new maxNamespaceSize setting.
DocumentationDocument the new plugin-store limit, accounting and operational guidance, migration implications, public reference additions, and chart release.
TestsCover configured limits and size accounting across memory and MongoDB backends, including expiry, concurrency, failures, and configuration behavior.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #774 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 74 74
Lines 9517 9651 +134
==========================================
+ Hits 9517 9651 +134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
uniqueg
marked this pull request as ready for review
November 13, 2026 23:45
Linked issuesChecked at commit b206036 #552 feat(plugins): limit store namespace size · 4 of 4 met
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 9 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="foca/plugins/store.py" line_range="725-738" />
<code_context>
+ pymongo.errors.PyMongoError: The database failed or was too
+ slow.
+ """
+ kept = self.sizes.find_one({"_id": ns})
+ before = int(kept.get("size", 0)) if kept is not None else 0
+ size = {"$ifNull": ["$size", STORE_ENTRY_OVERHEAD]}
+ counted = list(
+ self.collection.aggregate(
+ [
+ {"$match": {"ns": ns}},
+ {"$group": {"_id": None, "size": {"$sum": size}}},
+ ]
+ )
+ )
+ total = int(counted[0]["size"]) if counted else 0
+ self.sizes.update_one(
+ {"_id": ns}, {"$inc": {"size": total - before}}, upsert=True
+ )
+ return total
</code_context>
<issue_to_address>
**Concurrent recounts corrupt size totals**
When multiple processes recount the same namespace concurrently, including when an in-call recount fails while its background recount is still running, each `recount()` reads the same prior size and applies its independently computed `total - before` with `$inc`. The correction is applied more than once, leaving the size too high or too low; writes that fit are refused or writes beyond the limit are admitted.
Make recount updates concurrency-safe so overlapping recounts cannot apply the same correction more than once.
Also at `foca/plugins/store.py:763`.
</issue_to_address>
### Comment 2
<location path="foca/plugins/store.py" line_range="866-867" />
<code_context>
+ slow.
+ """
+ self._recount_if_due(ns, self._RECOUNT_INTERVAL, timeout)
+ if max_size is None or delta <= 0:
+ self._adjust(ns, delta)
+ return
+ if delta <= max_size:
</code_context>
<issue_to_address>
**Failed replacements undercount namespace size**
When `set()` replaces a larger entry with a smaller one and the MongoDB replacement fails before taking effect, `_reserve` subtracts the size reduction before the replacement succeeds, leaving the old larger entry in place while `plugin_store_sizes` is lower. Subsequent writes can exceed the namespace limit.
Apply the size reduction only after a successful replacement, or restore it when the replacement fails.
</issue_to_address>
### Comment 3
<location path="foca/plugins/store.py" line_range="1125-1128" />
<code_context>
+ """
+ if document is None:
+ return 0
+ size = document.get("size")
+ if isinstance(size, int) and not isinstance(size, bool):
+ return size
+ return STORE_ENTRY_OVERHEAD
+
+
</code_context>
<issue_to_address>
**Old writers bypass the namespace limit**
When old and new application versions share the database during a rolling deployment and an old-version pod writes to a namespace after a new process has counted it, old processes do not update `plugin_store_sizes` or store the new `size` field, so new processes enforce the limit against a stale count. The namespace can exceed its configured limit during the rollout.
Ensure old-version writes are accounted for during the rollout, or prevent mixed-version writers until the new size accounting is in use.
Also at `foca/plugins/store.py:966`.
</issue_to_address>
### Comment 4
<location path="foca/plugins/store.py" line_range="761-762" />
<code_context>
+ now = time.monotonic()
+ with self._counted_lock:
+ counted = self._counted.get(ns)
+ if counted is not None and now - counted < interval:
+ return False
+ self._counted[ns] = now
+ try:
</code_context>
<issue_to_address>
**Deleted size documents stay undetected**
When an operator deletes a namespace's size document while a process still considers its last recount recent, `_recount_if_due` skips the recount, and `_grow` recreates the missing size document using only the incoming delta. Existing entries are omitted from the total, so later writes can exceed the namespace limit.
Detect a missing size document and recount the namespace before reserving space for another write.
Also at `foca/plugins/store.py:894-898`.
</issue_to_address>
### Comment 5
<location path="tests/plugins/test_plugin_store.py" line_range="533" />
<code_context>
+ def test_too_large(self, backend: StoreBackend) -> None:
+ """Entries larger than the namespace never fit."""
+ store = make_store(backend, max_namespace_size=SMALL - 1)
+ with pytest.raises(StoreFullError, match=f"entry of {SMALL} bytes"):
+ store.set("a", 1)
+
</code_context>
<issue_to_address>
**Oversized write can persist unnoticed**
When an oversized write is committed before `StoreFullError` is raised, when `set()` stores the oversized entry before raising, `test_too_large` passes on the expected `StoreFullError` and never checks the store afterward; the caller sees a failed write whose value was actually saved.
Assert that `store.get("a") is None` after the exception.
</issue_to_address>
### Comment 6
<location path="foca/plugins/store.py" line_range="971" />
<code_context>
- {"_id": ident}, document, upsert=True
+ known = self.collection.find_one({"_id": ident}, projection=_SIZE_FIELDS)
+ self._reserve(ns, size, size - _document_size(known), max_size, timeout)
+ previous = _retry_upsert(
+ lambda: self.collection.find_one_and_replace(
+ {"_id": ident},
</code_context>
<issue_to_address>
**Concurrent same-key writes fail**
When the namespace has room for one entry and concurrent `set()` or `increment()` calls both observe the same absent key before either write becomes visible, `_reserve` charges each caller for the absent key before either write is visible; when the first reservation fills the namespace, the next caller raises `StoreFullError` before reaching the duplicate-key handling, although both writes would share one entry that fits.
Make reservations idempotent per key, or recheck and retry the key when a reservation fails so concurrent writes share one entry's capacity.
Also at `foca/plugins/store.py:1088`.
</issue_to_address>
### Comment 7
<location path="foca/plugins/store.py" line_range="969-980" />
<code_context>
- _retry_upsert(
- lambda: self.collection.replace_one(
- {"_id": ident}, document, upsert=True
+ known = self.collection.find_one({"_id": ident}, projection=_SIZE_FIELDS)
+ self._reserve(ns, size, size - _document_size(known), max_size, timeout)
+ previous = _retry_upsert(
+ lambda: self.collection.find_one_and_replace(
+ {"_id": ident},
+ document,
+ projection=_SIZE_FIELDS,
+ upsert=True,
)
)
+ # if the entry changed since it was read
+ self._adjust(ns, _document_size(known) - _document_size(previous))
def delete(self, ns: str, key: str, now: datetime, timeout: float) -> bool:
</code_context>
<issue_to_address>
**TTL cleanup inflates namespace size**
When mongoDB’s TTL monitor deletes an expired entry between the pre-read and the replacement or insertion, the size correction assumes the expired document is still present when replacement returns; if MongoDB’s TTL monitor removes it after the pre-read, `set()` or `increment()` adds its old size again even though the size document still counts it, so later writes can be refused with `StoreFullError`.
Account for TTL deletion in the reconciliation so the old entry’s size is not added again when its document is already absent.
Also at `foca/plugins/store.py:1085-1108`.
</issue_to_address>
### Comment 8
<location path="foca/plugins/store.py" line_range="1081" />
<code_context>
+ "key": key,
+ "value": value,
+ "expires_at": expires_at,
+ "size": size,
+ }
with _guard(timeout):
</code_context>
<issue_to_address>
**Incremented legacy counters stay undercounted**
When a pre-0.23.0 counter is incremented while it is still live, the live-counter path in `MongoStoreBackend.increment` updates only `n`, leaving the legacy document without `size`; subsequent recounts therefore continue counting it as only 64 bytes, so repeated live increments let the namespace exceed its configured limit by the counter key and value sizes.
When incrementing a legacy counter, populate its `size` and adjust the namespace total to account for the difference from the legacy 64-byte count.
</issue_to_address>
### Comment 9
<location path="tests/plugins/test_plugin_store.py" line_range="859" />
<code_context>
+ """A size inserted concurrently is grown, if it stays within bounds."""
+ backend = mongo
+ sizes = MagicMock()
+ sizes.update_one.side_effect = [
+ DuplicateKeyError("E11000"),
+ MagicMock(matched_count=1),
</code_context>
<issue_to_address>
**Concurrent limit check goes untested**
When the duplicate-key retry drops the size-bound filter, `test_concurrent_size_insert` asserts the mocked retry result, but the mock returns `matched_count=0` regardless of the query. A retry that updates by namespace only, ignoring the size limit, therefore passes this test while real MongoDB grows the namespace beyond its limit.
Assert that the retry query includes the size bound, or use a database-backed test that verifies the size remains within the limit.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 9 findings to address first, and a faulty size count can leave the persisted plugin_store_sizes value inaccurate, causing later writes to be rejected or the limit to be misapplied; recounting or resetting that metadata repairs it. Reverting restores the prior write behavior, though the added size metadata remains in MongoDB.
Blocking findings: foca/plugins/store.py:738, foca/plugins/store.py:867, foca/plugins/store.py:1128, foca/plugins/store.py:762, tests/plugins/test_plugin_store.py:533, and 4 more
Automated review fixes. A count's correction applies only if the size is still the generation it read (field gen), so that concurrent counts of this or other processes correct it once. Writes that shrink a namespace subtract only after they succeeded. A write that finds the size absent (e.g., deleted by an operator) makes the process count the namespace again at its next write. Tests: concurrent counts, a deleted size, a failed shrinking write, oversized writes not stored, bounded retries. The 0.23.0 migration guide notes rolling upgrades. Co-Authored-By: Claude Opus 5.5 <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
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.
Summary
Limits the size of each namespace of the plugin store (
ctx.store):max_namespace_size(bytes; default 16 MiB) inplugins.defaults.storeand per plugin inplugins.available.<id>.store; Helm library chart keymaxNamespaceSize(chart 0.6.0, with release notes).set(), andincrement()of a new counter, raisefoca.plugins.StoreFullError(aStoreError) if the entry doesn't fit; replacing an entry by one that isn't larger, incrementing existing counters and deleting are never refused.plugin_store_sizes, updated with$incon writes and deletes (conditional on the limit), each entry's size in a fieldsize. Each process counts a namespace again (one aggregation on{ns: 1}) before its first write, every 5 minutes, and when a write doesn't fit (at most every 10 s), as TTL expiry leaves the counts too high. A count corrects the size by the difference to what it read before ($inc), so that writes while it runs aren't lost. If a count fails in the call (it gets half the time left), the write goes on, and a thread counts the namespace in the background (60 s, outside the call's deadline).StoreFullError), API reference, the 0.23.0 migration guide (incl. the breaking change for customStoreBackendimplementations, which now getmax_size), and the chart's 0.6.0 release notes.Deviation from the proposal
The issue proposed LRU eviction for the in-memory store. Both backends instead refuse writes that don't fit (
StoreFullError): eviction would silently drop counters too, resetting budgets such as per-user quotas, and refusing keeps both backends' behaviour the same for plugins.Verification
ruff check,ruff format --check,mypy: pass.ast-grep scan --error=flask-import,ast-grep test --skip-snapshot-tests: pass.mkdocs build --strict: pass.coverage run -m pytest: all tests that don't need a MongoDB server pass;foca/plugins/*at 100% (the remaining gaps locally are lines covered by the MongoDB-only tests). No MongoDB on port 12345 locally, so themongodb-marked tests (incl. the new real-server variants of the size accounting tests) run in CI only.tes.routing.distanceplugin of foca-plugins (feat(distance): use plugin store for cache, budget foca-plugins#79) passes its tests (100% coverage) with this branch installed.Declined review findings
_reserve: a timed-out write may still have been applied, so giving back could under-count; the next count corrects either way (documented).MemoryStoreBackend._writeinto methods, move_document_size, and avoid private attributes in tests: style only; the tests follow the surrounding tests' style.Open questions
None.
Closes #552
🤖 Generated with Claude Code