Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport]Improve dynamic configurations by adding cache and simplifying client fetch #324

Merged
merged 1 commit into from
Apr 11, 2024
Merged

[backport]Improve dynamic configurations by adding cache and simplifying client fetch #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 92 additions & 14 deletions src/plugins/application_config/server/opensearch_config_client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);
});
Expand All @@ -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 () => {
Expand All @@ -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
Expand Down Expand Up @@ -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);
});
});

Expand All @@ -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 () => {
Expand All @@ -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
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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);
});
Expand All @@ -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 () => {
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 22 additions & 3 deletions src/plugins/application_config/server/opensearch_config_client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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}`;
Expand All @@ -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;
}

Expand Down
Loading
Loading