Memoize per-resource READ subject classification in PolicyEnforcer (per-event enforcement CPU hotspot)#2484
Open
thjaeckle wants to merge 1 commit into
Conversation
The things-service per-event read-authorization (ThingCommandEnforcement.addEffectedReadSubjectsToThingSignal) and read-grant collection (ReadGrantCollector) walk the full policy tree via Enforcer.classifySubjects on every emitted ThingEvent. After the forNamespace fix removed the per-signal enforcer *rebuild*, this per-event *evaluation* became the dominant things-service enforcement CPU consumer on prod JFR. classifySubjects(resource, READ) is a pure function of the resolved policy grants and the resource path -- independent of Thing field values -- so its result is memoizable per PolicyEnforcer instance. The instance is replaced wholesale by CachingPolicyEnforcerProvider on any grant change, invalidating the memo naturally (same lifetime/invalidation as the existing forNamespace and root-read-classification memos). - PolicyEnforcer: generalize the single-slot root-read-classification memo into a bounded per-ResourceKey Caffeine cache exposed as classifyReadSubjects(ResourceKey); getRootResourceReadClassification() delegates to it. Bound configurable via ditto.policies-enforcer-cache.read-classification-cache-max-size (default 1000), plumbed through PolicyEnforcerCacheLoader + helm. - Route addEffectedReadSubjectsToThingSignal and ReadGrantCollector.collectSubjectsForPointer through the memo. The read-granted-subjects set is identical to the previous formula (getSubjectsWithPartialPermission(root) == partialOnly + unrestricted). JMH (ClassifyReadSubjectsBenchmark): repeated-resource hit 2.12 -> 0.024 us/op (~88x); no-repeat miss stays within noise of baseline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
System tests passed: https://github.com/eclipse-ditto/ditto/actions/runs/28870858118 |
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.
What
Adds a per-resource memo of
Enforcer.classifySubjects(resource, READ)onPolicyEnforcer, removing a full policy-tree walk that ran on every emittedThingEvent.Why — analysis
Profiling
ditto-thingson production (JFR) showed that, once the recentPolicyEnforcer.forNamespace(...)caching fix removed the per-signal enforcer tree rebuild, the per-event read-subject evaluation became the dominant things-service enforcement CPU consumer. Two hot paths re-walk the fullTreeBasedPolicyEnforceron every event:ThingCommandEnforcement.addEffectedReadSubjectsToThingSignalcomputes thereadGrantedSubjectsfor every authorized signal (used for pub/sub routing). Withpartial-access-eventsenabled (the default) it walks the policy tree per event viagetSubjectsWithUnrestrictedPermission(eventResource)andgetSubjectsWithPartialPermission(root).ReadGrantCollector.collectSubjectsForPointercallsclassifySubjects(...)per pointer, per event.classifySubjects(resource, READ)is a pure function of the resolved policy grants and the resource path — it does not depend on the Thing's field values. IoT workloads are dominated by value-only telemetry updates that repeatedly touch the same resource paths, so the identical classification was recomputed on every event.What changed
PolicyEnforcer: generalize the existing single-slot root-read-classification memo into a bounded per-ResourceKeycache, exposed asclassifyReadSubjects(ResourceKey);getRootResourceReadClassification()now delegates to it. The memo lives on thePolicyEnforcerinstance, whichCachingPolicyEnforcerProviderreplaces wholesale on any grant change (own policy, an imported policy, or a namespace-root cascade), so it is invalidated naturally — the same lifetime and invalidation story as theforNamespacememo. The bound is propagated toforNamespacefiltered children, since that is the instance the things hot path enforces against.addEffectedReadSubjectsToThingSignalandReadGrantCollector.collectSubjectsForPointerthrough the memo. The resultingreadGrantedSubjectsset is identical to the previous formula, because at the rootgetSubjectsWithPartialPermission(root, READ) == partialOnly ∪ unrestricted(seeClassifySubjectsVisitor).ditto.policies-enforcer-cache.read-classification-cache-max-size(default1000), plumbed throughPolicyEnforcerCacheLoaderand the Helm chart (things/policies/connectivity).Correctness
classifyReadSubjectsreturns the sameSubjectClassificationas a directEnforcer.classifySubjects(resource, READ); a cache miss/eviction simply recomputes an equal result, never a wrong one. Behaviour preservation is covered by unit tests asserting the emittedreadGrantedSubjectsare identical to the pre-change formula, both with and without partial-read subjects, plus tests for the memo itself (equality with the direct call, memoization, correctness under a bounded cache).Performance — JMH
ClassifyReadSubjectsBenchmark(added),AverageTime, µs/op:baselineRecompute(previous behaviour)memoizedHit(repeated resource, prod shape)memoizedMiss(no-repeat + tiny cache)endToEndMemoized(full add-read-subjects)The repeated-resource case is the production shape (value-only telemetry re-touching the same paths); the pathological no-repeat case (64 distinct keys against a size-4 cache) shows the memo does not regress the worst case.
Memory
The memo adds a small, bounded retained set —
SubjectClassificationis a set of authorization-subject ids, capped at the configured bound per enforcer (and propagated toforNamespacechildren so it cannot grow unbounded). In exchange it removes the per-event transient allocation of the tree walk (temporary sets and visitor state), reducing young-gen allocation churn. The bound is operator-tunable.