Skip to content

Commit bbf9050

Browse files
thjaeckleclaude
thjaeckle
and
claude
committed
Cache namespace-filtered policy enforcers to fix CPU hotspot
PolicyEnforcer.forNamespace(ns) rebuilt the entire TreeBasedPolicyEnforcer on every enforced signal when a policy had namespace-scoped entries (introduced with #2325 in 3.9.0). A prod JFR profile of ditto-things showed this path (ThingEnforcerActor -> forNamespace -> defaultEvaluator -> createInstance) accounting for ~18% of JVM user CPU. The base PolicyEnforcer is cached per policy and replaced wholesale on every policy update, so forNamespace(ns) is a pure function of the namespace for a given instance. Memoize its result in a per-instance Caffeine cache so the enforcer tree is built at most once per distinct namespace instead of per signal. The memo is invalidated naturally when the instance is replaced. The "return this" fast-path (no entries filtered) is preserved. forNamespace is only ever called by the things-service enforcer on the long-lived, provider-cached instances (from PolicyEnforcerCacheLoader), so only those get a bounded cache whose size is operator-configurable via ditto.policies-enforcer-cache.namespace-filtered-enforcer-max-size (default 100, env DITTO_POLICIES_ENFORCER_NAMESPACE_FILTERED_MAX_SIZE). All other construction paths (of/embed/withResolvedImports, filtered children, and the policies-service enforcers) never accumulate in the cache and use an unbounded (always-empty) one, so no hard-coded default size is needed. Helm values and the policies/things/connectivity deployment templates expose the new setting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 812dff3 commit bbf9050

10 files changed

Lines changed: 111 additions & 22 deletions

File tree

‎deployment/helm/ditto/templates/connectivity-deployment.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,8 @@ spec:
310310
value: "{{ .Values.connectivity.config.policiesEnforcer.cache.enabled }}"
311311
- name: DITTO_POLICIES_ENFORCER_CACHE_MAX_SIZE
312312
value: "{{ .Values.connectivity.config.policiesEnforcer.cache.maxSize }}"
313+
- name: DITTO_POLICIES_ENFORCER_NAMESPACE_FILTERED_MAX_SIZE
314+
value: "{{ .Values.connectivity.config.policiesEnforcer.cache.namespaceFilteredMaxSize }}"
313315
- name: DITTO_POLICIES_ENFORCER_CACHE_EXPIRE_AFTER_WRITE
314316
value: "{{ .Values.connectivity.config.policiesEnforcer.cache.expireAfterWrite }}"
315317
- name: DITTO_POLICIES_ENFORCER_CACHE_EXPIRE_AFTER_ACCESS

‎deployment/helm/ditto/templates/policies-deployment.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,8 @@ spec:
314314
value: "{{ .Values.policies.config.policiesEnforcer.cache.enabled }}"
315315
- name: DITTO_POLICIES_ENFORCER_CACHE_MAX_SIZE
316316
value: "{{ .Values.policies.config.policiesEnforcer.cache.maxSize }}"
317+
- name: DITTO_POLICIES_ENFORCER_NAMESPACE_FILTERED_MAX_SIZE
318+
value: "{{ .Values.policies.config.policiesEnforcer.cache.namespaceFilteredMaxSize }}"
317319
- name: DITTO_POLICIES_ENFORCER_CACHE_EXPIRE_AFTER_WRITE
318320
value: "{{ .Values.policies.config.policiesEnforcer.cache.expireAfterWrite }}"
319321
- name: DITTO_POLICIES_ENFORCER_CACHE_EXPIRE_AFTER_ACCESS

‎deployment/helm/ditto/templates/things-deployment.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,8 @@ spec:
312312
value: "{{ .Values.things.config.policiesEnforcer.cache.enabled }}"
313313
- name: DITTO_POLICIES_ENFORCER_CACHE_MAX_SIZE
314314
value: "{{ .Values.things.config.policiesEnforcer.cache.maxSize }}"
315+
- name: DITTO_POLICIES_ENFORCER_NAMESPACE_FILTERED_MAX_SIZE
316+
value: "{{ .Values.things.config.policiesEnforcer.cache.namespaceFilteredMaxSize }}"
315317
- name: DITTO_POLICIES_ENFORCER_CACHE_EXPIRE_AFTER_WRITE
316318
value: "{{ .Values.things.config.policiesEnforcer.cache.expireAfterWrite }}"
317319
- name: DITTO_POLICIES_ENFORCER_CACHE_EXPIRE_AFTER_ACCESS

‎deployment/helm/ditto/values.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,9 @@ policies:
844844
enabled: true
845845
# maxSize the maximum size of policy enforcers to keep in the cache
846846
maxSize: 1000
847+
# namespaceFilteredMaxSize bounds the per-enforcer cache of namespace-filtered enforcers, avoiding
848+
# rebuilding the enforcer tree per signal for policies with namespace-scoped entries
849+
namespaceFilteredMaxSize: 100
847850
# expireAfterWrite the maximum duration of inconsistency after losing a cache invalidation
848851
expireAfterWrite: 4h
849852
# expireAfterAccess prolonged on each cache access by that duration
@@ -1212,6 +1215,9 @@ things:
12121215
enabled: true
12131216
# maxSize the maximum size of policy enforcers to keep in the cache
12141217
maxSize: 2000
1218+
# namespaceFilteredMaxSize bounds the per-enforcer cache of namespace-filtered enforcers, avoiding
1219+
# rebuilding the enforcer tree per signal for policies with namespace-scoped entries
1220+
namespaceFilteredMaxSize: 100
12151221
# expireAfterWrite the maximum duration of inconsistency after losing a cache invalidation
12161222
expireAfterWrite: 4h
12171223
# expireAfterAccess prolonged on each cache access by that duration
@@ -1920,6 +1926,9 @@ connectivity:
19201926
enabled: true
19211927
# maxSize the maximum size of policy enforcers to keep in the cache
19221928
maxSize: 1000
1929+
# namespaceFilteredMaxSize bounds the per-enforcer cache of namespace-filtered enforcers, avoiding
1930+
# rebuilding the enforcer tree per signal for policies with namespace-scoped entries
1931+
namespaceFilteredMaxSize: 100
19231932
# expireAfterWrite the maximum duration of inconsistency after losing a cache invalidation
19241933
expireAfterWrite: 4h
19251934
# expireAfterAccess prolonged on each cache access by that duration

‎policies/enforcement/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@
7474
<artifactId>ditto-internal-utils-tracing</artifactId>
7575
</dependency>
7676

77+
<dependency>
78+
<groupId>com.github.ben-manes.caffeine</groupId>
79+
<artifactId>caffeine</artifactId>
80+
</dependency>
81+
7782
<!-- ### Test ### -->
7883
<dependency>
7984
<groupId>org.eclipse.ditto</groupId>

‎policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/PolicyEnforcer.java‎

4.25 KB
Binary file not shown.

‎policies/enforcement/src/main/java/org/eclipse/ditto/policies/enforcement/PolicyEnforcerCacheLoader.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,14 @@ public final class PolicyEnforcerCacheLoader implements AsyncCacheLoader<PolicyI
3636

3737
public static final String ENFORCEMENT_CACHE_DISPATCHER = "enforcement-cache-dispatcher";
3838

39+
/** Config key (relative to {@link PolicyEnforcerProvider#ENFORCER_CACHE_CONFIG_KEY}) for the per-enforcer
40+
* namespace-filtered-enforcer cache size; default provided by reference.conf. */
41+
private static final String NAMESPACE_FILTERED_ENFORCER_MAX_SIZE_KEY = "namespace-filtered-enforcer-max-size";
42+
3943
private final PolicyCacheLoader delegate;
4044
private final Executor enforcementCacheExecutor;
4145
private final NamespacePoliciesConfig namespacePoliciesConfig;
46+
private final long namespaceFilteredEnforcerCacheMaxSize;
4247
@Nullable
4348
private final CompletableFuture<Cache<PolicyId, Entry<PolicyEnforcer>>> cacheFuture;
4449

@@ -75,6 +80,9 @@ public PolicyEnforcerCacheLoader(final PolicyCacheLoader policyCacheLoader, fina
7580
delegate = policyCacheLoader;
7681
enforcementCacheExecutor = actorSystem.dispatchers().lookup(ENFORCEMENT_CACHE_DISPATCHER);
7782
this.namespacePoliciesConfig = namespacePoliciesConfig;
83+
this.namespaceFilteredEnforcerCacheMaxSize = actorSystem.settings().config()
84+
.getLong(PolicyEnforcerProvider.ENFORCER_CACHE_CONFIG_KEY + "." +
85+
NAMESPACE_FILTERED_ENFORCER_MAX_SIZE_KEY);
7886
this.cacheFuture = cacheFuture;
7987
}
8088

@@ -106,7 +114,7 @@ private CompletionStage<Entry<PolicyEnforcer>> evaluatePolicy(final Entry<Policy
106114
final var revision = entry.getRevision();
107115
final var policy = entry.getValueOrThrow();
108116
return PolicyEnforcer.withResolvedImportsAndNamespacePolicies(policy, policyResolver,
109-
namespacePoliciesConfig)
117+
namespacePoliciesConfig, namespaceFilteredEnforcerCacheMaxSize)
110118
.thenApply(enforcer -> Entry.of(revision, enforcer));
111119
} else {
112120
return CompletableFuture.completedFuture(Entry.nonexistent());

‎policies/enforcement/src/main/resources/reference.conf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ ditto.policies-enforcer-cache {
4848
maximum-size = 50000
4949
maximum-size = ${?DITTO_POLICIES_ENFORCER_CACHE_MAX_SIZE}
5050

51+
# per-enforcer cache of namespace-filtered enforcers: avoids rebuilding the enforcer tree per signal
52+
# for policies with namespace-scoped entries; bounds the number of distinct namespaces cached per policy
53+
namespace-filtered-enforcer-max-size = 100
54+
namespace-filtered-enforcer-max-size = ${?DITTO_POLICIES_ENFORCER_NAMESPACE_FILTERED_MAX_SIZE}
55+
5156
# maximum duration of inconsistency after losing a cache invalidation
5257
expire-after-write = 1h
5358
expire-after-write = ${?DITTO_POLICIES_ENFORCER_CACHE_EXPIRE_AFTER_WRITE}

‎policies/enforcement/src/test/java/org/eclipse/ditto/policies/enforcement/PolicyEnforcerTest.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,46 @@ public void forNamespaceReturnsSameInstanceWhenNoEntriesAreRestricted() {
8181
assertThat(filtered).isSameAs(original);
8282
}
8383

84+
@Test
85+
public void forNamespaceMemoizesResultForRepeatedCalls() {
86+
final Policy policy = PoliciesModelFactory.newPolicyBuilder(PolicyId.of("test:policy"))
87+
.set(newScopedEntry("restricted", "google:tenant-user",
88+
Arrays.asList("com.acme", "com.acme.*")))
89+
.set(newScopedEntry("global", "google:global-user", Collections.emptyList()))
90+
.build();
91+
final PolicyEnforcer original = PolicyEnforcer.of(policy);
92+
93+
// "org.example" filters out the restricted entry, so a new (filtered) enforcer is built the first
94+
// time. A second call for the same namespace must return the memoized instance, not rebuild the tree.
95+
final PolicyEnforcer first = original.forNamespace("org.example");
96+
final PolicyEnforcer second = original.forNamespace("org.example");
97+
98+
assertThat(first).isNotSameAs(original);
99+
assertThat(second).isSameAs(first);
100+
}
101+
102+
@Test
103+
public void forNamespaceReturnsDistinctEnforcersForDistinctNamespaces() {
104+
final Policy policy = PoliciesModelFactory.newPolicyBuilder(PolicyId.of("test:policy"))
105+
.set(newScopedEntry("restricted", "google:tenant-user",
106+
Arrays.asList("com.acme", "com.acme.*")))
107+
.set(newScopedEntry("global", "google:global-user", Collections.emptyList()))
108+
.build();
109+
final PolicyEnforcer original = PolicyEnforcer.of(policy);
110+
111+
// "org.example" excludes the restricted entry; "com.acme.vehicles" keeps it.
112+
final PolicyEnforcer excluding = original.forNamespace("org.example");
113+
final PolicyEnforcer including = original.forNamespace("com.acme.vehicles");
114+
115+
assertThat(excluding).isNotSameAs(including);
116+
assertThat(excluding.getEnforcer().getSubjectsWithUnrestrictedPermission(
117+
PoliciesResourceType.thingResource("/"), Permission.READ))
118+
.doesNotContain(AuthorizationSubject.newInstance("google:tenant-user"));
119+
assertThat(including.getEnforcer().getSubjectsWithUnrestrictedPermission(
120+
PoliciesResourceType.thingResource("/"), Permission.READ))
121+
.contains(AuthorizationSubject.newInstance("google:tenant-user"));
122+
}
123+
84124
@Test
85125
public void forNamespaceWithNullPolicyReturnsSameInstance() {
86126
final PolicyEnforcer enforcer = PolicyEnforcer.embed(

‎policies/service/src/main/java/org/eclipse/ditto/policies/service/enforcement/PolicyCommandEnforcement.java‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import org.eclipse.ditto.policies.model.signals.commands.query.PolicyQueryCommandResponse;
7171
import org.eclipse.ditto.policies.model.signals.commands.query.RetrievePolicy;
7272
import org.eclipse.ditto.policies.model.signals.commands.query.RetrievePolicyResponse;
73+
import org.jspecify.annotations.NonNull;
7374

7475
/**
7576
* Authorizes {@link PolicyCommand}s and filters {@link PolicyCommandResponse}s.
@@ -315,10 +316,14 @@ private CompletableFuture<Optional<PolicyEnforcer>> resolveSourceEnforcer(final
315316
// admin) fails hasSourceSideRead for every entry in the source and silently loses all imported
316317
// entries in the resolved view.
317318
return policyResolver.apply(sourceId)
318-
.thenCompose(opt -> opt.isPresent()
319-
? PolicyEnforcer.withResolvedImportsAndNamespacePolicies(opt.get(), policyResolver,
319+
.thenCompose(opt -> opt
320+
.map(policyEntries -> PolicyEnforcer.withResolvedImportsAndNamespacePolicies(
321+
policyEntries,
322+
policyResolver,
320323
namespacePoliciesConfig).thenApply(Optional::of)
321-
: CompletableFuture.completedFuture(Optional.<PolicyEnforcer>empty()))
324+
)
325+
.orElseGet(() -> CompletableFuture.completedFuture(Optional.empty()))
326+
)
322327
.toCompletableFuture();
323328
}
324329

@@ -476,27 +481,38 @@ private JsonObject getJsonViewForPolicyQueryCommandResponse(final JsonObject res
476481
*/
477482
private static DittoRuntimeException errorForPolicyCommand(final Signal<?> policySignal) {
478483

479-
if (policySignal instanceof PolicyCommand<?> policyCommand) {
480-
final CommandToExceptionRegistry<PolicyCommand<?>, DittoRuntimeException> registry;
481-
if (policyCommand instanceof PolicyActionCommand) {
482-
registry = PolicyCommandToActionsExceptionRegistry.getInstance();
483-
} else if (policyCommand instanceof PolicyModifyCommand) {
484-
registry = PolicyCommandToModifyExceptionRegistry.getInstance();
485-
} else {
486-
registry = PolicyCommandToAccessExceptionRegistry.getInstance();
484+
switch (policySignal) {
485+
case PolicyCommand<?> policyCommand -> {
486+
final CommandToExceptionRegistry<PolicyCommand<?>, DittoRuntimeException> registry =
487+
getPolicyCommandDittoRuntimeExceptionCommandToExceptionRegistry(policyCommand);
488+
return registry.exceptionFrom(policyCommand);
487489
}
488-
return registry.exceptionFrom(policyCommand);
489-
} else if (policySignal instanceof WithEntityId withEntityId) {
490-
return PolicyNotAccessibleException.newBuilder(PolicyId.of(withEntityId.getEntityId()))
491-
.dittoHeaders(policySignal.getDittoHeaders())
492-
.build();
490+
case WithEntityId withEntityId -> {
491+
return PolicyNotAccessibleException.newBuilder(PolicyId.of(withEntityId.getEntityId()))
492+
.dittoHeaders(policySignal.getDittoHeaders())
493+
.build();
494+
}
495+
default -> {
496+
LOGGER.error("Received signal for which no DittoRuntimeException due to lack of access " +
497+
"could be determined: {}", policySignal);
498+
return DittoInternalErrorException.newBuilder()
499+
.dittoHeaders(policySignal.getDittoHeaders())
500+
.build();
501+
}
502+
}
503+
}
504+
505+
private static CommandToExceptionRegistry<PolicyCommand<?>, DittoRuntimeException>
506+
getPolicyCommandDittoRuntimeExceptionCommandToExceptionRegistry(PolicyCommand<?> policyCommand) {
507+
final CommandToExceptionRegistry<PolicyCommand<?>, DittoRuntimeException> registry;
508+
if (policyCommand instanceof PolicyActionCommand) {
509+
registry = PolicyCommandToActionsExceptionRegistry.getInstance();
510+
} else if (policyCommand instanceof PolicyModifyCommand) {
511+
registry = PolicyCommandToModifyExceptionRegistry.getInstance();
493512
} else {
494-
LOGGER.error("Received signal for which no DittoRuntimeException due to lack of access " +
495-
"could be determined: {}", policySignal);
496-
return DittoInternalErrorException.newBuilder()
497-
.dittoHeaders(policySignal.getDittoHeaders())
498-
.build();
513+
registry = PolicyCommandToAccessExceptionRegistry.getInstance();
499514
}
515+
return registry;
500516
}
501517

502518
/**

0 commit comments

Comments
 (0)