Skip to content

Commit b709ca9

Browse files
patriot1burkepatriot1burke
committed
Merge pull request keycloak#2327 from patriot1burke/master
cleanup cache
2 parents 8caad01 + 4be6dc2 commit b709ca9

File tree

6 files changed

+38
-105
lines changed

6 files changed

+38
-105
lines changed

‎model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamCacheRealmProvider.java‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@
6666
* it is added to the cache. So, we keep the version number around for this.
6767
* - In a transaction, objects are registered to be invalidated. If an object is marked for invalidation within a transaction
6868
* a cached object should never be returned. An DB adapter should always be returned.
69-
* - At prepare phase of the transaction, a local lock on the revision cache will be obtained for each object marked for invalidation
70-
* we sort the list of these keys to order local acquisition and avoid deadlocks.
7169
* - After DB commits, the objects marked for invalidation are invalidated, or rather removed from the cache. At this time
7270
* the revision cache entry for this object has its version number bumped.
7371
* - Whenever an object is marked for invalidation, the cache is also searched for any objects that are related to this object
@@ -88,15 +86,20 @@
8886
* - There is a Infinispan @Listener registered. If an invalidation event happens, this is treated like
8987
* the object was removed from the database and will perform evictions based on that assumption.
9088
* - Eviction events will also cascade other evictions, but not assume this is a db removal.
89+
* - With an invalidation cache, if you remove an entry on node 1 and this entry does not exist on node 2, node 2 will not receive a @Listener invalidation event.
90+
* so, hat we have to put a marker entry in the invalidation cache before we read from the DB, so if the DB changes in between reading and adding a cache entry, the cache will be notified and bump
91+
* the version information.
92+
*
93+
* DBs with Repeatable Read:
94+
* - DBs like MySQL are Repeatable Read by default. So, if you query a Client for instance, it will always return the same result in the same transaction even if the DB
95+
* was updated in between these queries. This makes it possible to store stale cache entries. To avoid this problem, this class stores the current local version counter
96+
* at the beginningof the transaction. Whenever an entry is added to the cache, the current coutner is compared against the counter at the beginning of the tx. If the current
97+
* is greater, then don't cache.
9198
*
9299
* Groups and Roles:
93100
* - roles are tricky because of composites. Composite lists are cached too. So, when a role is removed
94101
* we also iterate and invalidate any role or group that contains that role being removed.
95102
*
96-
* - Clustering gotchyas. With an invalidation cache, if you remove an entry on node 1 and this entry does not exist on node 2, node 2 will not receive a @Listener invalidation event.
97-
* so, hat we have to put a marker entry in the invalidation cache before we read from the DB, so if the DB changes in between reading and adding a cache entry, the cache will be notified and bump
98-
* the version information.
99-
*
100103
* - any relationship should be resolved from session.realms(). For example if JPA.getClientByClientId() is invoked,
101104
* JPA should find the id of the client and then call session.realms().getClientById(). THis is to ensure that the cached
102105
* object is invoked and all proper invalidation are being invoked.
@@ -124,14 +127,20 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
124127
protected Set<String> invalidations = new HashSet<>();
125128

126129
protected boolean clearAll;
130+
protected final long startupRevision;
127131

128132
public StreamCacheRealmProvider(StreamRealmCache cache, KeycloakSession session) {
129133
this.cache = cache;
130134
this.session = session;
135+
this.startupRevision = UpdateCounter.current();
131136
session.getTransaction().enlistPrepare(getPrepareTransaction());
132137
session.getTransaction().enlistAfterCompletion(getAfterTransaction());
133138
}
134139

