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.

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 2.x] Improve dynamic configurations by adding cache and simplifying client fetch #6403

Merged
merged 1 commit into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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