Improve ui settings performance

src/core/server/ui_settings/cache.test.ts

@@ -0,0 +1,50 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Cache } from './cache';
+
+describe('Cache', () => {
+  beforeEach(() => {
+    jest.useFakeTimers();
+  });
+
+  afterEach(() => {
+    jest.clearAllTimers();
+  });
+  it('stores value for maxAge ms', async () => {
+    const cache = new Cache<number>(500);
+    cache.set(42);
+    expect(cache.get()).toBe(42);
+    jest.advanceTimersByTime(100);
+    expect(cache.get()).toBe(42);
+  });
+  it('invalidates cache after maxAge ms', async () => {
+    const cache = new Cache<number>(500);
+    cache.set(42);
+    expect(cache.get()).toBe(42);
+    jest.advanceTimersByTime(1000);
+    expect(cache.get()).toBe(null);
+  });
+  it('del invalidates cache immediately', async () => {
+    const cache = new Cache<number>(10);
+    cache.set(42);
+    expect(cache.get()).toBe(42);
+    cache.del();
+    expect(cache.get()).toBe(null);
+  });
+});

src/core/server/ui_settings/cache.ts

@@ -0,0 +1,48 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+const oneSec = 1000;
+const defMaxAge = 5 * oneSec;
+/**
+ * @internal
+ */
+export class Cache<T = Record<string, any>> {
+  private value: T | null;
+  private timer?: NodeJS.Timeout;
+
+  /**
+   * Delete cached value after maxAge ms.
+   */
+  constructor(private readonly maxAge: number = defMaxAge) {
+    this.value = null;
+  }
+  get() {
+    return this.value;
+  }
+  set(value: T) {
+    this.del();
+    this.value = value;
+    this.timer = setTimeout(() => this.del(), this.maxAge);
+  }
+  del() {
+    if (this.timer) {
+      clearTimeout(this.timer);
+    }
+    this.value = null;
+  }
+}

src/core/server/ui_settings/ui_settings_client.test.ts

@@ -676,4 +676,111 @@ describe('ui settings', () => {
       expect(uiSettings.isOverridden('bar')).toBe(true);
     });
   });
+
+  describe('caching', () => {
+    describe('read operations cache user config', () => {
+      beforeEach(() => {
+        jest.useFakeTimers();
+      });
+
+      afterEach(() => {
+        jest.clearAllTimers();
+      });
+
+      it('get', async () => {
+        const esDocSource = {};
+        const { uiSettings, savedObjectsClient } = setup({ esDocSource });
+
+        await uiSettings.get('any');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        await uiSettings.get('foo');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        jest.advanceTimersByTime(10000);
+        await uiSettings.get('foo');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(2);
+      });
+
+      it('getAll', async () => {
+        const esDocSource = {};
+        const { uiSettings, savedObjectsClient } = setup({ esDocSource });
+
+        await uiSettings.getAll();
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        await uiSettings.getAll();
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        jest.advanceTimersByTime(10000);
+        await uiSettings.getAll();
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(2);
+      });
+
+      it('getUserProvided', async () => {
+        const esDocSource = {};
+        const { uiSettings, savedObjectsClient } = setup({ esDocSource });
+
+        await uiSettings.getUserProvided();
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        await uiSettings.getUserProvided();
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        jest.advanceTimersByTime(10000);
+        await uiSettings.getUserProvided();
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(2);
+      });
+    });
+
+    describe('write operations invalidate user config cache', () => {
+      it('set', async () => {
+        const esDocSource = {};
+        const { uiSettings, savedObjectsClient } = setup({ esDocSource });
+
+        await uiSettings.get('any');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        await uiSettings.set('foo', 'bar');
+        await uiSettings.get('foo');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(2);
+      });
+
+      it('setMany', async () => {
+        const esDocSource = {};
+        const { uiSettings, savedObjectsClient } = setup({ esDocSource });
+
+        await uiSettings.get('any');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        await uiSettings.setMany({ foo: 'bar' });
+        await uiSettings.get('foo');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(2);
+      });
+
+      it('remove', async () => {
+        const esDocSource = {};
+        const { uiSettings, savedObjectsClient } = setup({ esDocSource });
+
+        await uiSettings.get('any');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        await uiSettings.remove('foo');
+        await uiSettings.get('foo');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(2);
+      });
+
+      it('removeMany', async () => {
+        const esDocSource = {};
+        const { uiSettings, savedObjectsClient } = setup({ esDocSource });
+
+        await uiSettings.get('any');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
+
+        await uiSettings.removeMany(['foo', 'bar']);
+        await uiSettings.get('foo');
+        expect(savedObjectsClient.get).toHaveBeenCalledTimes(2);
+      });
+    });
+  });
 });

