From f4f629134a4fd42f808ed67c95e4a4ab68c997e5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 21 Jun 2023 13:37:20 -0500 Subject: [PATCH 1/3] chore: disable auto pick proxy based on latency --- site/src/contexts/ProxyContext.tsx | 67 +++++++++++++++++++----------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/site/src/contexts/ProxyContext.tsx b/site/src/contexts/ProxyContext.tsx index 89eec7bee96ef..ec5e14bf92242 100644 --- a/site/src/contexts/ProxyContext.tsx +++ b/site/src/contexts/ProxyContext.tsx @@ -136,6 +136,9 @@ export const ProxyProvider: FC = ({ children }) => { proxiesResp?.regions ?? [], loadUserSelectedProxy(), proxyLatencies, + // Do not auto select based on latencies, as inconsistent latencies can cause this + // to behave poorly. + false, ), ) }, [proxiesResp, proxyLatencies]) @@ -208,6 +211,7 @@ export const getPreferredProxy = ( proxies: Region[], selectedProxy?: Region, latencies?: Record, + autoSelectBasedOnLatency = true, ): PreferredProxy => { // If a proxy is selected, make sure it is in the list of proxies. If it is not // we should default to the primary. @@ -219,37 +223,52 @@ export const getPreferredProxy = ( if (!selectedProxy || !selectedProxy.healthy) { // By default, use the primary proxy. selectedProxy = proxies.find((proxy) => proxy.name === "primary") + // If we have latencies, then attempt to use the best proxy by latency instead. - if (latencies) { - const proxyMap = proxies.reduce((acc, proxy) => { - acc[proxy.id] = proxy - return acc - }, {} as Record) - - const best = Object.keys(latencies) - .map((proxyId) => { - return { - id: proxyId, - ...latencies[proxyId], - } - }) - // If the proxy is not in our list, or it is unhealthy, ignore it. - .filter((latency) => proxyMap[latency.id]?.healthy) - .sort((a, b) => a.latencyMS - b.latencyMS) - .at(0) - - // Found a new best, use it! - if (best) { - const bestProxy = proxies.find((proxy) => proxy.id === best.id) - // Default to w/e it was before - selectedProxy = bestProxy || selectedProxy - } + const best = selectByLatency(proxies, latencies) + if (autoSelectBasedOnLatency && best) { + selectedProxy = best } } return computeUsableURLS(selectedProxy) } +const selectByLatency = ( + proxies: Region[], + latencies?: Record, +): Region | undefined => { + if (!latencies) { + return undefined + } + + const proxyMap = proxies.reduce((acc, proxy) => { + acc[proxy.id] = proxy + return acc + }, {} as Record) + + const best = Object.keys(latencies) + .map((proxyId) => { + return { + id: proxyId, + ...latencies[proxyId], + } + }) + // If the proxy is not in our list, or it is unhealthy, ignore it. + .filter((latency) => proxyMap[latency.id]?.healthy) + .sort((a, b) => a.latencyMS - b.latencyMS) + .at(0) + + // Found a new best, use it! + if (best) { + const bestProxy = proxies.find((proxy) => proxy.id === best.id) + // Default to w/e it was before + return bestProxy + } + + return undefined +} + const computeUsableURLS = (proxy?: Region): PreferredProxy => { if (!proxy) { // By default use relative links for the primary proxy. From efb3bf60ee73c0fe2561409623288ea916f4e0fc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 21 Jun 2023 13:47:24 -0500 Subject: [PATCH 2/3] Remove latency pulled from storage --- site/src/contexts/useProxyLatency.ts | 30 +++------------------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/site/src/contexts/useProxyLatency.ts b/site/src/contexts/useProxyLatency.ts index 05f28e0f62bb7..ec28f5bf21af0 100644 --- a/site/src/contexts/useProxyLatency.ts +++ b/site/src/contexts/useProxyLatency.ts @@ -24,32 +24,8 @@ const proxyLatenciesReducer = ( state: Record, action: ProxyLatencyAction, ): Record => { - // TODO: We should probably not read from local storage on every action. - const history = loadStoredLatencies() - const proxyHistory = history[action.proxyID] || [] - const minReport = proxyHistory.reduce((min, report) => { - if (min.latencyMS === 0) { - // Not yet set, so use the new report. - return report - } - if (min.latencyMS < report.latencyMS) { - return min - } - return report - }, {} as ProxyLatencyReport) - - if ( - minReport.latencyMS > 0 && - minReport.latencyMS < action.report.latencyMS - ) { - // The new report is slower then the min report, so use the min report. - return { - ...state, - [action.proxyID]: minReport, - } - } - - // Use the new report + // Always return the new report. We have some saved latencies, but until we have a better + // way to utilize them, we will ignore them for all practical purposes. return { ...state, [action.proxyID]: action.report, @@ -65,7 +41,7 @@ export const useProxyLatency = ( proxyLatencies: Record } => { // maxStoredLatencies is the maximum number of latencies to store per proxy in local storage. - let maxStoredLatencies = 8 + let maxStoredLatencies = 1 // The reason we pull this from local storage is so for development purposes, a user can manually // set a larger number to collect data in their normal usage. This data can later be analyzed to come up // with some better magic numbers. From 4a2b55efc46483671990587611ce673ab8f8f8ab Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 21 Jun 2023 13:59:27 -0500 Subject: [PATCH 3/3] Fix js tesT --- site/src/contexts/ProxyContext.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/contexts/ProxyContext.test.tsx b/site/src/contexts/ProxyContext.test.tsx index 14b8418ca36cf..31dfaa8fc5bef 100644 --- a/site/src/contexts/ProxyContext.test.tsx +++ b/site/src/contexts/ProxyContext.test.tsx @@ -264,11 +264,11 @@ describe("ProxyContextSelection", () => { expUserProxyID: MockHealthyWildWorkspaceProxy.id, }, ], - // Latency behavior + // Latency behavior is disabled, so the primary should be selected. [ "regions_default_low_latency", { - expProxyID: MockHealthyWildWorkspaceProxy.id, + expProxyID: MockPrimaryWorkspaceProxy.id, regions: MockWorkspaceProxies, storageProxy: undefined, latencies: {