Improve dynamic configurations by adding cache and simplifying client…

… fetch (opensearch-project#6364) * Improve dynamic config Signed-off-by: Tianle Huang <[email protected]> * reset yml Signed-off-by: Tianle Huang <[email protected]> * bring back previous changes Signed-off-by: Tianle Huang <[email protected]> --------- Signed-off-by: Tianle Huang <[email protected]>
tianleh · Apr 11, 2024 · cd857cb · cd857cb
1 parent 7e1d940
commit cd857cb
Show file tree

Hide file tree

Showing 8 changed files with 249 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -86,6 +86,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Workspace] Add APIs to support plugin state in request ([#6303](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6303))
 - [Workspace] Filter left nav menu items according to the current workspace ([#6234](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6234))
 - [Multiple Datasource] Refactor data source selector component to include placeholder and add tests ([#6372](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6372))
+- [Dynamic Configurations] Improve dynamic configurations by adding cache and simplifying client fetch ([#6364](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6364))
 - [MD] Add OpenSearch cluster group label to top of single selectable dropdown  ([#6400](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6400))
 
 ### 🐛 Bug Fixes

diff --git a/src/plugins/application_config/server/opensearch_config_client.test.ts b/src/plugins/application_config/server/opensearch_config_client.test.ts
@@ -48,7 +48,9 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       const value = await client.getConfig();
 
@@ -77,7 +79,10 @@ describe('OpenSearch Configuration Client', () => {
           }),
         },
       };
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.getConfig()).rejects.toThrowError(ERROR_MESSAGE);
     });
@@ -99,11 +104,45 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {
+        has: jest.fn().mockReturnValue(false),
+        set: jest.fn(),
+      };
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       const value = await client.getEntityConfig('config1');
 
       expect(value).toBe('value1');
+      expect(cache.set).toBeCalledWith('config1', 'value1');
+    });
+
+    it('return configuration value from cache', async () => {
+      const opensearchClient = {
+        asInternalUser: {
+          get: jest.fn().mockImplementation(() => {
+            return {
+              body: {
+                _source: {
+                  value: 'value1',
+                },
+              },
+            };
+          }),
+        },
+      };
+
+      const cache = {
+        has: jest.fn().mockReturnValue(true),
+        get: jest.fn().mockReturnValue('cachedValue'),
+      };
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
+
+      const value = await client.getEntityConfig('config1');
+
+      expect(value).toBe('cachedValue');
+      expect(cache.get).toBeCalledWith('config1');
     });
 
     it('throws error when input is empty', async () => {
@@ -121,7 +160,9 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.getEntityConfig(EMPTY_INPUT)).rejects.toThrowError(
         ERROR_MESSSAGE_FOR_EMPTY_INPUT
@@ -151,9 +192,16 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {
+        has: jest.fn().mockReturnValue(false),
+        set: jest.fn(),
+      };
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.getEntityConfig('config1')).rejects.toThrowError(ERROR_MESSAGE);
+
+      expect(cache.set).toBeCalledWith('config1', undefined);
     });
   });
 
@@ -167,11 +215,16 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {
+        del: jest.fn(),
+      };
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       const value = await client.deleteEntityConfig('config1');
 
       expect(value).toBe('config1');
+      expect(cache.del).toBeCalledWith('config1');
     });
 
     it('throws error when input entity is empty', async () => {
@@ -183,7 +236,9 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.deleteEntityConfig(EMPTY_INPUT)).rejects.toThrowError(
         ERROR_MESSSAGE_FOR_EMPTY_INPUT
@@ -213,11 +268,16 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {
+        del: jest.fn(),
+      };
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       const value = await client.deleteEntityConfig('config1');
 
       expect(value).toBe('config1');
+      expect(cache.del).toBeCalledWith('config1');
     });
 
     it('return deleted document entity when deletion fails due to document not found', async () => {
@@ -241,11 +301,16 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {
+        del: jest.fn(),
+      };
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       const value = await client.deleteEntityConfig('config1');
 
       expect(value).toBe('config1');
+      expect(cache.del).toBeCalledWith('config1');
     });
 
     it('throws error when opensearch throws error', async () => {
@@ -271,7 +336,9 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.deleteEntityConfig('config1')).rejects.toThrowError(ERROR_MESSAGE);
     });
