Skip to content

chore: disable auto proxy selection based on latency #8137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 26, 2023
Merged

chore: disable auto proxy selection based on latency #8137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f4f6291
chore: disable auto pick proxy based on latency
Emyrk Jun 21, 2023
efb3bf6
Remove latency pulled from storage
Emyrk Jun 21, 2023
4a2b55e
Fix js tesT
Emyrk Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions site/src/contexts/ProxyContext.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
67 changes: 43 additions & 24 deletions site/src/contexts/ProxyContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ export const ProxyProvider: FC<PropsWithChildren> = ({ children }) => {
proxiesResp?.regions ?? [],
loadUserSelectedProxy(),
proxyLatencies,
// Do not auto select based on latencies, as inconsistent latencies can cause this
// to behave poorly.
false,
),
)
}, [proxiesResp, proxyLatencies])
Expand Down Expand Up @@ -208,6 +211,7 @@ export const getPreferredProxy = (
proxies: Region[],
selectedProxy?: Region,
latencies?: Record<string, ProxyLatencyReport>,
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.
Expand All @@ -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<string, Region>)

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<string, ProxyLatencyReport>,
): Region | undefined => {
if (!latencies) {
return undefined
}

const proxyMap = proxies.reduce((acc, proxy) => {
acc[proxy.id] = proxy
return acc
}, {} as Record<string, Region>)

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.
Expand Down
30 changes: 3 additions & 27 deletions site/src/contexts/useProxyLatency.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,32 +24,8 @@ const proxyLatenciesReducer = (
state: Record<string, ProxyLatencyReport>,
action: ProxyLatencyAction,
): Record<string, ProxyLatencyReport> => {
// 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,
Expand All @@ -65,7 +41,7 @@ export const useProxyLatency = (
proxyLatencies: Record<string, ProxyLatencyReport>
} => {
// 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.
Expand Down