From 871c7d6c3a329fdebe39104fac8b535f87dc3129 Mon Sep 17 00:00:00 2001 From: Marcus Pasell <3690498+rickyrombo@users.noreply.github.com> Date: Tue, 7 Feb 2023 19:35:29 -0800 Subject: [PATCH 1/3] SDK: Allow updating DN selector config after initializing --- .../DiscoveryNodeSelector.test.ts | 16 ++++++++++ .../DiscoveryNodeSelector.ts | 29 ++++++++++++++++++- .../services/DiscoveryNodeSelector/types.ts | 6 ---- libs/src/sdk/utils/deepPartial.ts | 2 ++ 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.test.ts b/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.test.ts index dd71ef8889a..04d25336df6 100644 --- a/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.test.ts +++ b/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.test.ts @@ -234,6 +234,14 @@ describe('discoveryNodeSelector', () => { const selected = await selector.getSelectedEndpoint() expect(selected).toBe(BEHIND_BLOCKDIFF_NODE) expect(selector.isBehind).toBe(true) + + // Update config and trigger reselection + selector.updateConfig({ + allowlist: new Set([HEALTHY_NODE]) + }) + const selected2 = await selector.getSelectedEndpoint() + expect(selected2).toBe(HEALTHY_NODE) + expect(selector.isBehind).toBe(false) }) test('respects blocklist', async () => { @@ -248,6 +256,14 @@ describe('discoveryNodeSelector', () => { const selected = await selector.getSelectedEndpoint() expect(selected).toBe(BEHIND_BLOCKDIFF_NODE) expect(selector.isBehind).toBe(true) + + // Update config and trigger reselection + selector.updateConfig({ + blocklist: new Set([BEHIND_BLOCKDIFF_NODE]) + }) + const selected2 = await selector.getSelectedEndpoint() + expect(selected2).toBe(HEALTHY_NODE) + expect(selector.isBehind).toBe(false) }) test('uses configured default', async () => { diff --git a/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts b/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts index ebf8d932412..b29a356be8e 100644 --- a/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts +++ b/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts @@ -41,7 +41,7 @@ export class DiscoveryNodeSelector implements DiscoveryNodeSelectorService { /** * Configuration passed in by consumer (with defaults) */ - private readonly config: DiscoveryNodeSelectorServiceConfigInternal + private config: DiscoveryNodeSelectorServiceConfigInternal /** * Whether or not we are using a backup, meaning we were @@ -113,6 +113,8 @@ export class DiscoveryNodeSelector implements DiscoveryNodeSelectorService { this.backupServices = {} this.selectedNode = this.config.initialSelectedNode && + (!this.config.allowlist || + this.config.allowlist?.has(this.config.initialSelectedNode)) && !this.config.blocklist?.has(this.config.initialSelectedNode) ? this.config.initialSelectedNode : null @@ -129,6 +131,30 @@ export class DiscoveryNodeSelector implements DiscoveryNodeSelectorService { ) } + /** + * Updates the config. + * Note that setting the initial node or bootstrap nodes here does nothing as the service is already initialized. + * Will force reselections if health check thresholds change (as that might cause the current node to be considered unhealthy) + * or if the selected node is excluded per allow/blocklists + */ + public updateConfig( + config: Exclude< + DiscoveryNodeSelectorServiceConfig, + 'initialSelectedNode' | 'bootstrapServices' + > + ) { + this.config = mergeConfigWithDefaults(config, this.config) + if (this.selectedNode) { + if (config.healthCheckThresholds) { + this.selectedNode = null + } else if (!config.allowlist?.has(this.selectedNode)) { + this.selectedNode = null + } else if (config.blocklist?.has(this.selectedNode)) { + this.selectedNode = null + } + } + } + /** * Returns a middleware that reselects if the current discovery node is behind * @returns the middleware @@ -328,6 +354,7 @@ export class DiscoveryNodeSelector implements DiscoveryNodeSelectorService { this.info(`Selected discprov ${selectedService}`, decisionTree, { attemptedServicesCount }) + this.isBehind = false return selectedService } diff --git a/libs/src/sdk/services/DiscoveryNodeSelector/types.ts b/libs/src/sdk/services/DiscoveryNodeSelector/types.ts index b3b39850fd4..081ac24f197 100644 --- a/libs/src/sdk/services/DiscoveryNodeSelector/types.ts +++ b/libs/src/sdk/services/DiscoveryNodeSelector/types.ts @@ -64,12 +64,6 @@ export type DiscoveryNodeSelectorServiceConfigInternal = { * tried again (re-requested) */ backupsTTL: number - /** - * How long the cache should live for selected nodes, in ms. - * If unset, never expires. - * @default 600000 ten minutes - */ - cacheTTL: number | null /** * Configuration for determining healthy status */ diff --git a/libs/src/sdk/utils/deepPartial.ts b/libs/src/sdk/utils/deepPartial.ts index c5484e10096..9244bbca08e 100644 --- a/libs/src/sdk/utils/deepPartial.ts +++ b/libs/src/sdk/utils/deepPartial.ts @@ -1,5 +1,7 @@ // Adjusted from Terry: https://stackoverflow.com/questions/61132262/typescript-deep-partial export type DeepPartial = T extends any[] + ? T + : T extends Set ? T : T extends object ? { From 8599813193af33e398c7ff0e9c367dee52dee3e7 Mon Sep 17 00:00:00 2001 From: Marcus Pasell <3690498+rickyrombo@users.noreply.github.com> Date: Wed, 8 Feb 2023 10:09:12 -0800 Subject: [PATCH 2/3] Remove default for unused config --- libs/src/sdk/services/DiscoveryNodeSelector/constants.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/src/sdk/services/DiscoveryNodeSelector/constants.ts b/libs/src/sdk/services/DiscoveryNodeSelector/constants.ts index e7cf442c665..88b9c718f10 100644 --- a/libs/src/sdk/services/DiscoveryNodeSelector/constants.ts +++ b/libs/src/sdk/services/DiscoveryNodeSelector/constants.ts @@ -15,7 +15,6 @@ export const defaultDiscoveryNodeSelectorConfig: DiscoveryNodeSelectorServiceCon requestTimeout: 30000, // 30s unhealthyTTL: 3600000, // 1 hour backupsTTL: 120000, // 2 min - cacheTTL: 600000, // 10 min healthCheckThresholds: { minVersion: bootstrap.version, maxSlotDiffPlays: null, From f5dfcc35a87ed16e79e99e93fb6c6e3f029ec6df Mon Sep 17 00:00:00 2001 From: Marcus Pasell <3690498+rickyrombo@users.noreply.github.com> Date: Wed, 8 Feb 2023 10:10:00 -0800 Subject: [PATCH 3/3] ensure we have an allowlist before checking the current node is in it --- .../sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts b/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts index b29a356be8e..d189808eaca 100644 --- a/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts +++ b/libs/src/sdk/services/DiscoveryNodeSelector/DiscoveryNodeSelector.ts @@ -147,7 +147,7 @@ export class DiscoveryNodeSelector implements DiscoveryNodeSelectorService { if (this.selectedNode) { if (config.healthCheckThresholds) { this.selectedNode = null - } else if (!config.allowlist?.has(this.selectedNode)) { + } else if (config.allowlist && !config.allowlist.has(this.selectedNode)) { this.selectedNode = null } else if (config.blocklist?.has(this.selectedNode)) { this.selectedNode = null