140+
public long getStartupRevision() {
141+
return startupRevision;
142+
}
143+
135144
@Override
136145
public void clear() {
137146
cache.clear();
@@ -305,7 +314,7 @@ public RealmModel getRealm(String id) {
305314
if (model == null) return null;
306315
if (invalidations.contains(id)) return model;
307316
cached = new CachedRealm(loaded, model);
308-
cache.addRevisioned(cached, session);
317+
cache.addRevisioned(cached, startupRevision);
309318
} else if (invalidations.contains(id)) {
310319
return getDelegate().getRealm(id);
311320
} else if (managedRealms.containsKey(id)) {
@@ -329,7 +338,7 @@ public RealmModel getRealmByName(String name) {
329338
if (model == null) return null;
330339
if (invalidations.contains(model.getId())) return model;
331340
query = new RealmListQuery(loaded, cacheKey, model.getId());
332-
cache.addRevisioned(query, session);
341+
cache.addRevisioned(query, startupRevision);
333342
return model;
334343
} else if (invalidations.contains(cacheKey)) {
335344
return getDelegate().getRealmByName(name);
@@ -435,7 +444,7 @@ public List<ClientModel> getClients(RealmModel realm) {
435444
for (ClientModel client : model) ids.add(client.getId());
436445
query = new ClientListQuery(loaded, cacheKey, realm, ids);
437446
logger.tracev("adding realm clients cache miss: realm {0} key {1}", realm.getName(), cacheKey);
438-
cache.addRevisioned(query, session);
447+
cache.addRevisioned(query, startupRevision);
439448
return model;
440449
}
441450
List<ClientModel> list = new LinkedList<>();
@@ -508,7 +517,7 @@ public Set<RoleModel> getRealmRoles(RealmModel realm) {
508517
for (RoleModel role : model) ids.add(role.getId());
509518
query = new RoleListQuery(loaded, cacheKey, realm, ids);
510519
logger.tracev("adding realm roles cache miss: realm {0} key {1}", realm.getName(), cacheKey);
511-
cache.addRevisioned(query, session);
520+
cache.addRevisioned(query, startupRevision);
512521
return model;
513522
}
514523
Set<RoleModel> list = new HashSet<>();
@@ -544,7 +553,7 @@ public Set<RoleModel> getClientRoles(RealmModel realm, ClientModel client) {
544553
for (RoleModel role : model) ids.add(role.getId());
545554
query = new RoleListQuery(loaded, cacheKey, realm, ids, client.getClientId());
546555
logger.tracev("adding client roles cache miss: client {0} key {1}", client.getClientId(), cacheKey);
547-
cache.addRevisioned(query, session);
556+
cache.addRevisioned(query, startupRevision);
548557
return model;
549558
}
550559
Set<RoleModel> list = new HashSet<>();
@@ -593,7 +602,7 @@ public RoleModel getRealmRole(RealmModel realm, String name) {
593602
if (model == null) return null;
594603
query = new RoleListQuery(loaded, cacheKey, realm, model.getId());
595604
logger.tracev("adding realm role cache miss: client {0} key {1}", realm.getName(), cacheKey);
596-
cache.addRevisioned(query, session);
605+
cache.addRevisioned(query, startupRevision);
597606
return model;
598607
}
599608
RoleModel role = getRoleById(query.getRoles().iterator().next(), realm);
@@ -623,7 +632,7 @@ public RoleModel getClientRole(RealmModel realm, ClientModel client, String name
623632
if (model == null) return null;
624633
query = new RoleListQuery(loaded, cacheKey, realm, model.getId(), client.getClientId());
625634
logger.tracev("adding client role cache miss: client {0} key {1}", client.getClientId(), cacheKey);
626-
cache.addRevisioned(query, session);
635+
cache.addRevisioned(query, startupRevision);
627636
return model;
628637
}
629638
RoleModel role = getRoleById(query.getRoles().iterator().next(), realm);
@@ -660,7 +669,7 @@ public RoleModel getRoleById(String id, RealmModel realm) {
660669
} else {
661670
cached = new CachedRealmRole(loaded, model, realm);
662671
}
663-
cache.addRevisioned(cached, session);
672+
cache.addRevisioned(cached, startupRevision);
664673

665674
} else if (invalidations.contains(id)) {
666675
return getDelegate().getRoleById(id, realm);
@@ -685,7 +694,7 @@ public GroupModel getGroupById(String id, RealmModel realm) {
685694
if (model == null) return null;
686695
if (invalidations.contains(id)) return model;
687696
cached = new CachedGroup(loaded, realm, model);
688-
cache.addRevisioned(cached, session);
697+
cache.addRevisioned(cached, startupRevision);
689698

690699
} else if (invalidations.contains(id)) {
691700
return getDelegate().getGroupById(id, realm);
@@ -725,7 +734,7 @@ public List<GroupModel> getGroups(RealmModel realm) {
725734
for (GroupModel client : model) ids.add(client.getId());
726735
query = new GroupListQuery(loaded, cacheKey, realm, ids);
727736
logger.tracev("adding realm getGroups cache miss: realm {0} key {1}", realm.getName(), cacheKey);
728-
cache.addRevisioned(query, session);
737+
cache.addRevisioned(query, startupRevision);
729738
return model;
730739
}
731740
List<GroupModel> list = new LinkedList<>();
@@ -761,7 +770,7 @@ public List<GroupModel> getTopLevelGroups(RealmModel realm) {
761770
for (GroupModel client : model) ids.add(client.getId());
762771
query = new GroupListQuery(loaded, cacheKey, realm, ids);
763772
logger.tracev("adding realm getTopLevelGroups cache miss: realm {0} key {1}", realm.getName(), cacheKey);
764-
cache.addRevisioned(query, session);
773+
cache.addRevisioned(query, startupRevision);
765774
return model;
766775
}
767776
List<GroupModel> list = new LinkedList<>();
@@ -837,7 +846,7 @@ public ClientModel getClientById(String id, RealmModel realm) {
837846
if (invalidations.contains(id)) return model;
838847
cached = new CachedClient(loaded, realm, model);
839848
logger.tracev("adding client by id cache miss: {0}", cached.getClientId());
840-
cache.addRevisioned(cached, session);
849+
cache.addRevisioned(cached, startupRevision);
841850
} else if (invalidations.contains(id)) {
842851
return getDelegate().getClientById(id, realm);
843852
} else if (managedApplications.containsKey(id)) {
@@ -866,7 +875,7 @@ public ClientModel getClientByClientId(String clientId, RealmModel realm) {
866875
id = model.getId();
867876
query = new ClientListQuery(loaded, cacheKey, realm, id);
868877
logger.tracev("adding client by name cache miss: {0}", clientId);
869-
cache.addRevisioned(query, session);
878+
cache.addRevisioned(query, startupRevision);
870879
} else if (invalidations.contains(cacheKey)) {
871880
return getDelegate().getClientByClientId(clientId, realm);
872881
} else {
@@ -895,7 +904,7 @@ public ClientTemplateModel getClientTemplateById(String id, RealmModel realm) {
895904
if (model == null) return null;
896905
if (invalidations.contains(id)) return model;
897906
cached = new CachedClientTemplate(loaded, realm, model);
898-
cache.addRevisioned(cached, session);
907+
cache.addRevisioned(cached, startupRevision);
899908
} else if (invalidations.contains(id)) {
900909
return getDelegate().getClientTemplateById(id, realm);
901910
} else if (managedClientTemplates.containsKey(id)) {

‎model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamRealmCache.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.keycloak.models.cache.infinispan.stream.InClientPredicate;
4040
import org.keycloak.models.cache.infinispan.stream.InRealmPredicate;
4141
import org.keycloak.models.cache.infinispan.stream.RealmQueryPredicate;
42-
import org.keycloak.models.utils.UpdateCounter;
4342

4443
import java.util.HashSet;
4544
import java.util.Iterator;
@@ -124,7 +123,7 @@ protected void bumpVersion(String id) {
124123
Object rev = revisions.put(id, next);
125124
}
126125

127-
public void addRevisioned(Revisioned object, KeycloakSession session) {
126+
public void addRevisioned(Revisioned object, long startupRevision) {
128127
//startRevisionBatch();
129128
String id = object.getId();
130129
try {
@@ -145,9 +144,9 @@ public void addRevisioned(Revisioned object, KeycloakSession session) {
145144
if (id.endsWith("realm.clients")) logger.trace("addRevisioned rev2 == null realm.clients");
146145
return;
147146
}
148-
if (rev > session.getTransaction().getStartupRevision()) { // revision is ahead transaction start. Other transaction updated in the meantime. Don't cache
147+
if (rev > startupRevision) { // revision is ahead transaction start. Other transaction updated in the meantime. Don't cache
149148
if (logger.isTraceEnabled()) {
150-
logger.tracev("Skipped cache. Current revision {0}, Transaction start revision {1}", object.getRevision(), session.getTransaction().getStartupRevision());
149+
logger.tracev("Skipped cache. Current revision {0}, Transaction start revision {1}", object.getRevision(), startupRevision);
151150
}
152151
return;
153152
}

server-spi/src/main/java/org/keycloak/models/utils/UpdateCounter.java renamed to model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UpdateCounter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package org.keycloak.models.utils;
1+
package org.keycloak.models.cache.infinispan;
22

33
import java.util.concurrent.atomic.AtomicLong;
44

0 commit comments

Comments
 (0)