From 345d6ed5c26ec71ccd2f33e5275c46c3d2043861 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 5 Feb 2024 10:15:38 -0600 Subject: [PATCH] chore(core): Redis Improvements #26932 (#26974) * #26932 fixes for redis * #26932 now works locally * #26932 fixing a test issue --------- Co-authored-by: Freddy Montes <751424+fmontes@users.noreply.github.com> --- .../lettuce/MasterReplicaLettuceClient.java | 19 +- .../cache/lettuce/NullLettuceClient.java | 5 + .../com/dotcms/cache/lettuce/RedisCache.java | 355 ++++++------------ .../com/dotcms/cache/lettuce/RedisClient.java | 6 + .../cache/lettuce/LettuceCacheTest.java | 31 +- 5 files changed, 153 insertions(+), 263 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/cache/lettuce/MasterReplicaLettuceClient.java b/dotCMS/src/main/java/com/dotcms/cache/lettuce/MasterReplicaLettuceClient.java index 867c090f60a3..08ae304a1778 100644 --- a/dotCMS/src/main/java/com/dotcms/cache/lettuce/MasterReplicaLettuceClient.java +++ b/dotCMS/src/main/java/com/dotcms/cache/lettuce/MasterReplicaLettuceClient.java @@ -59,6 +59,8 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import static io.lettuce.core.ScriptOutputType.STATUS; + /** * Master replica implementation of redis cache. It works as a replicator when there is more than 1 URIs as part of the * {@code REDIS_LETTUCECLIENT_URLS} config. This implementation wraps keys, members and channels by prefixing them with @@ -152,7 +154,7 @@ protected List createRedisConnection() { .map(RedisURI::create) .collect(Collectors.toList()); } - final String redisSessionEnabled = envVarService.getenv().getOrDefault("TOMCAT_REDIS_SESSION_ENABLED", "false"); + final String redisSessionEnabled = envVarService.getenv().getOrDefault("TOMCAT_REDIS_SESSION_ENABLED", "false"); if (Boolean.parseBoolean(redisSessionEnabled)) { final RedisURI redisURI = RedisURI.builder() .withHost(envVarService.getenv().getOrDefault("TOMCAT_REDIS_SESSION_HOST", "localhost")) @@ -164,7 +166,7 @@ protected List createRedisConnection() { .build(); return List.of(redisURI); } - return List.of(RedisURI.create("redis://password@oboxturbo")); + return List.of(RedisURI.create("redis://localhost")); } /** @@ -900,6 +902,19 @@ public Collection getChannels() { return channelReferenceMap.keySet().stream().map(this::unwrapKey).collect(Collectors.toList()); } + @Override + public void deleteFromPattern(final String pattern) { + + try (StatefulRedisConnection conn = this.getConn()) { + + if (this.isOpen(conn)) { + + conn.async().eval("return redis.call('del', unpack(redis.call('keys', '" + pattern + "')))", + STATUS, new String[0]); + } + } + } + @Override public Future publishMessage (final V message, final K channelIn) { diff --git a/dotCMS/src/main/java/com/dotcms/cache/lettuce/NullLettuceClient.java b/dotCMS/src/main/java/com/dotcms/cache/lettuce/NullLettuceClient.java index 87e76434e338..99b4b2a83ff9 100644 --- a/dotCMS/src/main/java/com/dotcms/cache/lettuce/NullLettuceClient.java +++ b/dotCMS/src/main/java/com/dotcms/cache/lettuce/NullLettuceClient.java @@ -170,6 +170,11 @@ public long getIncrement (final Object key) { return -1; } + @Override + public void deleteFromPattern(String pattern) { + + } + @Override public void scanKeys(String matchesPattern, int keyBatchingSize, Consumer keyConsumer) { diff --git a/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisCache.java b/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisCache.java index 172f8b434b26..787f7d85fa79 100644 --- a/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisCache.java +++ b/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisCache.java @@ -1,6 +1,5 @@ package com.dotcms.cache.lettuce; -import com.dotcms.concurrent.DotConcurrentFactory; import com.dotcms.util.DotCloneable; import com.dotmarketing.business.cache.provider.CacheProvider; import com.dotmarketing.business.cache.provider.CacheProviderStats; @@ -10,7 +9,6 @@ import com.dotmarketing.util.Logger; import com.dotmarketing.util.UtilMethods; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Objects; import com.liferay.util.StringPool; import io.lettuce.core.ScriptOutputType; import io.lettuce.core.api.StatefulRedisConnection; @@ -20,8 +18,6 @@ import java.io.Serializable; import java.text.DecimalFormat; import java.text.NumberFormat; -import java.text.SimpleDateFormat; -import java.util.Date; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; @@ -29,48 +25,43 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; -import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; /** - * Redis Cache implementation - * Notes: - * 1) This redis implementation does not have the clusted id, b/c redis handles each member, key, hash, incr and channel by using the cluster id - * so it is not need to repeat it here. - * - * The cache handles a ttl, one global by default REDIS_SERVER_DEFAULT_TTL, but can prefix a group to the same key in order to have a specific - * ttl for a section. - * - * 2) When a call is running on transaction to avoid dirty saves or fetches, the cache does not put or retrieve anything. - * - * 3) Since the information is being stored as a java byte (binary) only can use serializable values, non-serializable objects will be skipped from the cache. - * - * 4) Objects that implements {@link DotCloneable}, the cache will returns a Clone of the object stored on the cache instead of the actual copy on the cache - * this will helps - * + * Redis Cache implementation Notes: 1) This redis implementation does not have the clusted id, b/c redis handles each + * member, key, hash, incr and channel by using the cluster id so it is not need to repeat it here. + *

