From 88b2519e3faa0afeded806b65a1bcb4028d1cce2 Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Mon, 27 Apr 2020 19:17:25 -0700 Subject: [PATCH 1/8] Add DiscoveryProviderSelection --- libs/package.json | 2 +- libs/src/api/serviceProvider.js | 6 +- .../src/service-selection/ServiceSelection.js | 14 +- .../ServiceSelection.test.js | 30 ++ .../DiscoveryProviderSelection.js | 187 +++++++++ .../DiscoveryProviderSelection.test.js | 359 ++++++++++++++++++ .../services/discoveryProvider/constants.js | 2 + libs/src/services/discoveryProvider/index.js | 52 +-- .../services/discoveryProvider/selection.js | 0 libs/src/services/ethContracts/index.js | 15 +- libs/src/utils/network.js | 11 +- libs/src/utils/promiseFight.test.js | 16 + 12 files changed, 638 insertions(+), 56 deletions(-) create mode 100644 libs/src/services/discoveryProvider/DiscoveryProviderSelection.js create mode 100644 libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js delete mode 100644 libs/src/services/discoveryProvider/selection.js diff --git a/libs/package.json b/libs/package.json index 2e45c6b3145..42ecb3c9c07 100644 --- a/libs/package.json +++ b/libs/package.json @@ -11,7 +11,7 @@ "scripts": { "test": "./scripts/test.sh", "test-circle-ci": "./scripts/circleci-test.sh", - "test:units": "mocha ./src/**/*.test.js --exit", + "test:units": "mocha './src/**/*.test.js' --exit", "setup": "./scripts/migrate_contracts.sh", "lint": "./node_modules/.bin/standard", "lint-fix": "./node_modules/.bin/standard --fix" diff --git a/libs/src/api/serviceProvider.js b/libs/src/api/serviceProvider.js index 1d1b08332c3..170ecabf768 100644 --- a/libs/src/api/serviceProvider.js +++ b/libs/src/api/serviceProvider.js @@ -1,6 +1,6 @@ const _ = require('lodash') const { Base } = require('./base') -const Utils = require('../utils') +const { timeRequests } = require('../utils/network') const CREATOR_NODE_SERVICE_NAME = 'creator-node' const DISCOVERY_PROVIDER_SERVICE_NAME = 'discovery-provider' @@ -40,7 +40,7 @@ class ServiceProvider extends Base { } // Time requests and get version info - const timings = await Utils.timeRequests( + const timings = await timeRequests( creatorNodes.map(node => ({ id: node.endpoint, url: `${node.endpoint}/version` @@ -96,7 +96,7 @@ class ServiceProvider extends Base { .filter(Boolean) // Time requests and autoselect nodes - const timings = await Utils.timeRequests( + const timings = await timeRequests( creatorNodes.map(node => ({ id: node, url: `${node}/version` diff --git a/libs/src/service-selection/ServiceSelection.js b/libs/src/service-selection/ServiceSelection.js index 5ec65ee572c..39a7b3421d1 100644 --- a/libs/src/service-selection/ServiceSelection.js +++ b/libs/src/service-selection/ServiceSelection.js @@ -128,8 +128,16 @@ class ServiceSelection { clearTimeout(this.unhealthyCleanupTimeout) clearTimeout(this.backupCleanupTimeout) - this.unhealthyCleanupTimeout = setTimeout(() => { this.unhealthy = [] }, this.unhealthyTTL) - this.backupCleanupTimeout = setTimeout(() => { this.backups = {} }, this.backupsTTL) + this.unhealthyCleanupTimeout = setTimeout(() => { this.clearUnhealthy() }, this.unhealthyTTL) + this.backupCleanupTimeout = setTimeout(() => { this.clearBackups() }, this.backupsTTL) + } + + clearUnhealthy () { + this.unhealthy = new Set([]) + } + + clearBackups () { + this.backups = {} } /** A short-circuit. If overriden, can be used to skip selection (which could be slow) */ @@ -215,7 +223,7 @@ class ServiceSelection { * Controls how a backup is picked. Overriding methods may choose to use the backup's response. * e.g. pick a backup that's the fewest versions behind */ - selectFromBackups () { + async selectFromBackups () { return Object.keys(this.backups)[0] } } diff --git a/libs/src/service-selection/ServiceSelection.test.js b/libs/src/service-selection/ServiceSelection.test.js index 39ba11fbe4a..ff6caaa437c 100644 --- a/libs/src/service-selection/ServiceSelection.test.js +++ b/libs/src/service-selection/ServiceSelection.test.js @@ -1,6 +1,7 @@ const ServiceSelection = require('./ServiceSelection') const nock = require('nock') const assert = require('assert') +const Utils = require('../utils') describe('ServiceSelection', () => { it('prefers a healthy service', async () => { @@ -121,6 +122,35 @@ describe('ServiceSelection', () => { const service = await s.select() assert.strictEqual(service, slow) }) + + it('will recheck unhealthy ones', async () => { + const atFirstHealthy = 'https://atFirstHealthy.audius.co' + nock(atFirstHealthy) + .get('/health_check') + .reply(200) + nock(atFirstHealthy) + .get('/health_check') + .reply(400) + + const atFirstUnhealthy = 'https://atFirstUnhealthy.audius.co' + nock(atFirstUnhealthy) + .get('/health_check') + .reply(400) + nock(atFirstUnhealthy) + .get('/health_check') + .reply(200) + + const s = new ServiceSelection({ + getServices: () => [atFirstHealthy, atFirstUnhealthy], + unhealthyTTL: 0 + }) + const firstService = await s.select() + assert.strictEqual(firstService, atFirstHealthy) + + await Utils.wait(200) + const secondService = await s.select() + assert.strictEqual(secondService, atFirstUnhealthy) + }) }) describe('ServiceSelection withBackupCriteria', () => { diff --git a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js new file mode 100644 index 00000000000..aa87e3dd77b --- /dev/null +++ b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js @@ -0,0 +1,187 @@ +const ServiceSelection = require('../../service-selection/ServiceSelection') +const { + DISCOVERY_PROVIDER_TIMESTAMP, + DISCOVERY_SERVICE_NAME, + UNHEALTHY_BLOCK_DIFF, + DISCOVERY_PROVIDER_RESELECT_TIMEOUT, + REGRESSED_MODE_TIMEOUT +} = require('./constants') +const semver = require('semver') + +let localStorage +if (typeof window === 'undefined' || window === null) { + const LocalStorage = require('node-localstorage').LocalStorage + localStorage = new LocalStorage('./local-storage') +} else { + localStorage = window.localStorage +} + +class DiscoveryProviderSelection extends ServiceSelection { + constructor (config, ethContracts) { + super({ + /** + * Gets the "current" expected service version as well as + * the list of registered providers from chain + */ + getServices: async () => { + this.currentVersion = await ethContracts.getCurrentVersion(DISCOVERY_SERVICE_NAME) + return this.ethContracts.getServiceProviderList(DISCOVERY_SERVICE_NAME) + }, + ...config + }) + this.ethContracts = ethContracts + this.currentVersion = null + + // Whether or not we are running in `regressed` mode, meaning we were + // unable to select a discovery provider that was up-to-date. Clients may + // want to consider blocking writes. + this._regressedMode = false + } + + /** Retrieves a cached discovery provider from localstorage */ + getCached () { + if (localStorage) { + const discProvTimestamp = localStorage.getItem(DISCOVERY_PROVIDER_TIMESTAMP) + if (discProvTimestamp) { + const { endpoint: latestEndpoint, timestamp } = JSON.parse(discProvTimestamp) + const inWhitelist = !this.whitelist || this.whitelist.has(latestEndpoint) + const isExpired = (Date.now() - timestamp) > DISCOVERY_PROVIDER_RESELECT_TIMEOUT + if (!inWhitelist || isExpired) { + this.clearCached() + } else { + return latestEndpoint + } + } + } + return null + } + + /** Clears any cached discovery provider from localstorage */ + clearCached () { + if (localStorage) { + localStorage.removeItem(DISCOVERY_PROVIDER_TIMESTAMP) + } + } + + /** Sets a cached discovery provider in localstorage */ + setCached (endpoint) { + localStorage.setItem(DISCOVERY_PROVIDER_TIMESTAMP, JSON.stringify({ endpoint, timestamp: Date.now() })) + } + + /** Allows the selection take a shortcut if there's a cached provider */ + shortcircuit () { + return this.getCached() + } + + async select () { + const endpoint = await super.select() + this.setCached(endpoint) + return endpoint + } + + /** + * Checks whether a given response is healthy: + * - Not behind in blocks + * - 200 response + * - Current version + * + * Other responses are collected in `this.backups` if + * - Behind by only a patch version + * + * @param {Response} response axios response + * @param {{ [key: string]: string}} urlMap health check urls mapped to their cannonical url + * e.g. https://discoveryprovider.audius.co/health_check => https://discoveryprovider.audius.co + */ + isHealthy (response, urlMap) { + const { status, data } = response + const { block_difference: blockDiff, service, version } = data + if (status !== 200) return false + if (service !== DISCOVERY_SERVICE_NAME) return false + if (!semver.valid(version)) return false + if (!this.ethContracts.isValidSPVersion(version, this.currentVersion)) return false + + if ( + blockDiff > UNHEALTHY_BLOCK_DIFF || + version !== this.currentVersion + ) { + this.addBackup(urlMap[response.config.url], response.data) + return false + } + + return true + } + + /** + * Estabilishes that connection to discovery providers has regressed + */ + enterRegressedMode () { + console.info('Entering regressed mode') + this._regressedMode = true + setTimeout(() => { + console.info('Leaving regressed mode') + this._regressedMode = false + }, REGRESSED_MODE_TIMEOUT) + } + + isInRegressedMode () { + return this._regressedMode + } + + /** + * In the case of no "healthy" services, we resort to backups. + * 1. Pick the most recent version that's not behind + * 2. Pick the least behind provider and enter "regressed mode" + */ + selectFromBackups () { + const versions = [] + const blockDiffs = [] + + const versionMap = {} + const blockDiffMap = {} + + // Go through each backup and record version and block diff maps + Object.keys(this.backups).forEach(backup => { + const { block_difference: blockDiff, version } = this.backups[backup] + versions.push(version) + blockDiffs.push(blockDiff) + + if (version in versionMap) { + versionMap[version].push(backup) + } else { + versionMap[version] = [backup] + } + + if (blockDiff in blockDiffMap) { + blockDiffMap[blockDiff].push(backup) + } else { + blockDiffMap[blockDiff] = [backup] + } + }) + + // Sort the versions by desc semver + const sortedVersions = versions.sort(semver.rcompare) + + // Select the closest version that's a healthy # of blocks behind + let selected = null + for (const version of sortedVersions) { + const endpoints = versionMap[version] + for (let i = 0; i < endpoints.length; ++i) { + if (this.backups[endpoints[i]].block_difference < UNHEALTHY_BLOCK_DIFF) { + selected = endpoints[i] + break + } + } + if (selected) return selected + } + + // Select the best block diff provider + const bestBlockDiff = blockDiffs.sort().reverse()[0] + + selected = blockDiffMap[bestBlockDiff][0] + this.enterRegressedMode() + + return selected + } +} + +module.exports = DiscoveryProviderSelection diff --git a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js new file mode 100644 index 00000000000..84c84cf0f2b --- /dev/null +++ b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js @@ -0,0 +1,359 @@ +const nock = require('nock') +const assert = require('assert') +const semver = require('semver') + +const DiscoveryProviderSelection = require('./DiscoveryProviderSelection') + +const mockEthContracts = (urls, currrentVersion) => ({ + getCurrentVersion: async () => currrentVersion, + getServiceProviderList: async () => urls, + isValidSPVersion: (version1, version2) => { + return ( + semver.major(version1) === semver.major(version2) && + semver.minor(version1) === semver.minor(version2) && + semver.patch(version2) >= semver.patch(version1) + ) + } +}) + +describe('DiscoveryProviderSelection', () => { + beforeEach(() => { + const LocalStorage = require('node-localstorage').LocalStorage + const localStorage = new LocalStorage('./local-storage') + localStorage.removeItem('@audius/libs:discovery-provider-timestamp') + }) + + it('selects a healthy service', async () => { + const healthy = 'https://healthy.audius.co' + nock(healthy) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const s = new DiscoveryProviderSelection( + {}, + mockEthContracts([healthy], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, healthy) + }) + + it('selects a healthy service with an unhealthy one present', async () => { + const healthy = 'https://healthy.audius.co' + nock(healthy) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + const unhealthy = 'https://unhealthy.audius.co' + nock(unhealthy) + .get('/health_check') + .reply(400, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const s = new DiscoveryProviderSelection( + {}, + mockEthContracts([healthy, unhealthy], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, healthy) + }) + + it('prefers the correct vesion', async () => { + const healthy = 'https://healthy.audius.co' + nock(healthy) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + const outdated = 'https://outdated.audius.co' + nock(outdated) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.2', + block_difference: 0 + }) + + const s = new DiscoveryProviderSelection( + {}, + mockEthContracts([healthy, outdated], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, healthy) + }) + + it('prefers a healthy block diff', async () => { + const healthy = 'https://healthy.audius.co' + nock(healthy) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + const behind = 'https://behind.audius.co' + nock(behind) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 20 + }) + + const s = new DiscoveryProviderSelection( + {}, + mockEthContracts([healthy, behind], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, healthy) + }) + + it('can select an old version', async () => { + const healthyButBehind = 'https://healthyButBehind.audius.co' + nock(healthyButBehind) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 20 + }) + const pastVersionNotBehind = 'https://pastVersionNotBehind.audius.co' + nock(pastVersionNotBehind) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.2', + block_difference: 0 + }) + + const s = new DiscoveryProviderSelection( + { requestTimeout: 100 }, + mockEthContracts([healthyButBehind, pastVersionNotBehind], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, pastVersionNotBehind) + assert.deepStrictEqual(s.backups, { + [healthyButBehind]: { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 20 + }, + [pastVersionNotBehind]: { + service: 'discovery-provider', + version: '1.2.2', + block_difference: 0 + } + }) + assert.strictEqual(s.getTotalAttempts(), 2) + }) + + it('can select the discprov that is the least number of blocks behind', async () => { + const behind20 = 'https://behind20.audius.co' + nock(behind20) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.2', + block_difference: 20 + }) + const behind40 = 'https://behind40.audius.co' + nock(behind40) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 40 + }) + + const s = new DiscoveryProviderSelection( + { requestTimeout: 100 }, + mockEthContracts([behind20, behind40], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, behind40) + assert.deepStrictEqual(s.backups, { + [behind20]: { + service: 'discovery-provider', + version: '1.2.2', + block_difference: 20 + }, + [behind40]: { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 40 + } + }) + assert.strictEqual(s.getTotalAttempts(), 2) + assert.strictEqual(s.isInRegressedMode(), true) + }) + + it('will not pick a minor version behind provider', async () => { + const minorBehind = 'https://minorBehind.audius.co' + nock(minorBehind) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.1.3', + block_difference: 20 + }) + const s = new DiscoveryProviderSelection( + { requestTimeout: 100 }, + mockEthContracts([minorBehind], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, null) + }) + + it('respects a whitelist', async () => { + const healthy1 = 'https://healthy1.audius.co' + nock(healthy1) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const healthy2 = 'https://healthy2.audius.co' + nock(healthy2) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const s = new DiscoveryProviderSelection( + { + whitelist: new Set([healthy2]) + }, + mockEthContracts([healthy1, healthy2], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, healthy2) + }) + + it('will cache its choice', async () => { + const LocalStorage = require('node-localstorage').LocalStorage + const localStorage = new LocalStorage('./local-storage') + + const healthy1 = 'https://healthy1.audius.co' + nock(healthy1) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const s = new DiscoveryProviderSelection( + {}, + mockEthContracts([healthy1], '1.2.3') + ) + const service = await s.select() + assert.strictEqual(service, healthy1) + const { endpoint } = JSON.parse(localStorage.getItem('@audius/libs:discovery-provider-timestamp')) + assert.strictEqual( + endpoint, + healthy1 + ) + }) + + it('will cache its choice and reuse it', async () => { + const LocalStorage = require('node-localstorage').LocalStorage + const localStorage = new LocalStorage('./local-storage') + + const healthy1 = 'https://healthy1.audius.co' + nock(healthy1) + .get('/health_check') + .delay(100) + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const healthy2 = 'https://healthy2.audius.co' + nock(healthy2) + .get('/health_check') + .delay(100) + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const initiallyUnhealthy = 'https://initiallyUnhealthy.audius.co' + nock(initiallyUnhealthy) + .get('/health_check') + .reply(400, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const s = new DiscoveryProviderSelection( + {}, + mockEthContracts([healthy1, healthy2, initiallyUnhealthy], '1.2.3') + ) + const firstService = await s.select() + const { endpoint } = JSON.parse(localStorage.getItem('@audius/libs:discovery-provider-timestamp')) + assert.strictEqual( + endpoint, + firstService + ) + + const secondService = await s.select() + assert.strictEqual(firstService, secondService) + + const thirdService = await s.select() + assert.strictEqual(firstService, thirdService) + + const fourthService = await s.select() + assert.strictEqual(firstService, fourthService) + + // Clear the cached service + s.clearUnhealthy() + localStorage.removeItem('@audius/libs:discovery-provider-timestamp') + + // Make healthy1 start failing but healthy2 succeed + nock(healthy1) + .get('/health_check') + .reply(400, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + nock(healthy2) + .get('/health_check') + .reply(400, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + nock(initiallyUnhealthy) + .get('/health_check') + .reply(200, { + service: 'discovery-provider', + version: '1.2.3', + block_difference: 0 + }) + + const fifthService = await s.select() + assert.strictEqual(fifthService, initiallyUnhealthy) + + const sixthService = await s.select() + assert.strictEqual(sixthService, initiallyUnhealthy) + }) +}) diff --git a/libs/src/services/discoveryProvider/constants.js b/libs/src/services/discoveryProvider/constants.js index 6c5aba0c35d..2a4a6c7942f 100644 --- a/libs/src/services/discoveryProvider/constants.js +++ b/libs/src/services/discoveryProvider/constants.js @@ -1,5 +1,7 @@ module.exports.DISCOVERY_PROVIDER_TIMESTAMP = '@audius/libs:discovery-provider-timestamp' +module.exports.DISCOVERY_SERVICE_NAME = 'discovery-provider' module.exports.UNHEALTHY_BLOCK_DIFF = 15 +module.exports.REGRESSED_MODE_TIMEOUT = 2 * 60 * 1000 // two minutes // When to time out the cached discovery provider module.exports.DISCOVERY_PROVIDER_RESELECT_TIMEOUT = 1 /* min */ * 60 /* seconds */ * 1000 /* millisec */ diff --git a/libs/src/services/discoveryProvider/index.js b/libs/src/services/discoveryProvider/index.js index 7dfc2bd7fad..16d639a8185 100644 --- a/libs/src/services/discoveryProvider/index.js +++ b/libs/src/services/discoveryProvider/index.js @@ -3,6 +3,7 @@ const axios = require('axios') const Utils = require('../../utils') const { raceRequests } = require('../../utils/network') const { serviceType } = require('../ethContracts/index') +const DiscoveryProviderSelection = require('./DiscoveryProviderSelection') const { UNHEALTHY_BLOCK_DIFF, @@ -24,47 +25,15 @@ class DiscoveryProvider { this.userStateManager = userStateManager this.ethContracts = ethContracts this.web3Manager = web3Manager + + this.serviceSelector = new DiscoveryProviderSelection( + { whitelist }, + ethContracts + ) } async init () { - let endpoint - let pick - let isValid = null - - if (this.autoselect) { - endpoint = await this.autoSelectEndpoint() - } else { - if (typeof this.whitelist === 'string') { - endpoint = this.whitelist - } else { - if (!this.whitelist || this.whitelist.size === 0) { - throw new Error('Must pass autoselect true or provide whitelist.') - } - - // use this as a lookup between version endpoint and base url - const whitelistMap = {} - this.whitelist.forEach((url) => { - whitelistMap[urlJoin(url, '/version')] = url - }) - - try { - const { response } = await raceRequests(Object.keys(whitelistMap), (url) => { - pick = whitelistMap[url] - }, {}, REQUEST_TIMEOUT_MS) - - isValid = pick && response.data.service && (response.data.service === serviceType.DISCOVERY_PROVIDER) - if (isValid) { - console.info('Initial discovery provider was valid') - endpoint = pick - } else { - console.info('Initial discovery provider was invalid, searching for a new one') - endpoint = await this.ethContracts.selectDiscoveryProvider(this.whitelist) - } - } catch (e) { - throw new Error('Could not select a discprov from the whitelist', e) - } - } - } + const endpoint = await this.serviceSelector.select() this.setEndpoint(endpoint) if (endpoint && this.web3Manager && this.web3Manager.web3) { @@ -681,7 +650,8 @@ class DiscoveryProvider { } if (!this.discoveryProviderEndpoint) { - await this.autoSelectEndpoint() + const endpoint = await this.serviceSelector.select() + this.setEndpoint(endpoint) } if (retries === 0) { @@ -722,8 +692,8 @@ class DiscoveryProvider { const parsedResponse = Utils.parseDataFromResponse(response) if ( - this.ethContracts && - !this.ethContracts.isInRegressedMode() && + this.serviceSelector && + !this.serviceSelector.isInRegressedMode() && 'latest_indexed_block' in parsedResponse && 'latest_chain_block' in parsedResponse ) { diff --git a/libs/src/services/discoveryProvider/selection.js b/libs/src/services/discoveryProvider/selection.js deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/libs/src/services/ethContracts/index.js b/libs/src/services/ethContracts/index.js index 171b0b20242..510218449a7 100644 --- a/libs/src/services/ethContracts/index.js +++ b/libs/src/services/ethContracts/index.js @@ -176,6 +176,10 @@ class EthContracts { ) } + async getServiceProviderList (spType) { + return this.ServiceProviderFactoryClient.getServiceProviderList(spType) + } + /** * Returns a valid service provider url with the fastest response * @param {string} spType service provider type: 'discovery-provider' | 'content-service' | 'creator-node' @@ -389,15 +393,12 @@ class EthContracts { return endpoint } - async selectPriorVersionServiceProvider (spType, whitelist = null) { + async selectPriorVersionServiceProvider (spType) { if (!this.expectedServiceVersions) { this.expectedServiceVersions = await this.getExpectedServiceVersions() } let serviceProviders = await this.ServiceProviderFactoryClient.getServiceProviderList(spType) - if (whitelist) { - serviceProviders = serviceProviders.filter(d => whitelist.has(d.endpoint)) - } let numberOfServiceVersions = await this.VersioningFactoryClient.getNumberOfVersions(spType) @@ -435,9 +436,9 @@ class EthContracts { continue } - if (!semver.valid(serviceVersion)) { - throw new Error(`Invalid semver version found - ${serviceVersion}`) - } + // if (!semver.valid(serviceVersion)) { + // throw new Error(`Invalid semver version found - ${serviceVersion}`) + // } // Discovery provider specific validation if (spType === 'discovery-provider') { diff --git a/libs/src/utils/network.js b/libs/src/utils/network.js index 58c904ea514..1241273074e 100644 --- a/libs/src/utils/network.js +++ b/libs/src/utils/network.js @@ -87,7 +87,16 @@ async function raceRequests ( }) }) requests.push(Utils.wait(timeout)) - const { val: response, errored } = await promiseFight(requests, /* captureErrorred */ true) + let response + let errored + try { + const { val, errored: e } = await promiseFight(requests, /* captureErrorred */ true) + response = val + errored = e + } catch (e) { + response = null + errored = e + } sources.forEach(source => { source.cancel('Fetch already succeeded') }) diff --git a/libs/src/utils/promiseFight.test.js b/libs/src/utils/promiseFight.test.js index 55c99756f79..92b9edbee88 100644 --- a/libs/src/utils/promiseFight.test.js +++ b/libs/src/utils/promiseFight.test.js @@ -68,4 +68,20 @@ describe('promiseFight', () => { ], true) assert.deepStrictEqual(res, { val: 'first', errored: ['second', 'third'] }) }) + + it('should fail if all of the promises fail', async () => { + try { + await promiseFight([ + p('first', null, 100), + p('second', null, 10), + p('third', null, 20), + p('fourth', null, 200) + ], true) + } catch (e) { + assert.deepStrictEqual( + e, + ['first', 'second', 'third', 'fourth'] + ) + } + }) }) From c71f86a44683ceb718920fe5fb9cbaafcb2fb1c4 Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Mon, 27 Apr 2020 19:37:40 -0700 Subject: [PATCH 2/8] Rebase and cleanup --- libs/src/services/discoveryProvider/index.js | 32 +++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/libs/src/services/discoveryProvider/index.js b/libs/src/services/discoveryProvider/index.js index 16d639a8185..821163a8561 100644 --- a/libs/src/services/discoveryProvider/index.js +++ b/libs/src/services/discoveryProvider/index.js @@ -1,8 +1,6 @@ const axios = require('axios') const Utils = require('../../utils') -const { raceRequests } = require('../../utils/network') -const { serviceType } = require('../ethContracts/index') const DiscoveryProviderSelection = require('./DiscoveryProviderSelection') const { @@ -16,7 +14,6 @@ if (urlJoin && urlJoin.default) urlJoin = urlJoin.default const MAKE_REQUEST_RETRY_COUNT = 3 const MAX_MAKE_REQUEST_RETRY_COUNT = 50 -const AUTOSELECT_DISCOVERY_PROVIDER_RETRY_COUNT = 3 class DiscoveryProvider { constructor (autoselect, whitelist, userStateManager, ethContracts, web3Manager) { @@ -47,25 +44,6 @@ class DiscoveryProvider { this.discoveryProviderEndpoint = endpoint } - /** - * Wrapper method to auto select a valid discovery provider. - * @param {*} retries max retries before throwing an error - * @param {*} clearCachedDiscoveryProvider if set to true, implies that the previously - * selected discovery provider has been failing to serve requests. The prior recurring interval of - * checking local storage for DP and old DP local storage entry need to be cleared. - */ - async autoSelectEndpoint (retries = 3, clearCachedDiscoveryProvider = false) { - if (retries > 0) { - const endpoint = await this.ethContracts.autoselectDiscoveryProvider(this.whitelist, clearCachedDiscoveryProvider) - if (endpoint) { - this.setEndpoint(endpoint) - return endpoint - } - return this.autoSelectEndpoint(retries - 1) - } - throw new Error('Failed to autoselect discovery provider') - } - /** * get users with all relevant user data * can be filtered by providing an integer array of ids @@ -658,7 +636,8 @@ class DiscoveryProvider { // Reset the retries count in the case that the newly selected disc prov fails, we can // allow it to try MAKE_REQUEST_RETRIES_COUNT number of times before trying another retries = MAKE_REQUEST_RETRY_COUNT - await this.autoSelectEndpoint(AUTOSELECT_DISCOVERY_PROVIDER_RETRY_COUNT, true) + const endpoint = await this.serviceSelector.select() + this.setEndpoint(endpoint) } let requestUrl @@ -707,10 +686,13 @@ class DiscoveryProvider { !indexedBlock || (chainBlock - indexedBlock) > UNHEALTHY_BLOCK_DIFF ) { - // Select a new one console.info(`${this.discoveryProviderEndpoint} is too far behind, reselecting discovery provider`) - const endpoint = await this.autoSelectEndpoint(AUTOSELECT_DISCOVERY_PROVIDER_RETRY_COUNT, true) + // Mark the current selection as a backup + this.serviceSelector.addBackup(this.discoveryProviderEndpoint, response) + // Select a new one + const endpoint = await this.serviceSelector.select() this.setEndpoint(endpoint) + retries = MAKE_REQUEST_RETRY_COUNT // reset retry count when setting a new endpoint throw new Error(`Selected endpoint was too far behind. Indexed: ${indexedBlock} Chain: ${chainBlock}`) } From 57fc7f560c9871ecc93bf36509d337146945bd7f Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Mon, 27 Apr 2020 19:42:24 -0700 Subject: [PATCH 3/8] Undo comment --- libs/src/services/ethContracts/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/src/services/ethContracts/index.js b/libs/src/services/ethContracts/index.js index 510218449a7..cfb95bc031f 100644 --- a/libs/src/services/ethContracts/index.js +++ b/libs/src/services/ethContracts/index.js @@ -436,9 +436,9 @@ class EthContracts { continue } - // if (!semver.valid(serviceVersion)) { - // throw new Error(`Invalid semver version found - ${serviceVersion}`) - // } + if (!semver.valid(serviceVersion)) { + throw new Error(`Invalid semver version found - ${serviceVersion}`) + } // Discovery provider specific validation if (spType === 'discovery-provider') { From b9e5a5016dc10ec2bba54ba995c6ed9f9820b7b2 Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Wed, 29 Apr 2020 09:49:19 -0700 Subject: [PATCH 4/8] Reverse check valid dp versions --- .../DiscoveryProviderSelection.js | 28 ++++++++++++++++++- .../DiscoveryProviderSelection.test.js | 4 +++ libs/src/services/ethContracts/index.js | 8 ++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js index aa87e3dd77b..217aca510dd 100644 --- a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js +++ b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js @@ -8,6 +8,8 @@ const { } = require('./constants') const semver = require('semver') +const PREVIOUS_VERSIONS_TO_CHECK = 5 + let localStorage if (typeof window === 'undefined' || window === null) { const LocalStorage = require('node-localstorage').LocalStorage @@ -36,6 +38,9 @@ class DiscoveryProviderSelection extends ServiceSelection { // unable to select a discovery provider that was up-to-date. Clients may // want to consider blocking writes. this._regressedMode = false + + // Set of valid past discovery provider versions registered on chain + this.validVersions = null } /** Retrieves a cached discovery provider from localstorage */ @@ -132,16 +137,37 @@ class DiscoveryProviderSelection extends ServiceSelection { * 1. Pick the most recent version that's not behind * 2. Pick the least behind provider and enter "regressed mode" */ - selectFromBackups () { + async selectFromBackups () { const versions = [] const blockDiffs = [] const versionMap = {} const blockDiffMap = {} + // Go backwards in time on chain and get the registered versions up to PREVIOUS_VERSIONS_TO_CHECK. + // Record those versions in a set and validate any backups against that set. + // TODO: Clean up this logic when we can validate a specific version rather + // than traversing backwards through all the versions + if (!this.validVersions) { + this.validVersions = new Set([this.currentVersion]) + const numberOfVersions = await this.ethContracts.getNumberOfVersions(DISCOVERY_SERVICE_NAME) + for (let i = 0; i < Math.min(PREVIOUS_VERSIONS_TO_CHECK, numberOfVersions - 1); ++i) { + const pastServiceVersion = await this.ethContracts.getVersion( + DISCOVERY_SERVICE_NAME, + // Exclude the latest version when querying older versions + // Latest index is numberOfVersions - 1, so 2nd oldest version starts at numberOfVersions - 2 + numberOfVersions - 2 - i + ) + this.validVersions.add(pastServiceVersion) + } + } + // Go through each backup and record version and block diff maps Object.keys(this.backups).forEach(backup => { const { block_difference: blockDiff, version } = this.backups[backup] + // Filter out any version that wasn't registered on chain + if (!this.validVersions.has(version)) return + versions.push(version) blockDiffs.push(blockDiff) diff --git a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js index 84c84cf0f2b..f4b04628885 100644 --- a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js +++ b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js @@ -6,6 +6,10 @@ const DiscoveryProviderSelection = require('./DiscoveryProviderSelection') const mockEthContracts = (urls, currrentVersion) => ({ getCurrentVersion: async () => currrentVersion, + getNumberOfVersions: async (spType) => 2, + getVersion: async (spType, queryIndex) => { + return ['1.2.2', '1.2.3'][queryIndex] + }, getServiceProviderList: async () => urls, isValidSPVersion: (version1, version2) => { return ( diff --git a/libs/src/services/ethContracts/index.js b/libs/src/services/ethContracts/index.js index cfb95bc031f..ad12638c8c9 100644 --- a/libs/src/services/ethContracts/index.js +++ b/libs/src/services/ethContracts/index.js @@ -180,6 +180,14 @@ class EthContracts { return this.ServiceProviderFactoryClient.getServiceProviderList(spType) } + async getNumberOfVersions (spType) { + return this.VersioningFactoryClient.getNumberOfVersions(spType) + } + + async getVersion (spType, queryIndex) { + return this.VersioningFactoryClient.getVersion(spType, queryIndex) + } + /** * Returns a valid service provider url with the fastest response * @param {string} spType service provider type: 'discovery-provider' | 'content-service' | 'creator-node' From c331c6626501de9306a5372ac7c1801fabb8c9ac Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Mon, 4 May 2020 10:23:53 -0700 Subject: [PATCH 5/8] Clean up --- libs/src/service-selection/ServiceSelection.test.js | 3 ++- .../discoveryProvider/DiscoveryProviderSelection.js | 11 +++++++---- .../DiscoveryProviderSelection.test.js | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libs/src/service-selection/ServiceSelection.test.js b/libs/src/service-selection/ServiceSelection.test.js index ff6caaa437c..f15518b441e 100644 --- a/libs/src/service-selection/ServiceSelection.test.js +++ b/libs/src/service-selection/ServiceSelection.test.js @@ -147,7 +147,8 @@ describe('ServiceSelection', () => { const firstService = await s.select() assert.strictEqual(firstService, atFirstHealthy) - await Utils.wait(200) + // Push the event loop just to let the unhealthy list get cleared + await Utils.wait(0) const secondService = await s.select() assert.strictEqual(secondService, atFirstUnhealthy) }) diff --git a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js index 217aca510dd..f5cdb145592 100644 --- a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js +++ b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js @@ -133,9 +133,10 @@ class DiscoveryProviderSelection extends ServiceSelection { } /** - * In the case of no "healthy" services, we resort to backups. - * 1. Pick the most recent version that's not behind - * 2. Pick the least behind provider and enter "regressed mode" + * In the case of no "healthy" services, we resort to backups in the following order: + * 1. Pick the most recent (patch) version that's not behind + * 2. Pick the least behind provider that is a valid patch version and enter "regressed mode" + * 3. Pick `null` */ async selectFromBackups () { const versions = [] @@ -162,7 +163,9 @@ class DiscoveryProviderSelection extends ServiceSelection { } } - // Go through each backup and record version and block diff maps + // Go through each backup and create two keyed maps: + // { semver => [provider] } + // { blockdiff => [provider] } Object.keys(this.backups).forEach(backup => { const { block_difference: blockDiff, version } = this.backups[backup] // Filter out any version that wasn't registered on chain diff --git a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js index f4b04628885..7f581e4f59f 100644 --- a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js +++ b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.test.js @@ -162,7 +162,7 @@ describe('DiscoveryProviderSelection', () => { assert.strictEqual(s.getTotalAttempts(), 2) }) - it('can select the discprov that is the least number of blocks behind', async () => { + it('can select the discprov that is the least number of blocks behind for the current version', async () => { const behind20 = 'https://behind20.audius.co' nock(behind20) .get('/health_check') From 7b00c8ed0cdd83829f98bbf0e8e09983bd1635b5 Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Mon, 4 May 2020 10:26:46 -0700 Subject: [PATCH 6/8] Revert discprov index changes --- libs/src/services/discoveryProvider/index.js | 83 +++++++++++++++----- 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/libs/src/services/discoveryProvider/index.js b/libs/src/services/discoveryProvider/index.js index 821163a8561..73ff0289ecb 100644 --- a/libs/src/services/discoveryProvider/index.js +++ b/libs/src/services/discoveryProvider/index.js @@ -1,7 +1,7 @@ const axios = require('axios') const Utils = require('../../utils') -const DiscoveryProviderSelection = require('./DiscoveryProviderSelection') +const { serviceType } = require('../ethContracts/index') const { UNHEALTHY_BLOCK_DIFF, @@ -14,6 +14,7 @@ if (urlJoin && urlJoin.default) urlJoin = urlJoin.default const MAKE_REQUEST_RETRY_COUNT = 3 const MAX_MAKE_REQUEST_RETRY_COUNT = 50 +const AUTOSELECT_DISCOVERY_PROVIDER_RETRY_COUNT = 3 class DiscoveryProvider { constructor (autoselect, whitelist, userStateManager, ethContracts, web3Manager) { @@ -22,15 +23,47 @@ class DiscoveryProvider { this.userStateManager = userStateManager this.ethContracts = ethContracts this.web3Manager = web3Manager - - this.serviceSelector = new DiscoveryProviderSelection( - { whitelist }, - ethContracts - ) } async init () { - const endpoint = await this.serviceSelector.select() + let endpoint + let pick + let isValid = null + + if (this.autoselect) { + endpoint = await this.autoSelectEndpoint() + } else { + if (typeof this.whitelist === 'string') { + endpoint = this.whitelist + } else { + if (!this.whitelist || this.whitelist.size === 0) { + throw new Error('Must pass autoselect true or provide whitelist.') + } + + // use this as a lookup between version endpoint and base url + const whitelistMap = {} + this.whitelist.forEach((url) => { + whitelistMap[urlJoin(url, '/version')] = url + }) + + try { + let resp = await Utils.raceRequests(Object.keys(whitelistMap), (url) => { + pick = whitelistMap[url] + }, {}, REQUEST_TIMEOUT_MS) + + isValid = pick && resp.data.service && (resp.data.service === serviceType.DISCOVERY_PROVIDER) + if (isValid) { + console.info('Initial discovery provider was valid') + endpoint = pick + } else { + console.info('Initial discovery provider was invalid, searching for a new one') + endpoint = await this.ethContracts.selectDiscoveryProvider(this.whitelist) + } + } catch (e) { + throw new Error('Could not select a discprov from the whitelist', e) + } + } + } this.setEndpoint(endpoint) if (endpoint && this.web3Manager && this.web3Manager.web3) { @@ -44,6 +77,25 @@ class DiscoveryProvider { this.discoveryProviderEndpoint = endpoint } + /** + * Wrapper method to auto select a valid discovery provider. + * @param {*} retries max retries before throwing an error + * @param {*} clearCachedDiscoveryProvider if set to true, implies that the previously + * selected discovery provider has been failing to serve requests. The prior recurring interval of + * checking local storage for DP and old DP local storage entry need to be cleared. + */ + async autoSelectEndpoint (retries = 3, clearCachedDiscoveryProvider = false) { + if (retries > 0) { + const endpoint = await this.ethContracts.autoselectDiscoveryProvider(this.whitelist, clearCachedDiscoveryProvider) + if (endpoint) { + this.setEndpoint(endpoint) + return endpoint + } + return this.autoSelectEndpoint(retries - 1) + } + throw new Error('Failed to autoselect discovery provider') + } + /** * get users with all relevant user data * can be filtered by providing an integer array of ids @@ -628,16 +680,14 @@ class DiscoveryProvider { } if (!this.discoveryProviderEndpoint) { - const endpoint = await this.serviceSelector.select() - this.setEndpoint(endpoint) + await this.autoSelectEndpoint() } if (retries === 0) { // Reset the retries count in the case that the newly selected disc prov fails, we can // allow it to try MAKE_REQUEST_RETRIES_COUNT number of times before trying another retries = MAKE_REQUEST_RETRY_COUNT - const endpoint = await this.serviceSelector.select() - this.setEndpoint(endpoint) + await this.autoSelectEndpoint(AUTOSELECT_DISCOVERY_PROVIDER_RETRY_COUNT, true) } let requestUrl @@ -671,8 +721,8 @@ class DiscoveryProvider { const parsedResponse = Utils.parseDataFromResponse(response) if ( - this.serviceSelector && - !this.serviceSelector.isInRegressedMode() && + this.ethContracts && + !this.ethContracts.isInRegressedMode() && 'latest_indexed_block' in parsedResponse && 'latest_chain_block' in parsedResponse ) { @@ -686,13 +736,10 @@ class DiscoveryProvider { !indexedBlock || (chainBlock - indexedBlock) > UNHEALTHY_BLOCK_DIFF ) { - console.info(`${this.discoveryProviderEndpoint} is too far behind, reselecting discovery provider`) - // Mark the current selection as a backup - this.serviceSelector.addBackup(this.discoveryProviderEndpoint, response) // Select a new one - const endpoint = await this.serviceSelector.select() + console.info(`${this.discoveryProviderEndpoint} is too far behind, reselecting discovery provider`) + const endpoint = await this.autoSelectEndpoint(AUTOSELECT_DISCOVERY_PROVIDER_RETRY_COUNT, true) this.setEndpoint(endpoint) - retries = MAKE_REQUEST_RETRY_COUNT // reset retry count when setting a new endpoint throw new Error(`Selected endpoint was too far behind. Indexed: ${indexedBlock} Chain: ${chainBlock}`) } From 4a4c0d5a67f67cfddb78cd897a4721814e66cb2c Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Mon, 4 May 2020 10:28:18 -0700 Subject: [PATCH 7/8] Fix imports --- libs/src/services/discoveryProvider/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/src/services/discoveryProvider/index.js b/libs/src/services/discoveryProvider/index.js index 73ff0289ecb..7dfc2bd7fad 100644 --- a/libs/src/services/discoveryProvider/index.js +++ b/libs/src/services/discoveryProvider/index.js @@ -1,6 +1,7 @@ const axios = require('axios') const Utils = require('../../utils') +const { raceRequests } = require('../../utils/network') const { serviceType } = require('../ethContracts/index') const { @@ -47,11 +48,11 @@ class DiscoveryProvider { }) try { - let resp = await Utils.raceRequests(Object.keys(whitelistMap), (url) => { + const { response } = await raceRequests(Object.keys(whitelistMap), (url) => { pick = whitelistMap[url] }, {}, REQUEST_TIMEOUT_MS) - isValid = pick && resp.data.service && (resp.data.service === serviceType.DISCOVERY_PROVIDER) + isValid = pick && response.data.service && (response.data.service === serviceType.DISCOVERY_PROVIDER) if (isValid) { console.info('Initial discovery provider was valid') endpoint = pick From 063cb56e888821d7a2835c48023f90e73fa14c62 Mon Sep 17 00:00:00 2001 From: Raymond Jacobson Date: Mon, 4 May 2020 10:52:59 -0700 Subject: [PATCH 8/8] Fix api call and cleanup --- libs/src/api/file.js | 4 ++-- .../services/discoveryProvider/DiscoveryProviderSelection.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/src/api/file.js b/libs/src/api/file.js index d45bee74faa..28dce95a979 100644 --- a/libs/src/api/file.js +++ b/libs/src/api/file.js @@ -45,7 +45,7 @@ class File extends Base { return retry(async () => { try { - const { response } = raceRequests(urls, callback, { + const { response } = await raceRequests(urls, callback, { method: 'get', responseType: 'blob' }, FETCH_CID_TIMEOUT_MS) @@ -81,7 +81,7 @@ class File extends Base { try { // Races requests and fires the download callback for the first endpoint to // respond with a valid response to a `head` request. - const { response } = raceRequests(urls, (url) => downloadURL(url, filename), { + const { response } = await raceRequests(urls, (url) => downloadURL(url, filename), { method: 'head' }, /* timeout */ 10000) return response diff --git a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js index f5cdb145592..92a51bfee4c 100644 --- a/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js +++ b/libs/src/services/discoveryProvider/DiscoveryProviderSelection.js @@ -38,7 +38,7 @@ class DiscoveryProviderSelection extends ServiceSelection { // unable to select a discovery provider that was up-to-date. Clients may // want to consider blocking writes. this._regressedMode = false - + // Set of valid past discovery provider versions registered on chain this.validVersions = null }