Skip to content

Commit 4fa7167

Browse files
thjaecklethjaeckle
authored
Merge pull request #2480 from beyonnex-io/bugfix/policy-enforcer-namespace-cache-cpu-hotspot
Cache namespace-filtered policy enforcers to fix CPU hotspot
2 parents 124ad37 + 9f05e12 commit 4fa7167

11 files changed

Lines changed: 111 additions & 23 deletions

File tree

‎deployment/helm/ditto/Chart.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ description: |
1616
A digital twin is a virtual, cloud based, representation of his real world counterpart
1717
(real world “Things”, e.g. devices like sensors, smart heating, connected cars, smart grids, EV charging stations etc).
1818
type: application
19-
version: 4.2.1 # chart version is effectively set by release-job
19+
version: 4.2.2 # chart version is effectively set by release-job
2020
appVersion: 3.9.2
2121
keywords:
2222
- iot-chart

‎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(

0 commit comments

Comments
 (0)