+ * The cache handles a ttl, one global by default REDIS_SERVER_DEFAULT_TTL, but can prefix a group to the same key in + * order to have a specific ttl for a section. + *

+ * 2) When a call is running on transaction to avoid dirty saves or fetches, the cache does not put or retrieve + * anything. + *

+ * 3) Since the information is being stored as a java byte (binary) only can use serializable values, non-serializable + * objects will be skipped from the cache. + *

+ * 4) Objects that implements {@link DotCloneable}, the cache will returns a Clone of the object stored on the cache + * instead of the actual copy on the cache this will helps */ public class RedisCache extends CacheProvider { private static final long serialVersionUID = -855583393078878276L; private static final Long ZERO = 0L; - private static final String KEY_DATE_FORMAT = "yyyy-MM-dd_hh:mm:ss.S"; - private static final String PREFIX_UNSET = "PREFIX_UNSET"; - private final String REDIS_GROUP_KEY; - private final String REDIS_PREFIX_KEY; + protected final String REDIS_GROUP_KEY; + protected final String REDIS_PREFIX_KEY; private final Lazy> client; - private final int keyBatchingSize = Config.getIntProperty( "REDIS_SERVER_KEY_BATCH_SIZE", 1000); - private final long defaultTTL = Config.getLongProperty("REDIS_SERVER_DEFAULT_TTL", -1); - final static AtomicReference prefixKey = new AtomicReference(PREFIX_UNSET); - private final Map groupTTLMap = new ConcurrentHashMap<>(); + private final int keyBatchingSize = Config.getIntProperty("REDIS_SERVER_KEY_BATCH_SIZE", 1000); + private final long defaultTTL = Config.getLongProperty("REDIS_SERVER_DEFAULT_TTL", -1); + private final Map groupTTLMap = new ConcurrentHashMap<>(); - public RedisCache(final Lazy> client) { + public RedisCache(final Lazy> client) { - this.client = client; - this.REDIS_GROUP_KEY = "REDIS_GROUP_KEY"; - this.REDIS_PREFIX_KEY = "REDIS_PREFIX_KEY"; + this.client = client; + this.REDIS_GROUP_KEY = "GROUP_KEY"; + this.REDIS_PREFIX_KEY = "PREFIX_KEY"; } public RedisCache() { @@ -92,111 +83,30 @@ public boolean isDistributed() { return true; } - protected RedisClient getClient() { + protected RedisClient getClient() { return client.get(); } - /** - * all key values that are put into redis are prefixed by a random key. When a flushall is called, - * this key is cycled, which basically invalidates all cached entries. The prefix key is stored in - * redis itself so multiple servers in the cluster can read it. When the key is cycled, we send a - * CYCLE_KEY event to cluster to refresh the key across cluster - */ - @VisibleForTesting - String loadPrefix() { - - String key = prefixKey.get(); - if(!PREFIX_UNSET.equals(key)) { - return key; - } - if (PREFIX_UNSET.equals(key)) { - key = this.loadPrefixFromRedis(); - } - if (PREFIX_UNSET.equals(key)) { - key = setOrGet(); - } - if (PREFIX_UNSET.equals(key)) { - Logger.warn(this.getClass(), "unable to determine key prefix"); - key = PREFIX_UNSET; - } - prefixKey.set(key); - return key; - } - - String loadPrefixFromRedis() { - - try { - - final String value = (String)this.getClient().get(REDIS_PREFIX_KEY); - return null == value? PREFIX_UNSET: value; - } catch (Exception e) { - - Logger.debug(this.getClass(), ()-> "unable to get prefix:" + e.getMessage()); - return PREFIX_UNSET; - } - } - - String generateNewKey() { - - return "cache_" + new SimpleDateFormat(KEY_DATE_FORMAT).format(new Date()); - } - - /** - * This will forceably cycle the prefix key, effectivily wiping the cache. - * - * @return - */ - @VisibleForTesting - String cycleKey() { - - final String newKey = this.generateNewKey(); - - if (SetResult.SUCCESS != this.getClient().set(REDIS_PREFIX_KEY, newKey)) { - return PREFIX_UNSET; - } - - prefixKey.set(newKey); - return newKey; - } - - /** - * This checks if there is a prefix key in redis. If so, it will return the value stored in redis, - * otherwise, it will set redis to the value of the newKey and return it. - * - * @return - */ - @VisibleForTesting - String setOrGet() { - - final String newKey = this.generateNewKey(); - final SetResult result = getClient().setIfAbsent(REDIS_PREFIX_KEY, newKey); - - if (SetResult.NO_CONN == result) { - return PREFIX_UNSET; - } - - return SetResult.SUCCESS == result? - newKey : this.loadPrefixFromRedis(); - } - /** * returns a cache key - * + * * @param group * @param key * @return */ @VisibleForTesting String cacheKey(final String group, final String key) { - return loadPrefix() + - ( - group != null && key != null ? - "." + group + "." + key : - group != null ? - "." + group + "." : - "." - ); + return this.REDIS_PREFIX_KEY + + "." + + ( + group != null && key != null + ? group + "." + key + : group != null + ? group + : "" + ) + + "."; } @VisibleForTesting @@ -208,15 +118,16 @@ String cacheKey(final String group) { public void init() { Logger.info(this.getClass(), "*** Initializing [" + getName() + "]."); - Logger.info(this.getClass(), " prefix [" + this.loadPrefix() + "]"); + Logger.info(this.getClass(), " prefix [" + this.REDIS_PREFIX_KEY + "]"); Logger.info(this.getClass(), " inited [" + this.isInitialized() + "]"); Logger.info(this.getClass(), "*** Initialized [" + getName() + "]."); } @Override - public boolean isInitialized() { + public boolean isInitialized() { - return !PREFIX_UNSET.equals(prefixKey.get()); + return Try.of(() -> this.client.get().ping()).onFailure(e -> Logger.warn(RedisCache.class, e.getMessage())) + .getOrElse(false); } @Override @@ -224,47 +135,46 @@ public void put(final String group, final String key, final Object content) { // this avoid mutability and dirty cache issues if (DbConnectionFactory.inTransaction()) { - - Logger.debug(this, ()-> "In Transaction, Skipping the put to Redis cache for group: " + Logger.debug(this, () -> "In Transaction, Skipping the put to Redis cache for group: " + + group + "key: " + key); + return; + } + if (key == null || group == null || !(content instanceof Serializable)) { + Logger.debug(this, () -> "The content: " + (null != content ? content.getClass() : "unknown") + + " is not serialize, Skipping the put to Redis cache for group: " + group + "key: " + key); - } else if (key != null && group != null) { - - if (content instanceof Serializable) { - - Logger.debug(this, () -> "Redis, putting group: " + group + "key" + key); - final long ttl = this.getTTL(group); - final String cacheKey = this.cacheKey(group, key); - final Future future = this.getClient().setAsync(cacheKey, content, ttl); - this.getClient().addAsyncMembers(REDIS_GROUP_KEY, group); - if (Logger.isDebugEnabled(this.getClass())) { - - String msg = "Error"; - try { - msg = future.get(); - } catch (InterruptedException | ExecutionException e) { - msg = e.getMessage(); - } - if (!"OK".equalsIgnoreCase(msg)) { - Logger.debug(this, "Redis, putting group: " + group + - "key" + key + "result: " + msg); - } - } - } else { - - Logger.debug(this, ()-> "The content: " + (null != content?content.getClass():"unknown") + - " is not serialize, Skipping the put to Redis cache for group: " - + group + "key: " + key ); + return; + } + + Logger.debug(this, () -> "Redis, putting group: " + group + "key" + key); + final long ttl = this.getTTL(group); + final String cacheKey = this.cacheKey(group, key); + final Future future = this.getClient().setAsync(cacheKey, content, ttl); + this.getClient().addAsyncMembers(REDIS_GROUP_KEY, group); + if (Logger.isDebugEnabled(this.getClass())) { + + String msg = "Error"; + try { + msg = future.get(); + } catch (InterruptedException | ExecutionException e) { + msg = e.getMessage(); + } + if (!"OK".equalsIgnoreCase(msg)) { + Logger.debug(this, "Redis, putting group: " + group + + "key" + key + "result: " + msg); } } + + } - private long getTTL (final String group) { + private long getTTL(final String group) { // try to figured out if any time out by group, otherwise uses the default ttl - final String groupTTLKey = group + "_REDIS_SERVER_DEFAULT_TTL"; + final String groupTTLKey = "REDIS_SERVER_DEFAULT_TTL_" + group; final long ttl = this.groupTTLMap.computeIfAbsent(groupTTLKey, - k -> Config.getLongProperty(group + "_REDIS_SERVER_DEFAULT_TTL", -1)); - return -1 == ttl? this.defaultTTL : ttl; + k -> Config.getLongProperty(groupTTLKey, -1)); + return -1 == ttl ? this.defaultTTL : ttl; } @Override @@ -273,40 +183,42 @@ public Object get(final String group, final String key) { // this avoid mutability and dirty cache issues if (DbConnectionFactory.inTransaction()) { - Logger.debug(this, ()-> "In Transaction, Skipping the get to Redis cache for group: " + Logger.debug(this, () -> "In Transaction, Skipping the get to Redis cache for group: " + group + "key" + key); - } else if (null != key && null != group) { + return null; + } - final String cacheKey = this.cacheKey(group, key); - try { + if (key == null || group == null) { + return null; + } - return this.extractObject(this.getClient().get(cacheKey)); - } catch (CacheTimeoutException e) { + final String cacheKey = this.cacheKey(group, key); + try { - Logger.debug(this, "Timeout error on getting Redis cache for group: " - + group + "key" + key + " msg: " + e.getMessage()); - return null; - } + return this.extractObject(this.getClient().get(cacheKey)); + } catch (Exception e) { + + Logger.debug(this, "Timeout error on getting Redis cache for group: " + + group + "key" + key + " msg: " + e.getMessage()); + return null; } - return null; } - private Object extractObject (final Object o) { - - return o != null && o instanceof DotCloneable? - this.extractObject(DotCloneable.class.cast(o)): o; + private Object extractObject(final Object o) { + return o != null && o instanceof DotCloneable ? + this.extractObject(DotCloneable.class.cast(o)) : o; } - private Object extractObject (final DotCloneable o) { + private Object extractObject(final DotCloneable o) { - return Try.of(()-> o.clone()).getOrElse(o); + return Try.of(() -> o.clone()).getOrElse(o); } /** * removes cache keys async and resets the get timer that reenables get functions - * + * * @param keys */ private void removeKeys(final String... keys) { @@ -317,6 +229,19 @@ private void removeKeys(final String... keys) { } } + /** + * removes cache keys async and resets the get timer that reenables get functions + * + * @param keys + */ + private void removeKeysRaw(final String... keys) { + + if (UtilMethods.isSet(keys)) { + + this.getClient().deleteNonBlocking(keys); + } + } + @Override public void remove(final String group, final String key) { @@ -329,28 +254,28 @@ public void remove(final String group) { if (!UtilMethods.isEmpty(group)) { - final String prefix = cacheKey(group) + StringPool.STAR; + // todo: this should be a wildcard deletion instead of keys scan + + final String prefix = StringPool.STAR + cacheKey(group) + StringPool.STAR; // Getting all the keys for the given groups - DotConcurrentFactory.getInstance().getSingleSubmitter - (CacheWiper.class.getSimpleName()).submit(new CacheWiper(prefix)); + /*DotConcurrentFactory.getInstance().getSingleSubmitter + (CacheWiper.class.getSimpleName()).submit(new CacheWiper(prefix));*/ + this.getClient().deleteFromPattern(prefix); } } @Override public void removeAll() { - final String prefix = loadPrefix() + "." + StringPool.STAR; - this.cycleKey(); - // Getting all the keys for the given groups - DotConcurrentFactory.getInstance().getSingleSubmitter - (CacheWiper.class.getSimpleName()).submit(new CacheWiper(prefix)); + final String prefix = StringPool.STAR + this.REDIS_PREFIX_KEY + "." + StringPool.STAR; + this.getClient().deleteFromPattern(prefix); } @Override public Set getKeys(final String group) { - final String prefix = this.cacheKey(group); - final String matchesPattern = prefix + StringPool.STAR; + final String prefix = this.cacheKey(group); + final String matchesPattern = prefix + StringPool.STAR; final Set keys = new LinkedHashSet<>(); this.getClient().scanKeys(matchesPattern, this.keyBatchingSize, //keys::addAll); redisKeys -> redisKeys.stream().map(redisKey -> // we remove the prefix in order to have the real key @@ -369,17 +294,19 @@ Set getAllKeys() { /** * returns the number of cache keys in any given group - * + * * @param group * @return */ private long keyCount(final String group) { - final String prefix = LettuceAdapter.getMasterReplicaLettuceClient(this.getClient()).wrapKey(this.cacheKey(group) + StringPool.STAR); + final String prefix = LettuceAdapter.getMasterReplicaLettuceClient(this.getClient()) + .wrapKey(this.cacheKey(group) + StringPool.STAR); final String script = "return #redis.pcall('keys', '" + prefix + "')"; - Object keyCount = ZERO; + Object keyCount = ZERO; - try (StatefulRedisConnection conn = LettuceAdapter.getStatefulRedisConnection(this.getClient())) { + try (StatefulRedisConnection conn = LettuceAdapter.getStatefulRedisConnection( + this.getClient())) { if (conn.isOpen()) { @@ -391,7 +318,6 @@ private long keyCount(final String group) { } - @Override public Set getGroups() { @@ -406,7 +332,7 @@ public CacheProviderStats getStats() { final CacheProviderStats cacheProviderStats = new CacheProviderStats(providerStats, getName()); String memoryStats = null; - try (StatefulRedisConnection conn = + try (StatefulRedisConnection conn = LettuceAdapter.getStatefulRedisConnection(this.getClient())) { if (!conn.isOpen()) { @@ -448,12 +374,12 @@ public CacheProviderStats getStats() { public void shutdown() { Logger.info(this.getClass(), "*** Shutdown [" + getName() + "] ."); - prefixKey.set(PREFIX_UNSET); + } /** - * Reads and parses the string report generated for the INFO Redis command in order to return any - * specific required property. + * Reads and parses the string report generated for the INFO Redis command in order to return any specific required + * property. * * @param redisReport * @return Map @@ -477,38 +403,5 @@ private Map getRedisProperties(final String redisReport) { return map; } - class CacheWiper implements Runnable { - - final String prefix; - - @Override - public String toString() { - return "CacheWiper prefix:" + prefix; - } - public CacheWiper(final String prefix) { - this.prefix = prefix; - } - - @Override - public void run() { - - RedisCache.this.getClient().scanKeys(this.prefix, keyBatchingSize, - keyCollections -> removeKeys(keyCollections.toArray(new String[0]))); - } - } - - class PrefixChecker implements Runnable { - - @Override - public void run() { - - final String ourPrefix = prefixKey.get(); - final String redisPrefix = loadPrefixFromRedis(); - // force a reload if needed - if(!Objects.equal(ourPrefix, redisPrefix)) { - prefixKey.set(PREFIX_UNSET); - } - } - } } diff --git a/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisClient.java b/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisClient.java index 68f5b2d03b23..bbc2b0233da7 100644 --- a/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisClient.java +++ b/dotCMS/src/main/java/com/dotcms/cache/lettuce/RedisClient.java @@ -346,5 +346,11 @@ default Collection getSubscribers (final K channel) { default Collection getChannels() { return Collections.emptyList(); } + + /** + * Deletes all entries that matches the pattern + * @param pattern String + */ + void deleteFromPattern(String pattern); } diff --git a/dotcms-integration/src/test/java/com/dotcms/cache/lettuce/LettuceCacheTest.java b/dotcms-integration/src/test/java/com/dotcms/cache/lettuce/LettuceCacheTest.java index df902694665a..6d43f981ead3 100644 --- a/dotcms-integration/src/test/java/com/dotcms/cache/lettuce/LettuceCacheTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/cache/lettuce/LettuceCacheTest.java @@ -22,42 +22,13 @@ public class LettuceCacheTest { public static void startup() throws Exception { cache = new RedisCache(); } - - - /** - * all key values that are put into redis are prefixed by a random key. - * When a flushall is called, this key is cycled, which basically invalidates - * all cached entries. The prefix key is stored in redis itself so multiple - * servers in the cluster - */ - @Test - public void test_prefix_key_cycling() { - - if (RedisClientFactory.getClient("cache").ping()) { - final String prefix1 = cache.loadPrefix(); - assert (prefix1 != null && prefix1.length() > 3); - - cache.cycleKey(); - - final String prefix2 = cache.loadPrefix(); - assert (prefix2 != null); - - // keys don't match after they have been cycled - assert (!prefix1.equals(prefix2)); - - // getting the key again and the keys match - final String prefix3 = cache.loadPrefix(); - assert (prefix2.equals(prefix3)); - } - } - @Test public void test_cache_key_generation() { if (RedisClientFactory.getClient("cache").ping()) { - final String prefix = cache.loadPrefix(); + final String prefix = cache.REDIS_PREFIX_KEY; assert (prefix != null && prefix.length() > 3); String group = "group"; String key = "key";