src/core/server/ui_settings/ui_settings_client.ts

@@ -24,6 +24,7 @@ import { Logger } from '../logging';
 import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config';
 import { IUiSettingsClient, UiSettingsParams, PublicUiSettingsParams } from './types';
 import { CannotOverrideError } from './ui_settings_errors';
+import { Cache } from './cache';
 
 export interface UiSettingsServiceOptions {
   type: string;
@@ -36,7 +37,6 @@ export interface UiSettingsServiceOptions {
 }
 
 interface ReadOptions {
-  ignore401Errors?: boolean;
   autoCreateOrUpgradeIfMissing?: boolean;
 }
 
@@ -58,6 +58,7 @@ export class UiSettingsClient implements IUiSettingsClient {
   private readonly overrides: NonNullable<UiSettingsServiceOptions['overrides']>;
   private readonly defaults: NonNullable<UiSettingsServiceOptions['defaults']>;
   private readonly log: Logger;
+  private readonly cache: Cache;
 
   constructor(options: UiSettingsServiceOptions) {
     const { type, id, buildNum, savedObjectsClient, log, defaults = {}, overrides = {} } = options;
@@ -69,6 +70,7 @@ export class UiSettingsClient implements IUiSettingsClient {
     this.defaults = defaults;
     this.overrides = overrides;
     this.log = log;
+    this.cache = new Cache();
   }
 
   getRegistered() {
@@ -95,7 +97,12 @@ export class UiSettingsClient implements IUiSettingsClient {
   }
 
   async getUserProvided<T = unknown>(): Promise<UserProvided<T>> {
-    const userProvided: UserProvided<T> = this.onReadHook<T>(await this.read());
+    const cachedValue = this.cache.get();
+    if (cachedValue) {
+      return cachedValue;
+    }
+
+    const userProvided: UserProvided<T> = this.onReadHook(await this.read());
 
     // write all overridden keys, dropping the userValue is override is null and
     // adding keys for overrides that are not in saved object
@@ -104,10 +111,13 @@ export class UiSettingsClient implements IUiSettingsClient {
         value === null ? { isOverridden: true } : { isOverridden: true, userValue: value };
     }
 
+    this.cache.set(userProvided);
+
     return userProvided;
   }
 
   async setMany(changes: Record<string, any>) {
+    this.cache.del();
     this.onWriteHook(changes);
     await this.write({ changes });
   }
@@ -140,7 +150,7 @@ export class UiSettingsClient implements IUiSettingsClient {
 
   private async getRaw(): Promise<UiSettingsRaw> {
     const userProvided = await this.getUserProvided();
-    return defaultsDeep(userProvided, this.defaults);
+    return defaultsDeep({}, userProvided, this.defaults);
   }
 
   private validateKey(key: string, value: unknown) {
@@ -209,10 +219,9 @@ export class UiSettingsClient implements IUiSettingsClient {
     }
   }
 
-  private async read({
-    ignore401Errors = false,
-    autoCreateOrUpgradeIfMissing = true,
-  }: ReadOptions = {}): Promise<Record<string, any>> {
+  private async read({ autoCreateOrUpgradeIfMissing = true }: ReadOptions = {}): Promise<
+    Record<string, any>
+  > {
     try {
       const resp = await this.savedObjectsClient.get<Record<string, any>>(this.type, this.id);
       return resp.attributes;
@@ -227,34 +236,23 @@ export class UiSettingsClient implements IUiSettingsClient {
         });
 
         if (!failedUpgradeAttributes) {
-          return await this.read({
-            ignore401Errors,
-            autoCreateOrUpgradeIfMissing: false,
-          });
+          return await this.read({ autoCreateOrUpgradeIfMissing: false });
         }
 
         return failedUpgradeAttributes;
       }
 
-      if (this.isIgnorableError(error, ignore401Errors)) {
+      if (this.isIgnorableError(error)) {
         return {};
       }
 
       throw error;
     }
   }
 
-  private isIgnorableError(error: Error, ignore401Errors: boolean) {
-    const {
-      isForbiddenError,
-      isEsUnavailableError,
-      isNotAuthorizedError,
-    } = this.savedObjectsClient.errors;
-
-    return (
-      isForbiddenError(error) ||
-      isEsUnavailableError(error) ||
-      (ignore401Errors && isNotAuthorizedError(error))
-    );
+  private isIgnorableError(error: Error) {
+    const { isForbiddenError, isEsUnavailableError } = this.savedObjectsClient.errors;
+
+    return isForbiddenError(error) || isEsUnavailableError(error);
   }
 }