From 21af372d372b80b1a0449ed91fd7e012f2442505 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 04:48:49 -0700 Subject: [PATCH 01/29] wip cache resource implementation --- .../region/regionProfileManager.ts | 27 ++++- .../core/src/codewhisperer/util/authUtil.ts | 1 + .../webview/vue/amazonq/backend_amazonq.ts | 2 +- packages/core/src/shared/globalState.ts | 1 + .../src/shared/utilities/resourceCache.ts | 113 ++++++++++++++++++ 5 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/shared/utilities/resourceCache.ts diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index 039d5cefb59..88a4a04b67f 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -29,6 +29,7 @@ import { isAwsError, ToolkitError } from '../../shared/errors' import { telemetry } from '../../shared/telemetry/telemetry' import { localize } from '../../shared/utilities/vsCodeUtils' import { Commands } from '../../shared/vscode/commands2' +import { CachedResource } from '../../shared/utilities/resourceCache' // TODO: is there a better way to manage all endpoint strings in one place? export const defaultServiceConfig: CodeWhispererConfig = { @@ -59,6 +60,20 @@ export class RegionProfileManager { // Store the last API results (for UI propuse) so we don't need to call service again if doesn't require "latest" result private _profiles: RegionProfile[] = [] + private readonly cache = new (class extends CachedResource { + constructor(private readonly profileProvider: () => Promise) { + super('aws.amazonq.regionProfiles.cache', 60000, { + locked: false, + timestamp: 0, + result: undefined, + }) + } + + override resourceProvider(): Promise { + return this.profileProvider() + } + })(this.listRegionProfile.bind(this)) + get activeRegionProfile() { const conn = this.connectionProvider() if (isBuilderIdConnection(conn)) { @@ -104,6 +119,14 @@ export class RegionProfileManager { constructor(private readonly connectionProvider: () => Connection | undefined) {} + async resetCache() { + await this.cache.releaseLock() + } + + async getProfiles(): Promise { + return this.cache.getResource() + } + async listRegionProfile(): Promise { this._profiles = [] @@ -238,7 +261,7 @@ export class RegionProfileManager { return } // cross-validation - this.listRegionProfile() + this.getProfiles() .then(async (profiles) => { const r = profiles.find((it) => it.arn === previousSelected.arn) if (!r) { @@ -300,7 +323,7 @@ export class RegionProfileManager { const selected = this.activeRegionProfile let profiles: RegionProfile[] = [] try { - profiles = await this.listRegionProfile() + profiles = await this.getProfiles() } catch (e) { return [ { diff --git a/packages/core/src/codewhisperer/util/authUtil.ts b/packages/core/src/codewhisperer/util/authUtil.ts index 0898493b6db..7deae72a1c1 100644 --- a/packages/core/src/codewhisperer/util/authUtil.ts +++ b/packages/core/src/codewhisperer/util/authUtil.ts @@ -142,6 +142,7 @@ export class AuthUtil { if (!this.isConnected()) { await this.regionProfileManager.invalidateProfile(this.regionProfileManager.activeRegionProfile?.arn) + await this.regionProfileManager.resetCache() } }) diff --git a/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts b/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts index 0853f80e952..6ed152ab4ea 100644 --- a/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts +++ b/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts @@ -223,7 +223,7 @@ export class AmazonQLoginWebview extends CommonAuthWebview { */ override async listRegionProfiles(): Promise { try { - return await AuthUtil.instance.regionProfileManager.listRegionProfile() + return await AuthUtil.instance.regionProfileManager.getProfiles() } catch (e) { const conn = AuthUtil.instance.conn as SsoConnection | undefined telemetry.amazonq_didSelectProfile.emit({ diff --git a/packages/core/src/shared/globalState.ts b/packages/core/src/shared/globalState.ts index 44d848ec69d..6ecca56f90a 100644 --- a/packages/core/src/shared/globalState.ts +++ b/packages/core/src/shared/globalState.ts @@ -49,6 +49,7 @@ export type globalKey = | 'aws.toolkit.lsp.manifest' | 'aws.amazonq.customization.overrideV2' | 'aws.amazonq.regionProfiles' + | 'aws.amazonq.regionProfiles.cache' // Deprecated/legacy names. New keys should start with "aws.". | '#sessionCreationDates' // Legacy name from `ssoAccessTokenProvider.ts`. | 'CODECATALYST_RECONNECT' diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts new file mode 100644 index 00000000000..eeb7a905a17 --- /dev/null +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -0,0 +1,113 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import globals from '../extensionGlobals' +import { globalKey } from '../globalState' +import { getLogger } from '../logger/logger' +import { waitUntil } from '../utilities/timeoutUtils' + +interface WithLock { + locked: boolean + timestamp: number +} + +interface Resource extends WithLock { + result: V | undefined +} + +const logger = getLogger() + +function now() { + return globals.clock.Date.now() +} + +export abstract class CachedResource { + constructor( + readonly key: globalKey, + readonly expiration: number, + private readonly defaultValue: Resource + ) {} + + abstract resourceProvider(): Promise + + async getResource(): Promise { + const resource = await this.readResourceAndLock() + // if cache is still fresh, return + if (resource && resource.result) { + if (now() - resource.timestamp < this.expiration) { + logger.info(`cache hit`) + // release the lock + await globals.globalState.update(this.key, { + ...resource, + locked: false, + }) + return resource.result + } else { + logger.info(`cache hit but cached value is stale, invoking service API to pull the latest response`) + } + } + + // catch and error case? + logger.info(`cache miss, invoking service API to pull the latest response`) + const latest = await this.resourceProvider() + + // update resource cache and release the lock + const toUpdate: Resource = { + locked: false, + timestamp: now(), + result: latest, + } + await globals.globalState.update(this.key, toUpdate) + return latest + } + + async readResourceAndLock(): Promise | undefined> { + const _acquireLock = async () => { + const cachedValue = this.readCacheOrDefault() + + if (!cachedValue.locked) { + await globals.globalState.update(this.key, { + ...cachedValue, + locked: true, + }) + + return cachedValue + } + + return undefined + } + + const lock = await waitUntil( + async () => { + const lock = await _acquireLock() + logger.info(`try obtaining cache lock %s`, lock) + if (lock) { + return lock + } + }, + { timeout: 15000, interval: 1500, truthy: true } // TODO: pass via ctor + ) + + return lock + } + + async releaseLock() { + await globals.globalState.update(this.key, { + ...this.readCacheOrDefault(), + locked: false, + }) + } + + private readCacheOrDefault(): Resource { + const cachedValue = globals.globalState.tryGet>(this.key, Object, { + ...this.defaultValue, + locked: false, + result: undefined, + timestamp: 0, + }) + + return cachedValue + } +} From 1daab83cfe8ba259bd400804f1f41e64f89202d8 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 05:20:12 -0700 Subject: [PATCH 02/29] nit --- packages/core/src/shared/utilities/resourceCache.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index eeb7a905a17..0784e1884ac 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -26,7 +26,7 @@ function now() { export abstract class CachedResource { constructor( readonly key: globalKey, - readonly expiration: number, + readonly expirationInMilli: number, private readonly defaultValue: Resource ) {} @@ -36,7 +36,7 @@ export abstract class CachedResource { const resource = await this.readResourceAndLock() // if cache is still fresh, return if (resource && resource.result) { - if (now() - resource.timestamp < this.expiration) { + if (now() - resource.timestamp < this.expirationInMilli) { logger.info(`cache hit`) // release the lock await globals.globalState.update(this.key, { From 7380422ba6f1b065de35a705a9ff5aab3cb051bf Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 05:52:09 -0700 Subject: [PATCH 03/29] refacotr resourceCache.ts not verified yet --- .../src/shared/utilities/resourceCache.ts | 60 ++++++++++++------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 0784e1884ac..0a43535eb15 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -8,13 +8,16 @@ import { globalKey } from '../globalState' import { getLogger } from '../logger/logger' import { waitUntil } from '../utilities/timeoutUtils' -interface WithLock { +interface Resource { + result: V | undefined locked: boolean timestamp: number } -interface Resource extends WithLock { - result: V | undefined +// GlobalStates schema, which is used for vscode global states deserialization +// [globals.globalState.tryGet] +interface GlobalStateSchema { + resource: Resource } const logger = getLogger() @@ -25,21 +28,23 @@ function now() { export abstract class CachedResource { constructor( - readonly key: globalKey, - readonly expirationInMilli: number, - private readonly defaultValue: Resource + private readonly key: globalKey, + private readonly expirationInMilli: number, + private readonly defaultValue: GlobalStateSchema ) {} abstract resourceProvider(): Promise async getResource(): Promise { - const resource = await this.readResourceAndLock() + const cachedValue = await this.readResourceAndLock() + const resource = cachedValue?.resource + // if cache is still fresh, return - if (resource && resource.result) { + if (cachedValue && resource && resource.result) { if (now() - resource.timestamp < this.expirationInMilli) { logger.info(`cache hit`) // release the lock - await globals.globalState.update(this.key, { + await this.updateCache(cachedValue, { ...resource, locked: false, }) @@ -54,25 +59,24 @@ export abstract class CachedResource { const latest = await this.resourceProvider() // update resource cache and release the lock - const toUpdate: Resource = { + const r: Resource = { locked: false, timestamp: now(), result: latest, } - await globals.globalState.update(this.key, toUpdate) + await this.updateCache(cachedValue, r) return latest } - async readResourceAndLock(): Promise | undefined> { + async readResourceAndLock(): Promise | undefined> { const _acquireLock = async () => { const cachedValue = this.readCacheOrDefault() - if (!cachedValue.locked) { - await globals.globalState.update(this.key, { - ...cachedValue, + if (!cachedValue.resource.locked) { + await this.updateCache(cachedValue, { + ...cachedValue.resource, locked: true, }) - return cachedValue } @@ -82,7 +86,7 @@ export abstract class CachedResource { const lock = await waitUntil( async () => { const lock = await _acquireLock() - logger.info(`try obtaining cache lock %s`, lock) + logger.info(`try obtain resource cache read lock %s`, lock) if (lock) { return lock } @@ -100,12 +104,24 @@ export abstract class CachedResource { }) } - private readCacheOrDefault(): Resource { - const cachedValue = globals.globalState.tryGet>(this.key, Object, { + private async updateCache(cache: GlobalStateSchema | undefined, resource: Resource) { + // TODO: undefined? + + await globals.globalState.update(this.key, { + ...cache, + resource: resource, + }) + } + + private readCacheOrDefault(): GlobalStateSchema { + const cachedValue = globals.globalState.tryGet>(this.key, Object, { ...this.defaultValue, - locked: false, - result: undefined, - timestamp: 0, + resource: { + ...this.defaultValue.resource, + locked: false, + result: undefined, + timestamp: 0, + }, }) return cachedValue From 84609eccc0e7151969b3c7394e618a71a587c17c Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 06:01:18 -0700 Subject: [PATCH 04/29] verified --- packages/core/src/shared/utilities/resourceCache.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 0a43535eb15..7426266915c 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -105,10 +105,8 @@ export abstract class CachedResource { } private async updateCache(cache: GlobalStateSchema | undefined, resource: Resource) { - // TODO: undefined? - await globals.globalState.update(this.key, { - ...cache, + ...(cache ? cache : this.readCacheOrDefault()), resource: resource, }) } From ed782e2c89adc3f1ae84a0ea58b7b397cafd6882 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 06:03:42 -0700 Subject: [PATCH 05/29] patch not verified --- .../core/src/codewhisperer/region/regionProfileManager.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index 88a4a04b67f..c71490a5df9 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -63,9 +63,11 @@ export class RegionProfileManager { private readonly cache = new (class extends CachedResource { constructor(private readonly profileProvider: () => Promise) { super('aws.amazonq.regionProfiles.cache', 60000, { - locked: false, - timestamp: 0, - result: undefined, + resource: { + locked: false, + timestamp: 0, + result: undefined, + }, }) } From 3e501456fb1cd7b125819db2fbe7390cdfdc2558 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 06:40:15 -0700 Subject: [PATCH 06/29] merge 2 global states variable but might need to revert because it will require users to reselect profile --- .../region/regionProfileManager.ts | 52 +++++++++++++++---- .../src/shared/utilities/resourceCache.ts | 2 +- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index c71490a5df9..9c035ceace3 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -29,7 +29,7 @@ import { isAwsError, ToolkitError } from '../../shared/errors' import { telemetry } from '../../shared/telemetry/telemetry' import { localize } from '../../shared/utilities/vsCodeUtils' import { Commands } from '../../shared/vscode/commands2' -import { CachedResource } from '../../shared/utilities/resourceCache' +import { CachedResource, GlobalStateSchema } from '../../shared/utilities/resourceCache' // TODO: is there a better way to manage all endpoint strings in one place? export const defaultServiceConfig: CodeWhispererConfig = { @@ -51,6 +51,10 @@ const endpoints = createConstantMap({ */ export type ProfileSwitchIntent = 'user' | 'auth' | 'update' | 'reload' +interface RegionProfileGlobalState extends GlobalStateSchema { + previousSelection: { [label: string]: RegionProfile } +} + export class RegionProfileManager { private static logger = getLogger() private _activeRegionProfile: RegionProfile | undefined @@ -62,7 +66,7 @@ export class RegionProfileManager { private readonly cache = new (class extends CachedResource { constructor(private readonly profileProvider: () => Promise) { - super('aws.amazonq.regionProfiles.cache', 60000, { + super('aws.amazonq.regionProfiles', 60000, { resource: { locked: false, timestamp: 0, @@ -293,13 +297,20 @@ export class RegionProfileManager { } private loadPersistedRegionProfle(): { [label: string]: RegionProfile } { - const previousPersistedState = globals.globalState.tryGet<{ [label: string]: RegionProfile }>( + const previousPersistedState = globals.globalState.tryGet( 'aws.amazonq.regionProfiles', Object, - {} + { + previousSelection: {}, + resource: { + locked: false, + result: undefined, + timestamp: 0, + }, + } ) - return previousPersistedState + return previousPersistedState.previousSelection } async persistSelectRegionProfile() { @@ -311,13 +322,20 @@ export class RegionProfileManager { } // persist connectionId to profileArn - const previousPersistedState = globals.globalState.tryGet<{ [label: string]: RegionProfile }>( + const previousPersistedState = globals.globalState.tryGet( 'aws.amazonq.regionProfiles', Object, - {} + { + previousSelection: {}, + resource: { + locked: false, + result: undefined, + timestamp: 0, + }, + } ) - previousPersistedState[conn.id] = this.activeRegionProfile + previousPersistedState.previousSelection[conn.id] = this.activeRegionProfile await globals.globalState.update('aws.amazonq.regionProfiles', previousPersistedState) } @@ -368,7 +386,23 @@ export class RegionProfileManager { const updatedProfiles = Object.fromEntries( Object.entries(profiles).filter(([connId, profile]) => profile.arn !== arn) ) - await globals.globalState.update('aws.amazonq.regionProfiles', updatedProfiles) + const previousPersistedState = globals.globalState.tryGet( + 'aws.amazonq.regionProfiles', + Object, + { + previousSelection: {}, + resource: { + locked: false, + result: undefined, + timestamp: 0, + }, + } + ) + const toUpdate: RegionProfileGlobalState = { + previousSelection: updatedProfiles, + resource: previousPersistedState.resource, + } + await globals.globalState.update('aws.amazonq.regionProfiles', toUpdate) } } diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 7426266915c..f8ed5306a21 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -16,7 +16,7 @@ interface Resource { // GlobalStates schema, which is used for vscode global states deserialization // [globals.globalState.tryGet] -interface GlobalStateSchema { +export interface GlobalStateSchema { resource: Resource } From 0392707426aabbabc99c7f2eef4dc5bce8a5f671 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 15:13:13 -0700 Subject: [PATCH 07/29] Revert "merge 2 global states variable but might need to revert because it will require users to reselect profile" This reverts commit 3e501456fb1cd7b125819db2fbe7390cdfdc2558. --- .../region/regionProfileManager.ts | 52 ++++--------------- .../src/shared/utilities/resourceCache.ts | 2 +- 2 files changed, 10 insertions(+), 44 deletions(-) diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index 9c035ceace3..c71490a5df9 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -29,7 +29,7 @@ import { isAwsError, ToolkitError } from '../../shared/errors' import { telemetry } from '../../shared/telemetry/telemetry' import { localize } from '../../shared/utilities/vsCodeUtils' import { Commands } from '../../shared/vscode/commands2' -import { CachedResource, GlobalStateSchema } from '../../shared/utilities/resourceCache' +import { CachedResource } from '../../shared/utilities/resourceCache' // TODO: is there a better way to manage all endpoint strings in one place? export const defaultServiceConfig: CodeWhispererConfig = { @@ -51,10 +51,6 @@ const endpoints = createConstantMap({ */ export type ProfileSwitchIntent = 'user' | 'auth' | 'update' | 'reload' -interface RegionProfileGlobalState extends GlobalStateSchema { - previousSelection: { [label: string]: RegionProfile } -} - export class RegionProfileManager { private static logger = getLogger() private _activeRegionProfile: RegionProfile | undefined @@ -66,7 +62,7 @@ export class RegionProfileManager { private readonly cache = new (class extends CachedResource { constructor(private readonly profileProvider: () => Promise) { - super('aws.amazonq.regionProfiles', 60000, { + super('aws.amazonq.regionProfiles.cache', 60000, { resource: { locked: false, timestamp: 0, @@ -297,20 +293,13 @@ export class RegionProfileManager { } private loadPersistedRegionProfle(): { [label: string]: RegionProfile } { - const previousPersistedState = globals.globalState.tryGet( + const previousPersistedState = globals.globalState.tryGet<{ [label: string]: RegionProfile }>( 'aws.amazonq.regionProfiles', Object, - { - previousSelection: {}, - resource: { - locked: false, - result: undefined, - timestamp: 0, - }, - } + {} ) - return previousPersistedState.previousSelection + return previousPersistedState } async persistSelectRegionProfile() { @@ -322,20 +311,13 @@ export class RegionProfileManager { } // persist connectionId to profileArn - const previousPersistedState = globals.globalState.tryGet( + const previousPersistedState = globals.globalState.tryGet<{ [label: string]: RegionProfile }>( 'aws.amazonq.regionProfiles', Object, - { - previousSelection: {}, - resource: { - locked: false, - result: undefined, - timestamp: 0, - }, - } + {} ) - previousPersistedState.previousSelection[conn.id] = this.activeRegionProfile + previousPersistedState[conn.id] = this.activeRegionProfile await globals.globalState.update('aws.amazonq.regionProfiles', previousPersistedState) } @@ -386,23 +368,7 @@ export class RegionProfileManager { const updatedProfiles = Object.fromEntries( Object.entries(profiles).filter(([connId, profile]) => profile.arn !== arn) ) - const previousPersistedState = globals.globalState.tryGet( - 'aws.amazonq.regionProfiles', - Object, - { - previousSelection: {}, - resource: { - locked: false, - result: undefined, - timestamp: 0, - }, - } - ) - const toUpdate: RegionProfileGlobalState = { - previousSelection: updatedProfiles, - resource: previousPersistedState.resource, - } - await globals.globalState.update('aws.amazonq.regionProfiles', toUpdate) + await globals.globalState.update('aws.amazonq.regionProfiles', updatedProfiles) } } diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index f8ed5306a21..7426266915c 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -16,7 +16,7 @@ interface Resource { // GlobalStates schema, which is used for vscode global states deserialization // [globals.globalState.tryGet] -export interface GlobalStateSchema { +interface GlobalStateSchema { resource: Resource } From 95d5c9ef6e231f99b3b3933808f2d6bb107a45a3 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 15:28:53 -0700 Subject: [PATCH 08/29] cl --- .../Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json diff --git a/packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json b/packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json new file mode 100644 index 00000000000..087dd85d88d --- /dev/null +++ b/packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "Fix users can not log in successfully with 2+ IDE instnaces open due to throttle error throw by the service" +} From b7144f0b101fb969fd17cfd0af08de9bc889a19c Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 15:37:50 -0700 Subject: [PATCH 09/29] private --- packages/core/src/shared/utilities/resourceCache.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 7426266915c..1b1601c120e 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -68,7 +68,7 @@ export abstract class CachedResource { return latest } - async readResourceAndLock(): Promise | undefined> { + private async readResourceAndLock(): Promise | undefined> { const _acquireLock = async () => { const cachedValue = this.readCacheOrDefault() @@ -97,13 +97,6 @@ export abstract class CachedResource { return lock } - async releaseLock() { - await globals.globalState.update(this.key, { - ...this.readCacheOrDefault(), - locked: false, - }) - } - private async updateCache(cache: GlobalStateSchema | undefined, resource: Resource) { await globals.globalState.update(this.key, { ...(cache ? cache : this.readCacheOrDefault()), From 3fec764735bbe460a5293d80340a16832459cc03 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 15:42:52 -0700 Subject: [PATCH 10/29] compile error --- packages/core/src/shared/utilities/resourceCache.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 1b1601c120e..0cdfbac74a9 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -97,6 +97,13 @@ export abstract class CachedResource { return lock } + async releaseLock() { + await globals.globalState.update(this.key, { + ...this.readCacheOrDefault(), + locked: false, + }) + } + private async updateCache(cache: GlobalStateSchema | undefined, resource: Resource) { await globals.globalState.update(this.key, { ...(cache ? cache : this.readCacheOrDefault()), From 57e81a19e4f81046af1b9fe91cadb25694f49b34 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 16:32:52 -0700 Subject: [PATCH 11/29] some docstr --- packages/core/src/shared/utilities/resourceCache.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 0cdfbac74a9..ea78c5eebe4 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -39,7 +39,7 @@ export abstract class CachedResource { const cachedValue = await this.readResourceAndLock() const resource = cachedValue?.resource - // if cache is still fresh, return + // If cache is still fresh, return cached result, otherwise pull latest from the service if (cachedValue && resource && resource.result) { if (now() - resource.timestamp < this.expirationInMilli) { logger.info(`cache hit`) @@ -58,7 +58,7 @@ export abstract class CachedResource { logger.info(`cache miss, invoking service API to pull the latest response`) const latest = await this.resourceProvider() - // update resource cache and release the lock + // Update resource cache and release the lock const r: Resource = { locked: false, timestamp: now(), @@ -68,6 +68,7 @@ export abstract class CachedResource { return latest } + // This method will lock the resource so other callers have to wait until the lock is released, otherwise will return undefined if it times out private async readResourceAndLock(): Promise | undefined> { const _acquireLock = async () => { const cachedValue = this.readCacheOrDefault() From d388b8fb27a0ab60593c5d584d5cb63c0694c398 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 16:37:42 -0700 Subject: [PATCH 12/29] try catch --- .../src/shared/utilities/resourceCache.ts | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index ea78c5eebe4..dad4b36398a 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -54,18 +54,22 @@ export abstract class CachedResource { } } - // catch and error case? logger.info(`cache miss, invoking service API to pull the latest response`) - const latest = await this.resourceProvider() + try { + const latest = await this.resourceProvider() - // Update resource cache and release the lock - const r: Resource = { - locked: false, - timestamp: now(), - result: latest, + // Update resource cache and release the lock + const r: Resource = { + locked: false, + timestamp: now(), + result: latest, + } + await this.updateCache(cachedValue, r) + return latest + } catch (e) { + await this.releaseLock() + throw e } - await this.updateCache(cachedValue, r) - return latest } // This method will lock the resource so other callers have to wait until the lock is released, otherwise will return undefined if it times out From e50bfabc0766c9dd1fef90b3f9df5a73dca130d3 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 17:53:18 -0700 Subject: [PATCH 13/29] pass waitUntil option via ctor --- .../region/regionProfileManager.ts | 17 ++++++++++------ .../src/shared/utilities/resourceCache.ts | 20 +++++++++---------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index c71490a5df9..c1ab94c7174 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -62,13 +62,18 @@ export class RegionProfileManager { private readonly cache = new (class extends CachedResource { constructor(private readonly profileProvider: () => Promise) { - super('aws.amazonq.regionProfiles.cache', 60000, { - resource: { - locked: false, - timestamp: 0, - result: undefined, + super( + 'aws.amazonq.regionProfiles.cache', + 60000, + { + resource: { + locked: false, + timestamp: 0, + result: undefined, + }, }, - }) + { timeout: 15000, interval: 1500, truthy: true } + ) } override resourceProvider(): Promise { diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index dad4b36398a..3ef16b9949b 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -30,7 +30,8 @@ export abstract class CachedResource { constructor( private readonly key: globalKey, private readonly expirationInMilli: number, - private readonly defaultValue: GlobalStateSchema + private readonly defaultValue: GlobalStateSchema, + private readonly waitUntilOption: { timeout: number; interval: number; truthy: boolean } ) {} abstract resourceProvider(): Promise @@ -88,16 +89,13 @@ export abstract class CachedResource { return undefined } - const lock = await waitUntil( - async () => { - const lock = await _acquireLock() - logger.info(`try obtain resource cache read lock %s`, lock) - if (lock) { - return lock - } - }, - { timeout: 15000, interval: 1500, truthy: true } // TODO: pass via ctor - ) + const lock = await waitUntil(async () => { + const lock = await _acquireLock() + logger.info(`try obtain resource cache read lock %s`, lock) + if (lock) { + return lock + } + }, this.waitUntilOption) return lock } From 69a474e7f628f0c8a55f75039cf998cc6e4c3a40 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 17:59:50 -0700 Subject: [PATCH 14/29] docstr --- packages/core/src/shared/utilities/resourceCache.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 3ef16b9949b..d24dcb065b5 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -26,6 +26,17 @@ function now() { return globals.clock.Date.now() } +/** + * args: + * key: global state key, which is used for globals.globalState#update, #tryGet etc. + * expirationInMilli: cache expiration time in milli seconds + * defaultValue: default value for the cache if the cache doesn't pre-exist in users' FS + * waitUntilOption: waitUntil option for acquire lock + * + * methods: + * resourceProvider(): implementation needs to implement this method to obtain the latest resource either via network calls or FS read + * getResource(): obtain the resource from cache or pull the latest from the service if the cache either expires or doesn't exist + */ export abstract class CachedResource { constructor( private readonly key: globalKey, @@ -100,6 +111,7 @@ export abstract class CachedResource { return lock } + // TODO: releaseLock and updateCache do similar things, how to improve async releaseLock() { await globals.globalState.update(this.key, { ...this.readCacheOrDefault(), From 33dd9bf8cb1ea788aedae988fa503552e6709bf9 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 18:11:58 -0700 Subject: [PATCH 15/29] docstr --- .../src/shared/utilities/resourceCache.ts | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index d24dcb065b5..e7f73c86b69 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -8,15 +8,22 @@ import { globalKey } from '../globalState' import { getLogger } from '../logger/logger' import { waitUntil } from '../utilities/timeoutUtils' +/** + * result: the actual resource type callers want to use + * locked: readWriteLock, while the lock is acquired by one process, the other can't access to it until it's released by the previous + * timestamp: used for determining the resource is stale or not + */ interface Resource { result: V | undefined locked: boolean timestamp: number } -// GlobalStates schema, which is used for vscode global states deserialization -// [globals.globalState.tryGet] -interface GlobalStateSchema { +/** + * GlobalStates schema, which is used for vscode global states deserialization, [globals.globalState#tryGet] etc. + * The purpose of it is to allow devs to overload the resource into existing global key and no need to create a specific key for only this purpose. + */ +export interface GlobalStateSchema { resource: Resource } @@ -53,8 +60,9 @@ export abstract class CachedResource { // If cache is still fresh, return cached result, otherwise pull latest from the service if (cachedValue && resource && resource.result) { - if (now() - resource.timestamp < this.expirationInMilli) { - logger.info(`cache hit`) + const duration = now() - resource.timestamp + if (duration < this.expirationInMilli) { + logger.info(`cache hit, duration(%sms) is less than expiration(%sms)`, duration, this.expirationInMilli) // release the lock await this.updateCache(cachedValue, { ...resource, @@ -62,7 +70,11 @@ export abstract class CachedResource { }) return resource.result } else { - logger.info(`cache hit but cached value is stale, invoking service API to pull the latest response`) + logger.info( + `cached value is stale, duration(%sms) is older than expiration(%sms), invoking service API to pull the latest response`, + duration, + this.expirationInMilli + ) } } From d52d65944a7d232aabd60af0dbef7a7e8361c174 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 18:13:53 -0700 Subject: [PATCH 16/29] docstr --- .../src/shared/utilities/resourceCache.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index e7f73c86b69..7f35b3c5c62 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -9,9 +9,10 @@ import { getLogger } from '../logger/logger' import { waitUntil } from '../utilities/timeoutUtils' /** - * result: the actual resource type callers want to use - * locked: readWriteLock, while the lock is acquired by one process, the other can't access to it until it's released by the previous - * timestamp: used for determining the resource is stale or not + * args: + * [result]: the actual resource type callers want to use + * [locked]: readWriteLock, while the lock is acquired by one process, the other can't access to it until it's released by the previous + * [timestamp]: used for determining the resource is stale or not */ interface Resource { result: V | undefined @@ -35,14 +36,14 @@ function now() { /** * args: - * key: global state key, which is used for globals.globalState#update, #tryGet etc. - * expirationInMilli: cache expiration time in milli seconds - * defaultValue: default value for the cache if the cache doesn't pre-exist in users' FS - * waitUntilOption: waitUntil option for acquire lock + * [key]: global state key, which is used for globals.globalState#update, #tryGet etc. + * [expirationInMilli]: cache expiration time in milli seconds + * [defaultValue]: default value for the cache if the cache doesn't pre-exist in users' FS + * [waitUntilOption]: waitUntil option for acquire lock * * methods: - * resourceProvider(): implementation needs to implement this method to obtain the latest resource either via network calls or FS read - * getResource(): obtain the resource from cache or pull the latest from the service if the cache either expires or doesn't exist + * #resourceProvider: implementation needs to implement this method to obtain the latest resource either via network calls or FS read + * #getResource: obtain the resource from cache or pull the latest from the service if the cache either expires or doesn't exist */ export abstract class CachedResource { constructor( From 4c8a6b59c47aa7220421c6733b233065baace9ec Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 18:20:38 -0700 Subject: [PATCH 17/29] docstr --- packages/core/src/shared/utilities/resourceCache.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 7f35b3c5c62..7a101418a8a 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -81,6 +81,7 @@ export abstract class CachedResource { logger.info(`cache miss, invoking service API to pull the latest response`) try { + // Make the real network call / FS read to pull the resource const latest = await this.resourceProvider() // Update resource cache and release the lock From a3514ff04ab11ae75099f4fa53bdd089841d9733 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 18:24:59 -0700 Subject: [PATCH 18/29] rename --- packages/core/src/shared/utilities/resourceCache.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 7a101418a8a..7920aefc3ad 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -56,7 +56,7 @@ export abstract class CachedResource { abstract resourceProvider(): Promise async getResource(): Promise { - const cachedValue = await this.readResourceAndLock() + const cachedValue = await this.tryLoadResourceAndLock() const resource = cachedValue?.resource // If cache is still fresh, return cached result, otherwise pull latest from the service @@ -99,7 +99,7 @@ export abstract class CachedResource { } // This method will lock the resource so other callers have to wait until the lock is released, otherwise will return undefined if it times out - private async readResourceAndLock(): Promise | undefined> { + private async tryLoadResourceAndLock(): Promise | undefined> { const _acquireLock = async () => { const cachedValue = this.readCacheOrDefault() From 9eb095418fb61207b04a1e71a5e787a59b537b55 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 18:30:06 -0700 Subject: [PATCH 19/29] log --- packages/core/src/shared/utilities/resourceCache.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 7920aefc3ad..b7220b5e180 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -90,9 +90,14 @@ export abstract class CachedResource { timestamp: now(), result: latest, } + logger.info(`doen loading the latest of resource(%s), updating resource cache`, this.key) await this.updateCache(cachedValue, r) return latest } catch (e) { + logger.info( + `encountered unexpected error while loading the latest of resource(%s), releasing resource lock`, + this.key + ) await this.releaseLock() throw e } From 42c345110dd2508fed1b60b233ac9f53854954c7 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 18:38:37 -0700 Subject: [PATCH 20/29] docstr --- packages/core/src/shared/utilities/resourceCache.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index b7220b5e180..b77e83b2eec 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -79,6 +79,12 @@ export abstract class CachedResource { } } + /** + * Possible paths here + * 1. cache doesn't exist. + * 2. cache exists but expired. + * 3. lock is held by other process and the waiting time is greater than the specified waiting time + */ logger.info(`cache miss, invoking service API to pull the latest response`) try { // Make the real network call / FS read to pull the resource From b06932b879a0c0555ce2b8a85e632dab0521c475 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 18:44:33 -0700 Subject: [PATCH 21/29] rename updateResourceCache --- .../core/src/shared/utilities/resourceCache.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index b77e83b2eec..d317e0ec2cf 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -65,7 +65,7 @@ export abstract class CachedResource { if (duration < this.expirationInMilli) { logger.info(`cache hit, duration(%sms) is less than expiration(%sms)`, duration, this.expirationInMilli) // release the lock - await this.updateCache(cachedValue, { + await this.updateResourceCache(cachedValue, { ...resource, locked: false, }) @@ -97,7 +97,7 @@ export abstract class CachedResource { result: latest, } logger.info(`doen loading the latest of resource(%s), updating resource cache`, this.key) - await this.updateCache(cachedValue, r) + await this.updateResourceCache(cachedValue, r) return latest } catch (e) { logger.info( @@ -115,7 +115,7 @@ export abstract class CachedResource { const cachedValue = this.readCacheOrDefault() if (!cachedValue.resource.locked) { - await this.updateCache(cachedValue, { + await this.updateResourceCache(cachedValue, { ...cachedValue.resource, locked: true, }) @@ -144,11 +144,13 @@ export abstract class CachedResource { }) } - private async updateCache(cache: GlobalStateSchema | undefined, resource: Resource) { - await globals.globalState.update(this.key, { + private async updateResourceCache(cache: GlobalStateSchema | undefined, resource: Resource) { + const toUpdate: GlobalStateSchema = { ...(cache ? cache : this.readCacheOrDefault()), resource: resource, - }) + } + + await globals.globalState.update(this.key, toUpdate) } private readCacheOrDefault(): GlobalStateSchema { From 9b57d7b49494e446459c03ca71f4d68263236c50 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 20:58:57 -0700 Subject: [PATCH 22/29] docstr --- .../src/shared/utilities/resourceCache.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index d317e0ec2cf..c9d3d85585b 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -10,9 +10,9 @@ import { waitUntil } from '../utilities/timeoutUtils' /** * args: - * [result]: the actual resource type callers want to use - * [locked]: readWriteLock, while the lock is acquired by one process, the other can't access to it until it's released by the previous - * [timestamp]: used for determining the resource is stale or not + * @member result: the actual resource type callers want to use + * @member locked: readWriteLock, while the lock is acquired by one process, the other can't access to it until it's released by the previous + * @member timestamp: used for determining the resource is stale or not */ interface Resource { result: V | undefined @@ -35,15 +35,15 @@ function now() { } /** - * args: - * [key]: global state key, which is used for globals.globalState#update, #tryGet etc. - * [expirationInMilli]: cache expiration time in milli seconds - * [defaultValue]: default value for the cache if the cache doesn't pre-exist in users' FS - * [waitUntilOption]: waitUntil option for acquire lock + * constructor: + * @param key: global state key, which is used for globals.globalState#update, #tryGet etc. + * @param expirationInMilli: cache expiration time in milli seconds + * @param defaultValue: default value for the cache if the cache doesn't pre-exist in users' FS + * @param waitUntilOption: waitUntil option for acquire lock * * methods: - * #resourceProvider: implementation needs to implement this method to obtain the latest resource either via network calls or FS read - * #getResource: obtain the resource from cache or pull the latest from the service if the cache either expires or doesn't exist + * @method resourceProvider: implementation needs to implement this method to obtain the latest resource either via network calls or FS read + * @method getResource: obtain the resource from cache or pull the latest from the service if the cache either expires or doesn't exist */ export abstract class CachedResource { constructor( From 25b04d80bb90aae7cf4c7705326407b3fe4b73f3 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Thu, 24 Apr 2025 21:56:53 -0700 Subject: [PATCH 23/29] refactor and overload releaseLock instead of using updateResourceCache --- .../src/shared/utilities/resourceCache.ts | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index c9d3d85585b..1dc877652ad 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -65,10 +65,7 @@ export abstract class CachedResource { if (duration < this.expirationInMilli) { logger.info(`cache hit, duration(%sms) is less than expiration(%sms)`, duration, this.expirationInMilli) // release the lock - await this.updateResourceCache(cachedValue, { - ...resource, - locked: false, - }) + await this.releaseLock(resource, cachedValue) return resource.result } else { logger.info( @@ -97,7 +94,7 @@ export abstract class CachedResource { result: latest, } logger.info(`doen loading the latest of resource(%s), updating resource cache`, this.key) - await this.updateResourceCache(cachedValue, r) + await this.releaseLock(r) return latest } catch (e) { logger.info( @@ -115,10 +112,7 @@ export abstract class CachedResource { const cachedValue = this.readCacheOrDefault() if (!cachedValue.resource.locked) { - await this.updateResourceCache(cachedValue, { - ...cachedValue.resource, - locked: true, - }) + await this.lockResource(cachedValue) return cachedValue } @@ -136,18 +130,32 @@ export abstract class CachedResource { return lock } - // TODO: releaseLock and updateCache do similar things, how to improve - async releaseLock() { - await globals.globalState.update(this.key, { - ...this.readCacheOrDefault(), - locked: false, - }) + async lockResource(baseCache: GlobalStateSchema): Promise { + await this.updateResourceCache({ locked: true }, baseCache) + } + + async releaseLock(): Promise + async releaseLock(resource: Partial>): Promise + async releaseLock(resource: Partial>, baseCache: GlobalStateSchema): Promise + async releaseLock(resource?: Partial>, baseCache?: GlobalStateSchema): Promise { + if (!resource) { + await this.updateResourceCache({ locked: false }, undefined) + } else if (baseCache) { + await this.updateResourceCache(resource, baseCache) + } else { + await this.updateResourceCache(resource, undefined) + } } - private async updateResourceCache(cache: GlobalStateSchema | undefined, resource: Resource) { + private async updateResourceCache(resource: Partial>, cache: GlobalStateSchema | undefined) { + const baseCache = cache ?? this.readCacheOrDefault() + const toUpdate: GlobalStateSchema = { - ...(cache ? cache : this.readCacheOrDefault()), - resource: resource, + ...baseCache, + resource: { + ...baseCache.resource, + ...resource, + }, } await globals.globalState.update(this.key, toUpdate) From f6f17720fae4bcaa2846dca02fc2e7a8035bb790 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Fri, 25 Apr 2025 08:51:49 -0700 Subject: [PATCH 24/29] update log level / topic --- packages/core/src/shared/logger/logger.ts | 1 + .../core/src/shared/utilities/resourceCache.ts | 18 +++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/core/src/shared/logger/logger.ts b/packages/core/src/shared/logger/logger.ts index 9b4bead6a37..eac564b9c35 100644 --- a/packages/core/src/shared/logger/logger.ts +++ b/packages/core/src/shared/logger/logger.ts @@ -18,6 +18,7 @@ export type LogTopic = | 'chat' | 'stepfunctions' | 'unknown' + | 'resourceCache' class ErrorLog { constructor( diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 1dc877652ad..df77a29667a 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -28,7 +28,7 @@ export interface GlobalStateSchema { resource: Resource } -const logger = getLogger() +const logger = getLogger('resourceCache') function now() { return globals.clock.Date.now() @@ -63,12 +63,16 @@ export abstract class CachedResource { if (cachedValue && resource && resource.result) { const duration = now() - resource.timestamp if (duration < this.expirationInMilli) { - logger.info(`cache hit, duration(%sms) is less than expiration(%sms)`, duration, this.expirationInMilli) + logger.debug( + `cache hit, duration(%sms) is less than expiration(%sms)`, + duration, + this.expirationInMilli + ) // release the lock await this.releaseLock(resource, cachedValue) return resource.result } else { - logger.info( + logger.debug( `cached value is stale, duration(%sms) is older than expiration(%sms), invoking service API to pull the latest response`, duration, this.expirationInMilli @@ -82,7 +86,7 @@ export abstract class CachedResource { * 2. cache exists but expired. * 3. lock is held by other process and the waiting time is greater than the specified waiting time */ - logger.info(`cache miss, invoking service API to pull the latest response`) + logger.debug(`cache miss, invoking service API to pull the latest response`) try { // Make the real network call / FS read to pull the resource const latest = await this.resourceProvider() @@ -93,11 +97,11 @@ export abstract class CachedResource { timestamp: now(), result: latest, } - logger.info(`doen loading the latest of resource(%s), updating resource cache`, this.key) + logger.debug(`doen loading the latest of resource(%s), updating resource cache`, this.key) await this.releaseLock(r) return latest } catch (e) { - logger.info( + logger.debug( `encountered unexpected error while loading the latest of resource(%s), releasing resource lock`, this.key ) @@ -121,7 +125,7 @@ export abstract class CachedResource { const lock = await waitUntil(async () => { const lock = await _acquireLock() - logger.info(`try obtain resource cache read lock %s`, lock) + logger.debug(`try obtain resource cache read write lock for resource %s`, this.key) if (lock) { return lock } From cdb9bc879e4816538553a4bbb17cc48f446fcd59 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Fri, 25 Apr 2025 09:19:53 -0700 Subject: [PATCH 25/29] * update log string * remove regionProfileManager#resetCache --- .../codewhisperer/region/regionProfileManager.ts | 4 ---- packages/core/src/codewhisperer/util/authUtil.ts | 1 - .../core/src/shared/utilities/resourceCache.ts | 15 +++++++++------ 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index c1ab94c7174..27c6c29b4fd 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -126,10 +126,6 @@ export class RegionProfileManager { constructor(private readonly connectionProvider: () => Connection | undefined) {} - async resetCache() { - await this.cache.releaseLock() - } - async getProfiles(): Promise { return this.cache.getResource() } diff --git a/packages/core/src/codewhisperer/util/authUtil.ts b/packages/core/src/codewhisperer/util/authUtil.ts index 7deae72a1c1..0898493b6db 100644 --- a/packages/core/src/codewhisperer/util/authUtil.ts +++ b/packages/core/src/codewhisperer/util/authUtil.ts @@ -142,7 +142,6 @@ export class AuthUtil { if (!this.isConnected()) { await this.regionProfileManager.invalidateProfile(this.regionProfileManager.activeRegionProfile?.arn) - await this.regionProfileManager.resetCache() } }) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index df77a29667a..85e7384b69a 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -64,20 +64,24 @@ export abstract class CachedResource { const duration = now() - resource.timestamp if (duration < this.expirationInMilli) { logger.debug( - `cache hit, duration(%sms) is less than expiration(%sms)`, + `cache hit, duration(%sms) is less than expiration(%sms), returning cached value %s`, duration, - this.expirationInMilli + this.expirationInMilli, + this.key ) // release the lock await this.releaseLock(resource, cachedValue) return resource.result } else { logger.debug( - `cached value is stale, duration(%sms) is older than expiration(%sms), invoking service API to pull the latest response`, + `cache is stale, duration(%sms) is older than expiration(%sms), pulling latest resource %s`, duration, - this.expirationInMilli + this.expirationInMilli, + this.key ) } + } else { + logger.debug(`cache miss, pulling latest resource %s`, this.key) } /** @@ -86,7 +90,6 @@ export abstract class CachedResource { * 2. cache exists but expired. * 3. lock is held by other process and the waiting time is greater than the specified waiting time */ - logger.debug(`cache miss, invoking service API to pull the latest response`) try { // Make the real network call / FS read to pull the resource const latest = await this.resourceProvider() @@ -97,7 +100,7 @@ export abstract class CachedResource { timestamp: now(), result: latest, } - logger.debug(`doen loading the latest of resource(%s), updating resource cache`, this.key) + logger.debug(`doen loading latest resource, updating resource cache: %s`, this.key) await this.releaseLock(r) return latest } catch (e) { From 97a44821ee9bb65cdc737011e81dc6e87ab23136 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Fri, 25 Apr 2025 09:34:32 -0700 Subject: [PATCH 26/29] add clearCache --- .../core/src/codewhisperer/region/regionProfileManager.ts | 5 +++++ packages/core/src/codewhisperer/util/authUtil.ts | 2 ++ packages/core/src/shared/utilities/resourceCache.ts | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index 27c6c29b4fd..a28ac46ee1c 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -373,6 +373,11 @@ export class RegionProfileManager { } } + // Should be called on connection changed in case users change to a differnet connection and use the wrong resultset. + async clearCache() { + await this.cache.clearCache() + } + async createQClient(region: string, endpoint: string, conn: SsoConnection): Promise { const token = (await conn.getToken()).accessToken const serviceOption: ServiceOptions = { diff --git a/packages/core/src/codewhisperer/util/authUtil.ts b/packages/core/src/codewhisperer/util/authUtil.ts index 0898493b6db..c6458e9b6db 100644 --- a/packages/core/src/codewhisperer/util/authUtil.ts +++ b/packages/core/src/codewhisperer/util/authUtil.ts @@ -143,6 +143,8 @@ export class AuthUtil { if (!this.isConnected()) { await this.regionProfileManager.invalidateProfile(this.regionProfileManager.activeRegionProfile?.arn) } + + await this.regionProfileManager.clearCache() }) this.regionProfileManager.onDidChangeRegionProfile(async () => { diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 85e7384b69a..a9d8e715e24 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -154,6 +154,11 @@ export abstract class CachedResource { } } + async clearCache() { + const baseCache = this.readCacheOrDefault() + await this.updateResourceCache({ result: undefined, timestamp: 0, locked: false }, baseCache) + } + private async updateResourceCache(resource: Partial>, cache: GlobalStateSchema | undefined) { const baseCache = cache ?? this.readCacheOrDefault() From 8466c958c1a300c2b61fd9da4abf2073306a41f7 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Fri, 25 Apr 2025 10:01:06 -0700 Subject: [PATCH 27/29] docstr --- packages/core/src/shared/utilities/resourceCache.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index a9d8e715e24..de39836fa43 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -35,6 +35,9 @@ function now() { } /** + * CacheResource utilizes VSCode global states API to cache resources which are expensive to get so that the result can be shared across multiple VSCode instances. + * The first VSCode instance invoking #getResource will hold a lock and make the actual network call/FS read to pull the real response. + * When the pull is done, the lock will be released and it then caches the result in the global states. Then the rest of instances can now acquire the lock 1 by 1 and read the resource from the cache. * constructor: * @param key: global state key, which is used for globals.globalState#update, #tryGet etc. * @param expirationInMilli: cache expiration time in milli seconds From b7d5b73b3dc8e6d85f355e6b2ec40954ea4f7c32 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Fri, 25 Apr 2025 10:03:58 -0700 Subject: [PATCH 28/29] newline --- packages/core/src/shared/utilities/resourceCache.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index de39836fa43..622d03f4de9 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -38,6 +38,7 @@ function now() { * CacheResource utilizes VSCode global states API to cache resources which are expensive to get so that the result can be shared across multiple VSCode instances. * The first VSCode instance invoking #getResource will hold a lock and make the actual network call/FS read to pull the real response. * When the pull is done, the lock will be released and it then caches the result in the global states. Then the rest of instances can now acquire the lock 1 by 1 and read the resource from the cache. + * * constructor: * @param key: global state key, which is used for globals.globalState#update, #tryGet etc. * @param expirationInMilli: cache expiration time in milli seconds From 6a0c54f775a07405ef9b41f1ee705e78985d8c90 Mon Sep 17 00:00:00 2001 From: Will Lo Date: Fri, 25 Apr 2025 10:52:56 -0700 Subject: [PATCH 29/29] update log level --- packages/core/src/shared/utilities/resourceCache.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts index 622d03f4de9..8189f40a4c5 100644 --- a/packages/core/src/shared/utilities/resourceCache.ts +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -85,7 +85,7 @@ export abstract class CachedResource { ) } } else { - logger.debug(`cache miss, pulling latest resource %s`, this.key) + logger.info(`cache miss, pulling latest resource %s`, this.key) } /** @@ -104,14 +104,11 @@ export abstract class CachedResource { timestamp: now(), result: latest, } - logger.debug(`doen loading latest resource, updating resource cache: %s`, this.key) + logger.info(`doen loading latest resource, updating resource cache: %s`, this.key) await this.releaseLock(r) return latest } catch (e) { - logger.debug( - `encountered unexpected error while loading the latest of resource(%s), releasing resource lock`, - this.key - ) + logger.error(`failed to load latest resource, releasing lock: %s`, this.key) await this.releaseLock() throw e }