@@ -287,11 +354,16 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {
+        set: jest.fn(),
+      };
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       const value = await client.updateEntityConfig('config1', 'newValue1');
 
       expect(value).toBe('newValue1');
+      expect(cache.set).toBeCalledWith('config1', 'newValue1');
     });
 
     it('throws error when entity is empty ', async () => {
@@ -303,7 +375,9 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.updateEntityConfig(EMPTY_INPUT, 'newValue1')).rejects.toThrowError(
         ERROR_MESSSAGE_FOR_EMPTY_INPUT
@@ -319,7 +393,9 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.updateEntityConfig('config1', EMPTY_INPUT)).rejects.toThrowError(
         ERROR_MESSSAGE_FOR_EMPTY_INPUT
@@ -349,7 +425,9 @@ describe('OpenSearch Configuration Client', () => {
         },
       };
 
-      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger);
+      const cache = {};
+
+      const client = new OpenSearchConfigurationClient(opensearchClient, INDEX_NAME, logger, cache);
 
       await expect(client.updateEntityConfig('config1', 'newValue1')).rejects.toThrowError(
         ERROR_MESSAGE

diff --git a/src/plugins/application_config/server/opensearch_config_client.ts b/src/plugins/application_config/server/opensearch_config_client.ts
@@ -3,41 +3,54 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import LRUCache from 'lru-cache';
 import { IScopedClusterClient, Logger } from '../../../../src/core/server';
-
 import { ConfigurationClient } from './types';
 import { validate } from './string_utils';
 
 export class OpenSearchConfigurationClient implements ConfigurationClient {
   private client: IScopedClusterClient;
   private configurationIndexName: string;
   private readonly logger: Logger;
+  private cache: LRUCache<string, string | undefined>;
 
   constructor(
     scopedClusterClient: IScopedClusterClient,
     configurationIndexName: string,
-    logger: Logger
+    logger: Logger,
+    cache: LRUCache<string, string | undefined>
   ) {
     this.client = scopedClusterClient;
     this.configurationIndexName = configurationIndexName;
     this.logger = logger;
+    this.cache = cache;
   }
 
   async getEntityConfig(entity: string) {
     const entityValidated = validate(entity, this.logger);
 
+    if (this.cache.has(entityValidated)) {
+      return this.cache.get(entityValidated);
+    }
+
+    this.logger.info(`Key ${entityValidated} is not found from cache.`);
+
     try {
       const data = await this.client.asInternalUser.get({
         index: this.configurationIndexName,
         id: entityValidated,
       });
+      const value = data?.body?._source?.value;
 
-      return data?.body?._source?.value || '';
+      this.cache.set(entityValidated, value);
+
+      return value;
     } catch (e) {
       const errorMessage = `Failed to get entity ${entityValidated} due to error ${e}`;
 
       this.logger.error(errorMessage);
 
+      this.cache.set(entityValidated, undefined);
       throw e;
     }
   }
@@ -55,6 +68,8 @@ export class OpenSearchConfigurationClient implements ConfigurationClient {
         },
       });
 
+      this.cache.set(entityValidated, newValueValidated);
+
       return newValueValidated;
     } catch (e) {
       const errorMessage = `Failed to update entity ${entityValidated} with newValue ${newValueValidated} due to error ${e}`;
@@ -74,15 +89,19 @@ export class OpenSearchConfigurationClient implements ConfigurationClient {
         id: entityValidated,
       });
 
+      this.cache.del(entityValidated);
+
       return entityValidated;
     } catch (e) {
       if (e?.body?.error?.type === 'index_not_found_exception') {
         this.logger.info('Attemp to delete a not found index.');
+        this.cache.del(entityValidated);
         return entityValidated;
       }
 
       if (e?.body?.result === 'not_found') {
         this.logger.info('Attemp to delete a not found document.');
+        this.cache.del(entityValidated);
         return entityValidated;
       }