fix: optimize localstorage transactions (#1651)

* feat: avoid writing to reclaim fields continuously

* feat: add timer scale factor option

* fix: revert the localstorage clearing strategy

* fix: optimize localstorage key access

* test(analytics-js-plugins): improve test coverage for retryqueue

* test(analytics-js): improve test coverage for all the storages

* test(analytics-js): improve test coverage for all the storage modules

Author: saikumarrs

packages/analytics-js-common/src/types/LoadOptions.ts

@@ -78,6 +78,8 @@ export type QueueOpts = {
   maxItems?: number;
   // Options for batched requests
   batch?: BatchOpts;
+  // The scale factor applied to the default timer values
+  timerScaleFactor?: number;
 };
 
 /**

packages/analytics-js-common/src/types/Store.ts

@@ -54,6 +54,7 @@ export interface IStore {
 
 export interface IStorage extends Storage {
   configure?(options: StorageOptions): void;
+  keys(): string[];
   isEnabled?: boolean;
 }
 

packages/analytics-js-plugins/__tests__/utilities/retryQueue/RetryQueue.test.ts

@@ -37,7 +37,14 @@ describe('Queue', () => {
     schedule.now = () => +new window.Date();
 
     // Have the default function be a spied success
-    queue = new RetryQueue('test', {}, jest.fn(), defaultStoreManager);
+    queue = new RetryQueue(
+      'test',
+      {
+        timerScaleFactor: 2, // scales the timers by 2x. Not a necessity, but added this option to test the timer scaling
+      },
+      jest.fn(),
+      defaultStoreManager,
+    );
     queue.schedule = schedule;
   });
 
@@ -188,13 +195,7 @@ describe('Queue', () => {
   });
 
   it('should respect shouldRetry', () => {
-    queue.shouldRetry = (_, attemptNumber) => {
-      if (attemptNumber > 2) {
-        return false;
-      }
-
-      return true;
-    };
+    queue.shouldRetry = (_, attemptNumber) => !(attemptNumber > 2);
 
     const mockProcessItemCb = jest.fn((_, cb) => cb(new Error('no')));
 
@@ -205,21 +206,17 @@ describe('Queue', () => {
     // over maxattempts
     queue.requeue('a', 3);
     jest.advanceTimersByTime(queue.getDelay(3));
-    expect(queue.processQueueCb).toBeCalledTimes(0);
+    expect(queue.processQueueCb).toHaveBeenCalledTimes(0);
 
     mockProcessItemCb.mockReset();
     queue.requeue('a', 2);
     jest.advanceTimersByTime(queue.getDelay(2));
-    expect(queue.processQueueCb).toBeCalledTimes(1);
-
-    // logic based on item state (eg. could be msg timestamp field)
-    queue.shouldRetry = item => !(item.shouldRetry === false);
+    expect(queue.processQueueCb).toHaveBeenCalledTimes(1);
 
     mockProcessItemCb.mockReset();
-    queue.requeue({ shouldRetry: false }, 1);
+    queue.requeue('a', 3);
     jest.advanceTimersByTime(queue.getDelay(1));
-
-    expect(queue.processQueueCb).toBeCalledTimes(0);
+    expect(queue.processQueueCb).toHaveBeenCalledTimes(0);
   });
 
   it('should respect maxItems', () => {
@@ -382,8 +379,22 @@ describe('Queue', () => {
     // wait long enough for the other queue to expire and be reclaimed
     jest.advanceTimersByTime(queue.timeouts.reclaimTimer + queue.timeouts.reclaimWait * 2);
 
-    expect(queue.processQueueCb).nthCalledWith(1, 'a', expect.any(Function), 0, Infinity, true);
-    expect(queue.processQueueCb).nthCalledWith(2, 'b', expect.any(Function), 0, Infinity, true);
+    expect(queue.processQueueCb).toHaveBeenNthCalledWith(
+      1,
+      'a',
+      expect.any(Function),
+      0,
+      Infinity,
+      true,
+    );
+    expect(queue.processQueueCb).toHaveBeenNthCalledWith(
+      2,
+      'b',
+      expect.any(Function),
+      0,
+      Infinity,
+      true,
+    );
   });
 
   it('should deduplicate ids when reclaiming abandoned queue tasks', () => {
@@ -813,7 +824,7 @@ describe('Queue', () => {
       done(new Error());
     };
 
-    for (let i = 0; i < calls.length; i = i + 1) {
+    for (let i = 0; i < calls.length; i += 1) {
       queue.addItem({ index: i });
     }
 
@@ -849,7 +860,7 @@ describe('Queue', () => {
     expect(size(queue).inProgress).toEqual(queue.maxItems);
 
     // while the items are in progress let's add maxItems times two items
-    for (i = 0; i < queue.maxItems * 2; i = i + 1) {
+    for (i = 0; i < queue.maxItems * 2; i += 1) {
       queue.addItem({ index: i });
     }
 

packages/analytics-js-plugins/src/utilities/retryQueue/RetryQueue.ts

@@ -5,7 +5,7 @@ import type { StorageType } from '@rudderstack/analytics-js-common/types/Storage
 import type { Nullable } from '@rudderstack/analytics-js-common/types/Nullable';
 import type { ILogger } from '@rudderstack/analytics-js-common/types/Logger';
 import type { BatchOpts, QueueOpts } from '@rudderstack/analytics-js-common/types/LoadOptions';
-import { isDefined } from '@rudderstack/analytics-js-common/utilities/checks';
+import { isDefined, isNullOrUndefined } from '@rudderstack/analytics-js-common/utilities/checks';
 import { LOCAL_STORAGE } from '@rudderstack/analytics-js-common/constants/storages';
 import { generateUUID } from '@rudderstack/analytics-js-common/utilities/uuId';
 import type {
@@ -32,6 +32,8 @@ import {
   DEFAULT_RECLAIM_TIMEOUT_MS,
   DEFAULT_RECLAIM_WAIT_MS,
   DEFAULT_BATCH_FLUSH_INTERVAL_MS,
+  MIN_TIMER_SCALE_FACTOR,
+  MAX_TIMER_SCALE_FACTOR,
 } from './constants';
 
 const sortByTime = (a: QueueItem, b: QueueItem) => a.time - b.time;
@@ -64,6 +66,8 @@ class RetryQueue implements IQueue<QueueItemData> {
   flushBatchTaskId?: string;
   batchingInProgress?: boolean;
   batchSizeCalcCb?: QueueBatchItemsSizeCalculatorCallback<QueueItemData>;
+  reclaimStartVal?: Nullable<string>;
+  reclaimEndVal?: Nullable<string>;
 
   constructor(
     name: string,
@@ -95,12 +99,21 @@ class RetryQueue implements IQueue<QueueItemData> {
       jitter: options.backoffJitter || DEFAULT_BACKOFF_JITTER,
     };
 
+    // Limit the timer scale factor to the minimum value
+    let timerScaleFactor = Math.max(
+      options.timerScaleFactor ?? MIN_TIMER_SCALE_FACTOR,
+      MIN_TIMER_SCALE_FACTOR,
+    );
+
+    // Limit the timer scale factor to the maximum value
+    timerScaleFactor = Math.min(timerScaleFactor, MAX_TIMER_SCALE_FACTOR);
+
     // painstakingly tuned. that's why they're not "easily" configurable
     this.timeouts = {
-      ackTimer: DEFAULT_ACK_TIMER_MS,
-      reclaimTimer: DEFAULT_RECLAIM_TIMER_MS,
-      reclaimTimeout: DEFAULT_RECLAIM_TIMEOUT_MS,
-      reclaimWait: DEFAULT_RECLAIM_WAIT_MS,
+      ackTimer: Math.round(timerScaleFactor * DEFAULT_ACK_TIMER_MS),
+      reclaimTimer: Math.round(timerScaleFactor * DEFAULT_RECLAIM_TIMER_MS),
+      reclaimTimeout: Math.round(timerScaleFactor * DEFAULT_RECLAIM_TIMEOUT_MS),
+      reclaimWait: Math.round(timerScaleFactor * DEFAULT_RECLAIM_WAIT_MS),
     };
 
     this.schedule = new Schedule();
@@ -165,17 +178,21 @@ class RetryQueue implements IQueue<QueueItemData> {
   }
 
   getStorageEntry(
-    name?: string,
+    name: string,
   ): Nullable<QueueItem<QueueItemData>[] | Record<string, any> | number> {
-    return this.store.get(name ?? this.name);
+    return this.store.get(name);
   }
 
   // TODO: fix the type of different queues to be the same if possible
   setStorageEntry(
-    name?: string,
+    name: string,
     value?: Nullable<QueueItem<QueueItemData>[] | Record<string, any>> | number,
   ) {
-    this.store.set(name ?? this.name, value ?? []);
+    if (isNullOrUndefined(value)) {
+      this.store.remove(name);
+    } else {
+      this.store.set(name, value);
+    }
   }
 
   /**
@@ -305,7 +322,7 @@ class RetryQueue implements IQueue<QueueItemData> {
       batchQueue = batchQueue.slice(-batchQueue.length);
       batchQueue.push(entry);
 
-      const batchDispatchInfo = this.getBatchDispInfo(batchQueue);
+      const batchDispatchInfo = this.getBatchDispatchInfo(batchQueue);
       // if batch criteria is met, queue the batch events to the main queue and clear batch queue
       if (batchDispatchInfo.criteriaMet || batchDispatchInfo.criteriaExceeded) {
         let batchItems;
@@ -399,7 +416,7 @@ class RetryQueue implements IQueue<QueueItemData> {
    * @param batchItems Prospective batch items
    * @returns Batch dispatch info
    */
-  getBatchDispInfo(batchItems: QueueItem[]) {
+  getBatchDispatchInfo(batchItems: QueueItem[]) {
     let lengthCriteriaMet = false;
     let lengthCriteriaExceeded = false;
     const configuredBatchMaxItems = this.batch?.maxItems as number;
@@ -520,8 +537,17 @@ class RetryQueue implements IQueue<QueueItemData> {
   // Ack continuously to prevent other tabs from claiming our queue
   ack() {
     this.setStorageEntry(QueueStatuses.ACK, this.schedule.now());
-    this.setStorageEntry(QueueStatuses.RECLAIM_START, null);
-    this.setStorageEntry(QueueStatuses.RECLAIM_END, null);
+
+    if (this.reclaimStartVal != null) {
+      this.reclaimStartVal = null;
+      this.setStorageEntry(QueueStatuses.RECLAIM_START, null);
+    }
+
+    if (this.reclaimEndVal != null) {
+      this.reclaimEndVal = null;
+      this.setStorageEntry(QueueStatuses.RECLAIM_END, null);
+    }
+
     this.schedule.run(this.ack, this.timeouts.ackTimer, ScheduleModes.ASAP);
   }
 
@@ -609,9 +635,10 @@ class RetryQueue implements IQueue<QueueItemData> {
 
   removeStorageEntry(store: IStore, entryIdx: number, backoff: number, attempt = 1) {
     const maxAttempts = 2;
+    const queueEntryKeys = Object.keys(QueueStatuses);
+    const entry = QueueStatuses[queueEntryKeys[entryIdx] as keyof typeof QueueStatuses];
+
     (globalThis as typeof window).setTimeout(() => {
-      const queueEntryKeys = Object.keys(QueueStatuses);
-      const entry = QueueStatuses[queueEntryKeys[entryIdx] as keyof typeof QueueStatuses];
       try {
         store.remove(entry);
 
@@ -633,7 +660,7 @@ class RetryQueue implements IQueue<QueueItemData> {
           this.logger?.error(RETRY_QUEUE_ENTRY_REMOVE_ERROR(RETRY_QUEUE, entry, attempt), err);
         }
 
-        // clear the next entry
+        // clear the next entry after we've exhausted our attempts
         if (attempt === maxAttempts && entryIdx + 1 < queueEntryKeys.length) {
           this.removeStorageEntry(store, entryIdx + 1, backoff);
         }
@@ -678,45 +705,31 @@ class RetryQueue implements IQueue<QueueItemData> {
     };
     const findOtherQueues = (name: string): IStore[] => {
       const res: IStore[] = [];
-      const storage = this.store.getOriginalEngine();
-
-      for (let i = 0; i < storage.length; i++) {
-        const k = storage.key(i);
-        const parts: string[] = k ? k.split('.') : [];
-
-        if (parts.length !== 3) {
-          // eslint-disable-next-line no-continue
-          continue;
-        }
-
-        if (parts[0] !== name) {
-          // eslint-disable-next-line no-continue
-          continue;
-        }
-
-        if (parts[2] !== QueueStatuses.ACK) {
-          // eslint-disable-next-line no-continue
-          continue;
+      const storageKeys = this.store.getOriginalEngine().keys();
+      storageKeys.forEach((k: string) => {
+        const keyParts: string[] = k ? k.split('.') : [];
+
+        if (
+          keyParts.length >= 3 &&
+          keyParts[0] === name &&
+          keyParts[1] !== this.id &&
+          keyParts[2] === QueueStatuses.ACK
+        ) {
+          res.push(
+            this.storeManager.setStore({
+              id: keyParts[1] as string,
+              name,
+              validKeys: QueueStatuses,
+              type: LOCAL_STORAGE,
+            }),
+          );
         }
-
-        res.push(
-          this.storeManager.setStore({
-            id: parts[1] as string,
-            name,
-            validKeys: QueueStatuses,
-            type: LOCAL_STORAGE,
-          }),
-        );
-      }
+      });
 
       return res;
     };
 
     findOtherQueues(this.name).forEach(store => {
-      if (store.id === this.id) {
-        return;
-      }
-
       if (this.schedule.now() - store.get(QueueStatuses.ACK) < this.timeouts.reclaimTimeout) {
         return;
       }

packages/analytics-js-plugins/src/utilities/retryQueue/constants.ts

@@ -10,6 +10,8 @@ const DEFAULT_ACK_TIMER_MS = 1000;
 const DEFAULT_RECLAIM_TIMER_MS = 3000;
 const DEFAULT_RECLAIM_TIMEOUT_MS = 10000;
 const DEFAULT_RECLAIM_WAIT_MS = 500;
+const MIN_TIMER_SCALE_FACTOR = 1;
+const MAX_TIMER_SCALE_FACTOR = 10;
 
 const DEFAULT_MAX_BATCH_SIZE_BYTES = 512 * 1024; // 512 KB; this is also the max size of a batch
 const DEFAULT_MAX_BATCH_ITEMS = 100;
@@ -29,4 +31,6 @@ export {
   DEFAULT_MAX_BATCH_SIZE_BYTES,
   DEFAULT_MAX_BATCH_ITEMS,
   DEFAULT_BATCH_FLUSH_INTERVAL_MS,
+  MIN_TIMER_SCALE_FACTOR,
+  MAX_TIMER_SCALE_FACTOR,
 };

packages/analytics-js/__tests__/services/StoreManager/storages/CookieStorage.test.ts

@@ -1,4 +1,4 @@
-import { IStorage } from '@rudderstack/analytics-js-common/types/Store';
+import type { IStorage } from '@rudderstack/analytics-js-common/types/Store';
 import {
   getStorageEngine,
   configureCookieStorageEngine,
@@ -17,10 +17,13 @@ describe('CookieStorage', () => {
     expect(engine.getItem('test-key')).toStrictEqual('abc');
     expect(engine.length).toStrictEqual(1);
     expect(engine.key(0)).toStrictEqual('test-key');
+    expect(engine.key(1)).toBeNull();
+    expect(engine.keys()).toStrictEqual(['test-key']);
 
     engine.removeItem('test-key');
     expect(engine.getItem('test-key')).toBeNull();
     expect(engine.length).toStrictEqual(0);
+    expect(engine.keys()).toStrictEqual([]);
 
     // clear not implemented intentionally, see source code comments
     // engine.setItem('test-key', 'abc');

packages/analytics-js/__tests__/services/StoreManager/storages/InMemoryStorage.test.ts

@@ -1,4 +1,4 @@
-import { IStorage } from '@rudderstack/analytics-js-common/types/Store';
+import type { IStorage } from '@rudderstack/analytics-js-common/types/Store';
 import { getStorageEngine } from '../../../../src/services/StoreManager/storages/storageEngine';
 
 describe('InMemoryStorage', () => {
@@ -14,13 +14,18 @@ describe('InMemoryStorage', () => {
     expect(engine.getItem('test-key')).toStrictEqual('abc');
     expect(engine.length).toStrictEqual(1);
     expect(engine.key(0)).toStrictEqual('test-key');
+    expect(engine.key(1)).toBeNull();
+    expect(engine.keys()).toStrictEqual(['test-key']);
 
     engine.removeItem('test-key');
     expect(engine.getItem('test-key')).toBeNull();
     expect(engine.length).toStrictEqual(0);
+    expect(engine.keys()).toStrictEqual([]);
 
     engine.setItem('test-key', 'abc');
     engine.clear();
+    expect(engine.getItem('test-key')).toBeNull();
+    expect(engine.keys()).toStrictEqual([]);
     expect(engine.length).toStrictEqual(0);
   });
 });