fix: HotFix for using mSet instead of Set for redis cache when storin…

…g synthetic transactions (#2197) * Fix to use mSet instead of set, so we can batch the requests to the redis cache Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com> * Feedback of PR Review Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com> * some improvements Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com> * add missing metric observations and leaving the metrics observations to the end so we dont capture and then fail (if it fails) Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com> * adding unit tests for MultiSet state changes on the cache Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com> --------- Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com>
hashgraph · Mar 14, 2024 · e196bcd · e196bcd
1 parent 6acced8
commit e196bcd
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 16 deletions.
diff --git a/packages/relay/src/lib/clients/cache/ICacheClient.ts b/packages/relay/src/lib/clients/cache/ICacheClient.ts
@@ -23,4 +23,5 @@ export interface ICacheClient {
   set(key: string, value: any, callingMethod: string, ttl?: number, requestIdPrefix?: string): void;
   delete(key: string, callingMethod: string, requestIdPrefix?: string): void;
   clear(): void;
+  multiSet(keyValuePairs: Record<string, any>, callingMethod: string, requestIdPrefix?: string): void;
 }
diff --git a/packages/relay/src/lib/clients/cache/localLRUCache.ts b/packages/relay/src/lib/clients/cache/localLRUCache.ts
@@ -129,6 +129,21 @@ export class LocalLRUCache implements ICacheClient {
     this.cache.set(key, value, { ttl: resolvedTtl });
   }
 
+  /**
+   * Stores multiple key-value pairs in the cache.
+   *
+   * @param keyValuePairs - An object where each property is a key and its value is the value to be cached.
+   * @param callingMethod - The name of the calling method.
+   * @param requestIdPrefix - Optional request ID prefix for logging.
+   * @returns {Promise<void>} A Promise that resolves when the values are cached.
+   */
+  public multiSet(keyValuePairs: Record<string, any>, callingMethod: string, requestIdPrefix?: string): void {
+    // Iterate over each entry in the keyValuePairs object
+    for (const [key, value] of Object.entries(keyValuePairs)) {
+      this.set(key, value, callingMethod, undefined, requestIdPrefix);
+    }
+  }
+
   /**
    * Deletes a cached value associated with the given key.
    * Logs the deletion of the cache entry.

diff --git a/packages/relay/src/lib/clients/cache/redisCache.ts b/packages/relay/src/lib/clients/cache/redisCache.ts
@@ -137,6 +137,29 @@ export class RedisCache implements ICacheClient {
     // TODO: add metrics
   }
 
+  /**
+   * Stores multiple key-value pairs in the cache.
+   *
+   * @param keyValuePairs - An object where each property is a key and its value is the value to be cached.
+   * @param callingMethod - The name of the calling method.
+   * @param requestIdPrefix - Optional request ID prefix for logging.
+   * @returns {Promise<void>} A Promise that resolves when the values are cached.
+   */
+  async multiSet(keyValuePairs: Record<string, any>, callingMethod: string, requestIdPrefix?: string): Promise<void> {
+    // Serialize values
+    const serializedKeyValuePairs: Record<string, string> = {};
+    for (const [key, value] of Object.entries(keyValuePairs)) {
+      serializedKeyValuePairs[key] = JSON.stringify(value);
+    }
+
+    // Perform mSet operation
+    await this.client.mSet(serializedKeyValuePairs);
+
+    // Log the operation
+    const entriesLength = Object.keys(keyValuePairs).length;
+    this.logger.trace(`${requestIdPrefix} caching multiple keys via ${callingMethod}, total keys: ${entriesLength}`);
+  }
+
   /**
    * Deletes a value from the cache.
    *

diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -2051,6 +2051,8 @@ export class EthImpl implements Eth {
     requestIdPrefix?: string,
   ): void {
     let filteredLogs: Log[];
+    const keyValuePairs: Record<string, any> = {}; // Object to accumulate cache entries
+
     if (showDetails) {
       filteredLogs = logs.filter(
         (log) => !transactionArray.some((transaction) => transaction.hash === log.transactionHash),
@@ -2060,35 +2062,25 @@ export class EthImpl implements Eth {
         transactionArray.push(transaction);
 
         const cacheKey = `${constants.CACHE_KEY.SYNTHETIC_LOG_TRANSACTION_HASH}${log.transactionHash}`;
-        this.cacheService.set(
-          cacheKey,
-          log,
-          EthImpl.ethGetBlockByHash,
-          this.syntheticLogCacheTtl,
-          requestIdPrefix,
-          true,
-        );
+        keyValuePairs[cacheKey] = log;
       });
     } else {
       filteredLogs = logs.filter((log) => !transactionArray.includes(log.transactionHash));
       filteredLogs.forEach((log) => {
         transactionArray.push(log.transactionHash);
 
         const cacheKey = `${constants.CACHE_KEY.SYNTHETIC_LOG_TRANSACTION_HASH}${log.transactionHash}`;
-        this.cacheService.set(
-          cacheKey,
-          log,
-          EthImpl.ethGetBlockByHash,
-          this.syntheticLogCacheTtl,
-          requestIdPrefix,
-          true,
-        );
+        keyValuePairs[cacheKey] = log;
       });
 
       this.logger.trace(
         `${requestIdPrefix} ${filteredLogs.length} Synthetic transaction hashes will be added in the block response`,
       );
     }
+    // cache the whole array using mSet
+    if (Object.keys(keyValuePairs).length > 0) {
+      this.cacheService.multiSet(keyValuePairs, EthImpl.ethGetBlockByHash, requestIdPrefix, true);
+    }
   }
 
   /**

diff --git a/packages/relay/src/lib/services/cacheService/cacheService.ts b/packages/relay/src/lib/services/cacheService/cacheService.ts
@@ -74,6 +74,7 @@ export class CacheService {
     GET_ASYNC: 'getAsync',
     SET: 'set',
     DELETE: 'delete',
+    MSET: 'mSet',
   };
 
   private readonly cacheMethodsCounter: Counter;
@@ -214,6 +215,40 @@ export class CacheService {
     }
   }
 
+  /**
+   * Sets multiple values in the cache, each associated with its respective key.
+   * If the shared cache is enabled and an error occurs while setting in it,
+   * the internal cache is used as a fallback.
+   * @param {Record<string, any>} entries - An object containing key-value pairs to cache.
+   * @param {string} callingMethod - The name of the method calling the cache.
+   * @param {string} requestIdPrefix - A prefix to include in log messages (optional).
+   * @param {boolean} shared - Whether to use the shared cache (optional, default: false).
+   */
+  public multiSet(
+    entries: Record<string, any>,
+    callingMethod: string,
+    requestIdPrefix?: string,
+    shared: boolean = true,
+  ): void {
+    if (shared && this.isSharedCacheEnabled) {
+      try {
+        this.sharedCache.multiSet(entries, callingMethod, requestIdPrefix);
+        this.cacheMethodsCounter.labels(callingMethod, CacheService.cacheTypes.REDIS, CacheService.methods.MSET).inc(1);
+      } catch (error) {
+        const redisError = new RedisCacheError(error);
+        this.logger.error(
+          `${requestIdPrefix} Error occurred while setting the cache to Redis. Fallback to internal cache. Error is: ${redisError.fullError}`,
+        );
+        // Fallback to internal cache
+        this.internalCache.multiSet(entries, callingMethod, requestIdPrefix);
+        this.cacheMethodsCounter.labels(callingMethod, CacheService.cacheTypes.LRU, CacheService.methods.MSET).inc(1);
+      }
+    } else {
+      this.internalCache.multiSet(entries, callingMethod, requestIdPrefix);
+      this.cacheMethodsCounter.labels(callingMethod, CacheService.cacheTypes.LRU, CacheService.methods.MSET).inc(1);
+    }
+  }
+
   /**
    * Deletes a cached value associated with the given key.
    * If the shared cache is enabled and an error occurs while deleting from it, just logs the error.

diff --git a/packages/relay/tests/lib/services/cacheService/cacheService.spec.ts b/packages/relay/tests/lib/services/cacheService/cacheService.spec.ts
@@ -76,6 +76,20 @@ describe('CacheService Test Suite', async function () {
 
       expect(cachedValue).eq(value);
     });
+
+    it('should be able to set using multiSet and get them separately', async function () {
+      const entries: Record<string, any> = {};
+      entries['key1'] = 'value1';
+      entries['key2'] = 'value2';
+      entries['key3'] = 'value3';
+
+      cacheService.multiSet(entries, callingMethod, undefined, true);
+
+      for (const [key, value] of Object.entries(entries)) {
+        const valueFromCache = cacheService.getSharedWithFallback(key, callingMethod, undefined);
+        expect(valueFromCache).eq(value);
+      }
+    });
   });
 
   describe('Shared Cache Test Suite', async function () {
@@ -137,5 +151,19 @@ describe('CacheService Test Suite', async function () {
 
       expect(cachedValue).eq(value);
     });
+
+    it('should be able to set using multiSet and get them separately using internal cache', async function () {
+      const entries: Record<string, any> = {};
+      entries['key1'] = 'value1';
+      entries['key2'] = 'value2';
+      entries['key3'] = 'value3';
+
+      cacheService.multiSet(entries, callingMethod, undefined, false);
+
+      for (const [key, value] of Object.entries(entries)) {
+        const valueFromCache = cacheService.getSharedWithFallback(key, callingMethod, undefined);
+        expect(valueFromCache).eq(value);
+      }
+    });
   });
 });