From a69f513a2483bb36d89d04349d00f95b4b37fc08 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 12 Sep 2022 18:13:09 +1000 Subject: [PATCH 01/40] feat: `PolykeyAgent.ts` using `TaskManager` --- src/PolykeyAgent.ts | 46 +++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/PolykeyAgent.ts b/src/PolykeyAgent.ts index 528a092b5..377f816bc 100644 --- a/src/PolykeyAgent.ts +++ b/src/PolykeyAgent.ts @@ -8,7 +8,6 @@ import process from 'process'; import Logger from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { CreateDestroyStartStop } from '@matrixai/async-init/dist/CreateDestroyStartStop'; -import Queue from './nodes/Queue'; import * as networkUtils from './network/utils'; import KeyManager from './keys/KeyManager'; import Status from './status/Status'; @@ -35,6 +34,7 @@ import * as errors from './errors'; import * as utils from './utils'; import * as keysUtils from './keys/utils'; import * as nodesUtils from './nodes/utils'; +import TaskManager from './tasks/TaskManager'; type NetworkConfig = { forwardHost?: Host; @@ -87,8 +87,8 @@ class PolykeyAgent { acl, gestaltGraph, proxy, + taskManager, nodeGraph, - queue, nodeConnectionManager, nodeManager, discovery, @@ -134,8 +134,8 @@ class PolykeyAgent { acl?: ACL; gestaltGraph?: GestaltGraph; proxy?: Proxy; + taskManager?: TaskManager; nodeGraph?: NodeGraph; - queue?: Queue; nodeConnectionManager?: NodeConnectionManager; nodeManager?: NodeManager; discovery?: Discovery; @@ -285,18 +285,21 @@ class PolykeyAgent { keyManager, logger: logger.getChild(NodeGraph.name), })); - queue = - queue ?? - new Queue({ - logger: logger.getChild(Queue.name), - }); + taskManager = + taskManager ?? + (await TaskManager.createTaskManager({ + db, + fresh, + lazy: true, + logger, + })); nodeConnectionManager = nodeConnectionManager ?? new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, seedNodes, ...nodeConnectionManagerConfig_, logger: logger.getChild(NodeConnectionManager.name), @@ -309,7 +312,7 @@ class PolykeyAgent { keyManager, nodeGraph, nodeConnectionManager, - queue, + taskManager, logger: logger.getChild(NodeManager.name), }); await nodeManager.start(); @@ -373,6 +376,7 @@ class PolykeyAgent { await notificationsManager?.stop(); await vaultManager?.stop(); await discovery?.stop(); + await taskManager?.stop(); await proxy?.stop(); await gestaltGraph?.stop(); await acl?.stop(); @@ -396,7 +400,7 @@ class PolykeyAgent { gestaltGraph, proxy, nodeGraph, - queue, + taskManager, nodeConnectionManager, nodeManager, discovery, @@ -429,7 +433,7 @@ class PolykeyAgent { public readonly gestaltGraph: GestaltGraph; public readonly proxy: Proxy; public readonly nodeGraph: NodeGraph; - public readonly queue: Queue; + public readonly taskManager: TaskManager; public readonly nodeConnectionManager: NodeConnectionManager; public readonly nodeManager: NodeManager; public readonly discovery: Discovery; @@ -454,7 +458,7 @@ class PolykeyAgent { gestaltGraph, proxy, nodeGraph, - queue, + taskManager, nodeConnectionManager, nodeManager, discovery, @@ -478,7 +482,7 @@ class PolykeyAgent { gestaltGraph: GestaltGraph; proxy: Proxy; nodeGraph: NodeGraph; - queue: Queue; + taskManager: TaskManager; nodeConnectionManager: NodeConnectionManager; nodeManager: NodeManager; discovery: Discovery; @@ -504,7 +508,7 @@ class PolykeyAgent { this.proxy = proxy; this.discovery = discovery; this.nodeGraph = nodeGraph; - this.queue = queue; + this.taskManager = taskManager; this.nodeConnectionManager = nodeConnectionManager; this.nodeManager = nodeManager; this.vaultManager = vaultManager; @@ -667,7 +671,7 @@ class PolykeyAgent { proxyPort: networkConfig_.proxyPort, tlsConfig, }); - await this.queue.start(); + await this.taskManager.start({ fresh, lazy: true }); await this.nodeManager.start(); await this.nodeConnectionManager.start({ nodeManager: this.nodeManager }); await this.nodeGraph.start({ fresh }); @@ -676,6 +680,7 @@ class PolykeyAgent { await this.vaultManager.start({ fresh }); await this.notificationsManager.start({ fresh }); await this.sessionManager.start({ fresh }); + await this.taskManager.startProcessing(); await this.status.finishStart({ pid: process.pid, nodeId: this.keyManager.getNodeId(), @@ -693,11 +698,13 @@ class PolykeyAgent { this.logger.warn(`Failed Starting ${this.constructor.name}`); this.events.removeAllListeners(); await this.status?.beginStop({ pid: process.pid }); + await this.taskManager?.stopProcessing(); + await this.taskManager?.stopTasks(); await this.sessionManager?.stop(); await this.notificationsManager?.stop(); await this.vaultManager?.stop(); await this.discovery?.stop(); - await this.queue?.stop(); + await this.taskManager?.stop(); await this.nodeGraph?.stop(); await this.nodeConnectionManager?.stop(); await this.nodeManager?.stop(); @@ -723,6 +730,8 @@ class PolykeyAgent { this.logger.info(`Stopping ${this.constructor.name}`); this.events.removeAllListeners(); await this.status.beginStop({ pid: process.pid }); + await this.taskManager.stopProcessing(); + await this.taskManager.stopTasks(); await this.sessionManager.stop(); await this.notificationsManager.stop(); await this.vaultManager.stop(); @@ -730,7 +739,7 @@ class PolykeyAgent { await this.nodeConnectionManager.stop(); await this.nodeGraph.stop(); await this.nodeManager.stop(); - await this.queue.stop(); + await this.taskManager.stop(); await this.proxy.stop(); await this.grpcServerAgent.stop(); await this.grpcServerClient.stop(); @@ -755,6 +764,7 @@ class PolykeyAgent { await this.discovery.destroy(); await this.nodeGraph.destroy(); await this.gestaltGraph.destroy(); + await this.taskManager.destroy(); await this.acl.destroy(); await this.sigchain.destroy(); await this.identitiesManager.destroy(); From 66ee6307c97814c64c07f20bc54dcef7f6e940ea Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 12 Sep 2022 18:31:58 +1000 Subject: [PATCH 02/40] feat: updated `NodeManager` to use `TaskManager` --- src/nodes/NodeManager.ts | 384 +++++++++++++++++--------------- tests/nodes/NodeManager.test.ts | 302 +++++-------------------- 2 files changed, 264 insertions(+), 422 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index aa0740ee5..cb7b79992 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -1,7 +1,6 @@ import type { DB, DBTransaction } from '@matrixai/db'; import type NodeConnectionManager from './NodeConnectionManager'; import type NodeGraph from './NodeGraph'; -import type Queue from './Queue'; import type KeyManager from '../keys/KeyManager'; import type { PublicKeyPem } from '../keys/types'; import type Sigchain from '../sigchain/Sigchain'; @@ -14,7 +13,8 @@ import type { } from '../nodes/types'; import type { ClaimEncoded } from '../claims/types'; import type { Timer } from '../types'; -import type { PromiseDeconstructed } from '../types'; +import type TaskManager from '../tasks/TaskManager'; +import type { TaskHandler, TaskHandlerId, Task } from '../tasks/types'; import Logger from '@matrixai/logger'; import { StartStop, ready } from '@matrixai/async-init/dist/StartStop'; import * as nodesErrors from './errors'; @@ -25,7 +25,7 @@ import * as utilsPB from '../proto/js/polykey/v1/utils/utils_pb'; import * as claimsErrors from '../claims/errors'; import * as sigchainUtils from '../sigchain/utils'; import * as claimsUtils from '../claims/utils'; -import { promise, timerStart } from '../utils/utils'; +import { timerStart, never } from '../utils/utils'; interface NodeManager extends StartStop {} @StartStop() @@ -36,19 +36,40 @@ class NodeManager { protected keyManager: KeyManager; protected nodeConnectionManager: NodeConnectionManager; protected nodeGraph: NodeGraph; - protected queue: Queue; - // Refresh bucket timer - protected refreshBucketDeadlineMap: Map = new Map(); - protected refreshBucketTimer: NodeJS.Timer; - protected refreshBucketNext: NodeBucketIndex; - public readonly refreshBucketTimerDefault; - protected refreshBucketQueue: Set = new Set(); - protected refreshBucketQueueRunning: boolean = false; - protected refreshBucketQueueRunner: Promise; - protected refreshBucketQueuePlug_: PromiseDeconstructed = promise(); - protected refreshBucketQueueDrained_: PromiseDeconstructed = promise(); - protected refreshBucketQueuePause_: PromiseDeconstructed = promise(); - protected refreshBucketQueueAbortController: AbortController; + protected taskManager: TaskManager; + protected refreshBucketDelay: number; + public readonly setNodeHandlerId = + 'NodeManager.setNodeHandler' as TaskHandlerId; + public readonly refreshBucketHandlerId = + 'NodeManager.refreshBucketHandler' as TaskHandlerId; + + private refreshBucketHandler: TaskHandler = async ( + context, + taskInfo, + bucketIndex, + ) => { + await this.refreshBucket(bucketIndex, { signal: context.signal }); + // When completed reschedule the task + await this.taskManager.scheduleTask({ + delay: this.refreshBucketDelay, + handlerId: this.refreshBucketHandlerId, + lazy: true, + parameters: [bucketIndex], + path: ['refreshBucket', `${bucketIndex}`], + priority: 0, + }); + }; + + private setNodeHandler: TaskHandler = async ( + context, + taskInfo, + nodeIdEncoded, + nodeAddress: NodeAddress, + timeout: number, + ) => { + const nodeId: NodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; + await this.setNode(nodeId, nodeAddress, true, false, timeout); + }; constructor({ db, @@ -56,8 +77,8 @@ class NodeManager { sigchain, nodeConnectionManager, nodeGraph, - queue, - refreshBucketTimerDefault = 3600000, // 1 hour in milliseconds + taskManager, + refreshBucketDelay = 3600000, // 1 hour in milliseconds logger, }: { db: DB; @@ -65,8 +86,8 @@ class NodeManager { sigchain: Sigchain; nodeConnectionManager: NodeConnectionManager; nodeGraph: NodeGraph; - queue: Queue; - refreshBucketTimerDefault?: number; + taskManager: TaskManager; + refreshBucketDelay?: number; logger?: Logger; }) { this.logger = logger ?? new Logger(this.constructor.name); @@ -75,21 +96,30 @@ class NodeManager { this.sigchain = sigchain; this.nodeConnectionManager = nodeConnectionManager; this.nodeGraph = nodeGraph; - this.queue = queue; - this.refreshBucketTimerDefault = refreshBucketTimerDefault; + this.taskManager = taskManager; + this.refreshBucketDelay = refreshBucketDelay; } public async start() { this.logger.info(`Starting ${this.constructor.name}`); - this.startRefreshBucketTimers(); - this.refreshBucketQueueRunner = this.startRefreshBucketQueue(); + this.logger.info(`Registering handler for setNode`); + this.taskManager.registerHandler( + this.setNodeHandlerId, + this.setNodeHandler, + ); + this.taskManager.registerHandler( + this.refreshBucketHandlerId, + this.refreshBucketHandler, + ); + await this.setupRefreshBucketTasks(); this.logger.info(`Started ${this.constructor.name}`); } public async stop() { this.logger.info(`Stopping ${this.constructor.name}`); - await this.stopRefreshBucketTimers(); - await this.stopRefreshBucketQueue(); + this.logger.info(`Unregistering handler for setNode`); + this.taskManager.deregisterHandler(this.setNodeHandlerId); + this.taskManager.deregisterHandler(this.refreshBucketHandlerId); this.logger.info(`Stopped ${this.constructor.name}`); } @@ -390,6 +420,7 @@ class NodeManager { ); } + // FIXME: make cancelable /** * Adds a node to the node graph. This assumes that you have already authenticated the node * Updates the node if the node already exists @@ -444,7 +475,12 @@ class NodeManager { // We want to add or update the node await this.nodeGraph.setNode(nodeId, nodeAddress, tran); // Updating the refreshBucket timer - this.refreshBucketUpdateDeadline(bucketIndex); + await this.updateRefreshBucketDelay( + bucketIndex, + this.refreshBucketDelay, + true, + tran, + ); } else { // We want to add a node but the bucket is full // We need to ping the oldest node @@ -461,7 +497,12 @@ class NodeManager { await this.nodeGraph.unsetNode(oldNodeId, tran); await this.nodeGraph.setNode(nodeId, nodeAddress, tran); // Updating the refreshBucket timer - this.refreshBucketUpdateDeadline(bucketIndex); + await this.updateRefreshBucketDelay( + bucketIndex, + this.refreshBucketDelay, + true, + tran, + ); return; } else if (block) { this.logger.debug( @@ -481,14 +522,21 @@ class NodeManager { nodeId, )} to queue`, ); - // Re-attempt this later asynchronously by adding the the queue - this.queue.push(() => - this.setNode(nodeId, nodeAddress, true, false, timeout), + // Re-attempt this later asynchronously by adding to the scheduler + await this.taskManager.scheduleTask( + { + handlerId: this.setNodeHandlerId, + parameters: [nodesUtils.toString(), nodeAddress, timeout], + path: ['setNode'], + lazy: true, + }, + tran, ); } } } + // FIXME: make cancellable private async garbageCollectOldNode( bucketIndex: number, nodeId: NodeId, @@ -497,6 +545,8 @@ class NodeManager { ) { const oldestNodeIds = await this.nodeGraph.getOldestNode(bucketIndex, 3); // We want to concurrently ping the nodes + // Fixme, remove concurrency? we'd want to stick to 1 active connection per + // background task const pingPromises = oldestNodeIds.map((nodeId) => { const doPing = async (): Promise<{ nodeId: NodeId; @@ -521,10 +571,13 @@ class NodeManager { const node = (await this.nodeGraph.getNode(nodeId))!; await this.nodeGraph.setNode(nodeId, node.address); // Updating the refreshBucket timer - this.refreshBucketUpdateDeadline(bucketIndex); + await this.updateRefreshBucketDelay( + bucketIndex, + this.refreshBucketDelay, + ); } else { this.logger.debug(`Ping failed for ${nodesUtils.encodeNodeId(nodeId)}`); - // Otherwise we remove the node + // Otherwise, we remove the node await this.nodeGraph.unsetNode(nodeId); } } @@ -534,7 +587,7 @@ class NodeManager { this.logger.debug(`Bucket ${bucketIndex} now has room, adding new node`); await this.nodeGraph.setNode(nodeId, nodeAddress); // Updating the refreshBucket timer - this.refreshBucketUpdateDeadline(bucketIndex); + await this.updateRefreshBucketDelay(bucketIndex, this.refreshBucketDelay); } } @@ -576,166 +629,139 @@ class NodeManager { await this.nodeConnectionManager.findNode(bucketRandomNodeId, { signal }); } - // Refresh bucket activity timer methods + private async setupRefreshBucketTasks(tran?: DBTransaction) { + if (tran == null) { + return this.db.withTransactionF((tran) => + this.setupRefreshBucketTasks(tran), + ); + } - private startRefreshBucketTimers() { - // Setting initial bucket to refresh - this.refreshBucketNext = 0; - // Setting initial deadline - this.refreshBucketTimerReset(this.refreshBucketTimerDefault); + this.logger.info('Setting up refreshBucket tasks'); + // 1. Iterate over existing tasks and reset the delay + const existingTasks: Array = new Array(this.nodeGraph.nodeIdBits); + for await (const task of this.taskManager.getTasks( + 'asc', + true, + ['refreshBucket'], + tran, + )) { + const bucketIndex = parseInt(task.path[0]); + switch (task.status) { + case 'scheduled': + { + // If it's scheduled then reset delay + existingTasks[bucketIndex] = true; + this.logger.debug( + `Updating refreshBucket delay for bucket ${bucketIndex}`, + ); + // Total delay is refreshBucketDelay + time since task creation + const delay = + performance.now() + + performance.timeOrigin - + task.created.getTime() + + this.refreshBucketDelay; + await this.taskManager.updateTask(task.id, { delay }, tran); + } + break; + case 'queued': + case 'active': + // If it's running then leave it + this.logger.debug( + `RefreshBucket task for bucket ${bucketIndex} is already active, ignoring`, + ); + existingTasks[bucketIndex] = true; + break; + default: + // Otherwise ignore it, should be re-created + existingTasks[bucketIndex] = false; + } + } + + // 2. Recreate any missing tasks for buckets for ( let bucketIndex = 0; - bucketIndex < this.nodeGraph.nodeIdBits; + bucketIndex < existingTasks.length; bucketIndex++ ) { - const deadline = Date.now() + this.refreshBucketTimerDefault; - this.refreshBucketDeadlineMap.set(bucketIndex, deadline); + const exists = existingTasks[bucketIndex]; + if (!exists) { + // Create a new task + this.logger.debug( + `Creating refreshBucket task for bucket ${bucketIndex}`, + ); + await this.taskManager.scheduleTask({ + handlerId: this.refreshBucketHandlerId, + delay: this.refreshBucketDelay, + lazy: true, + parameters: [bucketIndex], + path: ['refreshBucket', `${bucketIndex}`], + priority: 0, + }); + } } + this.logger.info('Set up refreshBucket tasks'); } - private async stopRefreshBucketTimers() { - clearTimeout(this.refreshBucketTimer); - } - - private refreshBucketTimerReset(timeout: number) { - clearTimeout(this.refreshBucketTimer); - this.refreshBucketTimer = setTimeout(() => { - this.refreshBucketRefreshTimer(); - }, timeout); - } - - public refreshBucketUpdateDeadline(bucketIndex: NodeBucketIndex) { - // Update the map deadline - this.refreshBucketDeadlineMap.set( - bucketIndex, - Date.now() + this.refreshBucketTimerDefault, - ); - // If the bucket was pending a refresh we remove it - this.refreshBucketQueueRemove(bucketIndex); - if (bucketIndex === this.refreshBucketNext) { - // Bucket is same as next bucket, this affects the timer - this.refreshBucketRefreshTimer(); + @ready(new nodesErrors.ErrorNodeManagerNotRunning()) + public async updateRefreshBucketDelay( + bucketIndex: number, + delay: number = this.refreshBucketDelay, + lazy: boolean = true, + tran?: DBTransaction, + ): Promise { + if (tran == null) { + return this.db.withTransactionF((tran) => + this.updateRefreshBucketDelay(bucketIndex, delay, lazy, tran), + ); } - } - private refreshBucketRefreshTimer() { - // Getting new closest deadline - let closestBucket = this.refreshBucketNext; - let closestDeadline = Date.now() + this.refreshBucketTimerDefault; - const now = Date.now(); - for (const [bucketIndex, deadline] of this.refreshBucketDeadlineMap) { - // Skip any queued buckets marked by 0 deadline - if (deadline === 0) continue; - if (deadline <= now) { - // Deadline for this has already passed, we add it to the queue - this.refreshBucketQueueAdd(bucketIndex); - continue; - } - if (deadline < closestDeadline) { - closestBucket = bucketIndex; - closestDeadline = deadline; + let foundTask: Task | undefined; + let count = 0; + for await (const task of this.taskManager.getTasks( + 'asc', + true, + ['refreshBucket', `${bucketIndex}`], + tran, + )) { + count += 1; + if (count <= 1) { + foundTask = task; + // Update the first one + // total delay is refreshBucketDelay + time since task creation + const delay = + performance.now() + + performance.timeOrigin - + task.created.getTime() + + this.refreshBucketDelay; + await this.taskManager.updateTask(task.id, { delay }, tran); + this.logger.debug( + 'Updating refreshBucket task for bucket ${bucketIndex}', + ); + } else { + // These are extra, so we cancel them + // TODO: make error + task.cancel(Error('TMP, cancel extra tasks')); + this.logger.warn( + `Duplicate refreshBucket task was found for bucket ${bucketIndex}, cancelling`, + ); } } - // Working out time left - const timeout = closestDeadline - Date.now(); - this.logger.debug( - `Refreshing refreshBucket timer with new timeout ${timeout}`, - ); - // Updating timer and next - this.refreshBucketNext = closestBucket; - this.refreshBucketTimerReset(timeout); - } - - // Refresh bucket async queue methods - - public refreshBucketQueueAdd(bucketIndex: NodeBucketIndex) { - this.logger.debug(`Adding bucket ${bucketIndex} to queue`); - this.refreshBucketDeadlineMap.set(bucketIndex, 0); - this.refreshBucketQueue.add(bucketIndex); - this.refreshBucketQueueUnplug(); - } - - public refreshBucketQueueRemove(bucketIndex: NodeBucketIndex) { - this.logger.debug(`Removing bucket ${bucketIndex} from queue`); - this.refreshBucketQueue.delete(bucketIndex); - } - - public async refreshBucketQueueDrained() { - await this.refreshBucketQueueDrained_.p; - } - - public refreshBucketQueuePause() { - this.logger.debug('Pausing refreshBucketQueue'); - this.refreshBucketQueuePause_ = promise(); - } - - public refreshBucketQueueResume() { - this.logger.debug('Resuming refreshBucketQueue'); - this.refreshBucketQueuePause_.resolveP(); - } - - private async startRefreshBucketQueue(): Promise { - this.refreshBucketQueueRunning = true; - this.refreshBucketQueuePlug(); - this.refreshBucketQueueResume(); - let iterator: IterableIterator | undefined; - this.refreshBucketQueueAbortController = new AbortController(); - const pace = async () => { - // Wait if paused - await this.refreshBucketQueuePause_.p; - // Wait for plug - await this.refreshBucketQueuePlug_.p; - if (iterator == null) { - iterator = this.refreshBucketQueue[Symbol.iterator](); - } - return this.refreshBucketQueueRunning; - }; - while (await pace()) { - const bucketIndex: NodeBucketIndex = iterator?.next().value; - if (bucketIndex == null) { - // Iterator is empty, plug and continue - iterator = undefined; - this.refreshBucketQueuePlug(); - continue; - } - // Do the job - this.logger.debug( - `processing refreshBucket for bucket ${bucketIndex}, ${this.refreshBucketQueue.size} left in queue`, + if (count === 0) { + this.logger.warn( + `No refreshBucket task for bucket ${bucketIndex}, new one was created`, ); - try { - await this.refreshBucket(bucketIndex, { - signal: this.refreshBucketQueueAbortController.signal, - }); - } catch (e) { - if (e instanceof nodesErrors.ErrorNodeAborted) break; - throw e; - } - // Remove from queue and update bucket deadline - this.refreshBucketQueue.delete(bucketIndex); - this.refreshBucketUpdateDeadline(bucketIndex); + foundTask = await this.taskManager.scheduleTask({ + delay: this.refreshBucketDelay, + handlerId: this.refreshBucketHandlerId, + lazy: true, + parameters: [bucketIndex], + path: ['refreshBucket', `${bucketIndex}`], + priority: 0, + }); } - this.logger.debug('startRefreshBucketQueue has ended'); - } - - private async stopRefreshBucketQueue(): Promise { - // Flag end and await queue finish - this.refreshBucketQueueAbortController.abort(); - this.refreshBucketQueueRunning = false; - this.refreshBucketQueueUnplug(); - this.refreshBucketQueueResume(); - } - - private refreshBucketQueuePlug() { - this.logger.debug('refresh bucket queue has plugged'); - this.refreshBucketQueuePlug_ = promise(); - this.refreshBucketQueueDrained_?.resolveP(); - } - - private refreshBucketQueueUnplug() { - this.logger.debug('refresh bucket queue has unplugged'); - this.refreshBucketQueueDrained_ = promise(); - this.refreshBucketQueuePlug_?.resolveP(); + if (foundTask == null) never(); + return foundTask; } } diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index f2ed4dfb5..3c0650742 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -7,7 +7,7 @@ import fs from 'fs'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import UTP from 'utp-native'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; import * as keysUtils from '@/keys/utils'; @@ -17,13 +17,15 @@ import NodeManager from '@/nodes/NodeManager'; import Proxy from '@/network/Proxy'; import Sigchain from '@/sigchain/Sigchain'; import * as claimsUtils from '@/claims/utils'; -import { promise, promisify, sleep } from '@/utils'; +import { never, promise, promisify, sleep } from '@/utils'; import * as nodesUtils from '@/nodes/utils'; import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb'; import * as nodesErrors from '@/nodes/errors'; import * as nodesTestUtils from './utils'; import { generateNodeIdForBucket } from './utils'; import { globalRootKeyPems } from '../fixtures/globalRootKeyPems'; +import { before } from 'cheerio/lib/api/manipulation'; +import { Task } from '@/tasks/types'; describe(`${NodeManager.name} test`, () => { const password = 'password'; @@ -32,7 +34,7 @@ describe(`${NodeManager.name} test`, () => { ]); let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let proxy: Proxy; let keyManager: KeyManager; @@ -108,11 +110,16 @@ describe(`${NodeManager.name} test`, () => { keyManager, logger, }); - queue = new Queue({ logger }); + taskManager = await TaskManager.createTaskManager({ + activeLimit: 0, + db, + lazy: true, + logger, + }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, - queue, + taskManager, proxy, logger, }); @@ -121,7 +128,7 @@ describe(`${NodeManager.name} test`, () => { mockedPingNode.mockClear(); mockedPingNode.mockImplementation(async (_) => true); await nodeConnectionManager.stop(); - await queue.stop(); + await taskManager.stop(); await nodeGraph.stop(); await nodeGraph.destroy(); await sigchain.stop(); @@ -168,11 +175,12 @@ describe(`${NodeManager.name} test`, () => { keyManager, nodeGraph, nodeConnectionManager, - queue, + taskManager, logger, }); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); // Set server node offline await server.stop(); @@ -244,11 +252,12 @@ describe(`${NodeManager.name} test`, () => { keyManager, nodeGraph, nodeConnectionManager, - queue, + taskManager, logger, }); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); // We want to get the public key of the server const key = await nodeManager.getPublicKey(serverNodeId); @@ -435,11 +444,12 @@ describe(`${NodeManager.name} test`, () => { keyManager, nodeGraph, nodeConnectionManager, - queue, + taskManager, logger, }); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); await nodeGraph.setNode(xNodeId, xNodeAddress); @@ -455,20 +465,19 @@ describe(`${NodeManager.name} test`, () => { }); }); test('should add a node when bucket has room', async () => { - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: {} as NodeConnectionManager, - queue, + taskManager, logger, }); try { - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; const nodeId = nodesTestUtils.generateNodeIdForBucket( @@ -482,24 +491,22 @@ describe(`${NodeManager.name} test`, () => { expect(bucket).toHaveLength(1); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should update a node if node exists', async () => { - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: {} as NodeConnectionManager, - queue, + taskManager, logger, }); try { - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; const nodeId = nodesTestUtils.generateNodeIdForBucket( @@ -525,24 +532,22 @@ describe(`${NodeManager.name} test`, () => { expect(newNodeData.lastUpdated).not.toEqual(nodeData.lastUpdated); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should not add node if bucket is full and old node is alive', async () => { - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: {} as NodeConnectionManager, - queue, + taskManager, logger, }); try { - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; // Creating 20 nodes in bucket @@ -579,24 +584,22 @@ describe(`${NodeManager.name} test`, () => { nodeManagerPingMock.mockRestore(); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should add node if bucket is full, old node is alive and force is set', async () => { - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: {} as NodeConnectionManager, - queue, + taskManager, logger, }); try { - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; // Creating 20 nodes in bucket @@ -635,24 +638,22 @@ describe(`${NodeManager.name} test`, () => { nodeManagerPingMock.mockRestore(); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should add node if bucket is full and old node is dead', async () => { - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: {} as NodeConnectionManager, - queue, + taskManager, logger, }); try { - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; // Creating 20 nodes in bucket @@ -683,25 +684,23 @@ describe(`${NodeManager.name} test`, () => { nodeManagerPingMock.mockRestore(); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should add node when an incoming connection is established', async () => { let server: PolykeyAgent | undefined; - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: {} as NodeConnectionManager, - queue, + taskManager, logger, }); try { - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); server = await PolykeyAgent.createPolykeyAgent({ password: 'password', nodePath: path.join(dataDir, 'server'), @@ -742,25 +741,23 @@ describe(`${NodeManager.name} test`, () => { await server?.stop(); await server?.destroy(); await nodeManager.stop(); - await queue.stop(); } }); test('should not add nodes to full bucket if pings succeeds', async () => { mockedPingNode.mockImplementation(async (_) => true); - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: dummyNodeConnectionManager, - queue, + taskManager, logger, }); try { - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -784,25 +781,23 @@ describe(`${NodeManager.name} test`, () => { ); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should add nodes to full bucket if pings fail', async () => { mockedPingNode.mockImplementation(async (_) => true); - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: dummyNodeConnectionManager, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); try { await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -825,14 +820,12 @@ describe(`${NodeManager.name} test`, () => { await nodeManager.setNode(newNode1, address); await nodeManager.setNode(newNode2, address); await nodeManager.setNode(newNode3, address); - await queue.drained(); const list = await listBucket(100); expect(list).toContain(nodesUtils.encodeNodeId(newNode1)); expect(list).toContain(nodesUtils.encodeNodeId(newNode2)); expect(list).toContain(nodesUtils.encodeNodeId(newNode3)); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should not block when bucket is full', async () => { @@ -842,20 +835,19 @@ describe(`${NodeManager.name} test`, () => { logger, }); mockedPingNode.mockImplementation(async (_) => true); - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph: tempNodeGraph, nodeConnectionManager: dummyNodeConnectionManager, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); try { await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -876,30 +868,27 @@ describe(`${NodeManager.name} test`, () => { nodeManager.setNode(newNode4, address, false), ).resolves.toBeUndefined(); delayPing.resolveP(); - await queue.drained(); } finally { await nodeManager.stop(); - await queue.stop(); await tempNodeGraph.stop(); await tempNodeGraph.destroy(); } }); test('should block when blocking is set to true', async () => { mockedPingNode.mockImplementation(async (_) => true); - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: dummyNodeConnectionManager, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); try { await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -918,20 +907,18 @@ describe(`${NodeManager.name} test`, () => { expect(mockedPingNode).toBeCalled(); } finally { await nodeManager.stop(); - await queue.stop(); } }); test('should update deadline when updating a bucket', async () => { const refreshBucketTimeout = 100000; - const queue = new Queue({ logger }); const nodeManager = new NodeManager({ db, sigchain: {} as Sigchain, keyManager, nodeGraph, nodeConnectionManager: dummyNodeConnectionManager, - queue, - refreshBucketTimerDefault: refreshBucketTimeout, + taskManager, + refreshBucketDelay: refreshBucketTimeout, logger, }); const mockRefreshBucket = jest.spyOn( @@ -940,204 +927,32 @@ describe(`${NodeManager.name} test`, () => { ); try { mockRefreshBucket.mockImplementation(async () => {}); - await queue.start(); + await taskManager.startProcessing(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); - // @ts-ignore: kidnap map - const deadlineMap = nodeManager.refreshBucketDeadlineMap; // Getting starting value - const bucket = 0; - const startingDeadline = deadlineMap.get(bucket); + const bucketIndex = 100; + let refreshBucketTask: Task | undefined; + for await (const task of taskManager.getTasks('asc', true, ['refreshBucket', `${bucketIndex}`])){ + refreshBucketTask = task; + } + if (refreshBucketTask == null) never(); const nodeId = nodesTestUtils.generateNodeIdForBucket( keyManager.getNodeId(), - bucket, + bucketIndex, ); - await sleep(1000); + await sleep(100); await nodeManager.setNode(nodeId, {} as NodeAddress); // Deadline should be updated - const newDeadline = deadlineMap.get(bucket); - expect(newDeadline).not.toEqual(startingDeadline); - } finally { - mockRefreshBucket.mockRestore(); - await nodeManager.stop(); - await queue.stop(); - } - }); - test('should add buckets to the queue when exceeding deadline', async () => { - const refreshBucketTimeout = 100; - const queue = new Queue({ logger }); - const nodeManager = new NodeManager({ - db, - sigchain: {} as Sigchain, - keyManager, - nodeGraph, - nodeConnectionManager: dummyNodeConnectionManager, - queue, - refreshBucketTimerDefault: refreshBucketTimeout, - logger, - }); - const mockRefreshBucket = jest.spyOn( - NodeManager.prototype, - 'refreshBucket', - ); - const mockRefreshBucketQueueAdd = jest.spyOn( - NodeManager.prototype, - 'refreshBucketQueueAdd', - ); - try { - mockRefreshBucket.mockImplementation(async () => {}); - await queue.start(); - await nodeManager.start(); - await nodeConnectionManager.start({ nodeManager }); - // Getting starting value - expect(mockRefreshBucketQueueAdd).toHaveBeenCalledTimes(0); - await sleep(200); - expect(mockRefreshBucketQueueAdd).toHaveBeenCalledTimes(256); - } finally { - mockRefreshBucketQueueAdd.mockRestore(); - mockRefreshBucket.mockRestore(); - await nodeManager.stop(); - await queue.stop(); - } - }); - test('should digest queue to refresh buckets', async () => { - const refreshBucketTimeout = 1000000; - const queue = new Queue({ logger }); - const nodeManager = new NodeManager({ - db, - sigchain: {} as Sigchain, - keyManager, - nodeGraph, - nodeConnectionManager: dummyNodeConnectionManager, - queue, - refreshBucketTimerDefault: refreshBucketTimeout, - logger, - }); - const mockRefreshBucket = jest.spyOn( - NodeManager.prototype, - 'refreshBucket', - ); - try { - await queue.start(); - await nodeManager.start(); - await nodeConnectionManager.start({ nodeManager }); - mockRefreshBucket.mockImplementation(async () => {}); - nodeManager.refreshBucketQueueAdd(1); - nodeManager.refreshBucketQueueAdd(2); - nodeManager.refreshBucketQueueAdd(3); - nodeManager.refreshBucketQueueAdd(4); - nodeManager.refreshBucketQueueAdd(5); - await nodeManager.refreshBucketQueueDrained(); - expect(mockRefreshBucket).toHaveBeenCalledTimes(5); - - // Add buckets to queue - // check if refresh buckets was called - } finally { - mockRefreshBucket.mockRestore(); - await nodeManager.stop(); - await queue.stop(); - } - }); - test('should abort refreshBucket queue when stopping', async () => { - const refreshBucketTimeout = 1000000; - const queue = new Queue({ logger }); - const nodeManager = new NodeManager({ - db, - sigchain: {} as Sigchain, - keyManager, - nodeGraph, - nodeConnectionManager: dummyNodeConnectionManager, - queue, - refreshBucketTimerDefault: refreshBucketTimeout, - logger, - }); - const mockRefreshBucket = jest.spyOn( - NodeManager.prototype, - 'refreshBucket', - ); - try { - await queue.start(); - await nodeManager.start(); - await nodeConnectionManager.start({ nodeManager }); - mockRefreshBucket.mockImplementation( - async (bucket, options: { signal?: AbortSignal } = {}) => { - const { signal } = { ...options }; - const prom = promise(); - signal?.addEventListener('abort', () => - prom.rejectP(new nodesErrors.ErrorNodeAborted()), - ); - await prom.p; - }, - ); - nodeManager.refreshBucketQueueAdd(1); - nodeManager.refreshBucketQueueAdd(2); - nodeManager.refreshBucketQueueAdd(3); - nodeManager.refreshBucketQueueAdd(4); - nodeManager.refreshBucketQueueAdd(5); - await nodeManager.stop(); - } finally { - mockRefreshBucket.mockRestore(); - await nodeManager.stop(); - await queue.stop(); - } - }); - test('should pause, resume and stop queue while paused', async () => { - const refreshBucketTimeout = 1000000; - const queue = new Queue({ logger }); - const nodeManager = new NodeManager({ - db, - sigchain: {} as Sigchain, - keyManager, - nodeGraph, - nodeConnectionManager: dummyNodeConnectionManager, - queue, - refreshBucketTimerDefault: refreshBucketTimeout, - logger, - }); - const mockRefreshBucket = jest.spyOn( - NodeManager.prototype, - 'refreshBucket', - ); - try { - logger.setLevel(LogLevel.WARN); - await queue.start(); - await nodeManager.start(); - await nodeConnectionManager.start({ nodeManager }); - mockRefreshBucket.mockImplementation( - async (bucket, options: { signal?: AbortSignal } = {}) => { - const { signal } = { ...options }; - const prom = promise(); - const timer = setTimeout(prom.resolveP, 10); - signal?.addEventListener('abort', () => { - clearTimeout(timer); - prom.rejectP(new nodesErrors.ErrorNodeAborted()); - }); - await prom.p; - }, - ); - nodeManager.refreshBucketQueueAdd(1); - nodeManager.refreshBucketQueueAdd(2); - nodeManager.refreshBucketQueueAdd(3); - nodeManager.refreshBucketQueueAdd(4); - nodeManager.refreshBucketQueueAdd(5); - - // Can pause and resume - nodeManager.refreshBucketQueuePause(); - nodeManager.refreshBucketQueueAdd(6); - nodeManager.refreshBucketQueueAdd(7); - nodeManager.refreshBucketQueueResume(); - await nodeManager.refreshBucketQueueDrained(); - - // Can pause and stop - nodeManager.refreshBucketQueuePause(); - nodeManager.refreshBucketQueueAdd(8); - nodeManager.refreshBucketQueueAdd(9); - nodeManager.refreshBucketQueueAdd(10); - await nodeManager.stop(); + let refreshBucketTaskUpdated: Task | undefined; + for await (const task of taskManager.getTasks('asc', true, ['refreshBucket', `${bucketIndex}`])){ + refreshBucketTaskUpdated = task; + } + if (refreshBucketTaskUpdated == null) never(); + expect(refreshBucketTaskUpdated.delay).not.toEqual(refreshBucketTask.delay); } finally { mockRefreshBucket.mockRestore(); await nodeManager.stop(); - await queue.stop(); } }); test('refreshBucket should not throw errors when network is empty', async () => { @@ -1147,11 +962,12 @@ describe(`${NodeManager.name} test`, () => { keyManager, nodeGraph, nodeConnectionManager, - queue, - refreshBucketTimerDefault: 10000000, + taskManager, + refreshBucketDelay: 10000000, logger, }); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); try { await expect(nodeManager.refreshBucket(100)).resolves.not.toThrow(); } finally { From 41d3a00f2c64e3c22b31fd35e683a2f62a56c08b Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 12 Sep 2022 18:36:35 +1000 Subject: [PATCH 03/40] feat: updated `NodeConnectionManager` to use `TaskManager` --- src/nodes/NodeConnectionManager.ts | 96 +++++++++++++------ src/nodes/NodeManager.ts | 11 ++- tests/nodes/NodeConnection.test.ts | 17 ++-- .../NodeConnectionManager.general.test.ts | 24 +++-- .../NodeConnectionManager.lifecycle.test.ts | 52 ++++++---- .../NodeConnectionManager.seednodes.test.ts | 56 +++++------ .../NodeConnectionManager.termination.test.ts | 24 +++-- .../NodeConnectionManager.timeout.test.ts | 12 ++- tests/nodes/NodeManager.test.ts | 18 ++-- 9 files changed, 181 insertions(+), 129 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index c1f5c1a85..2068e6aaf 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -4,7 +4,7 @@ import type Proxy from '../network/Proxy'; import type { Host, Hostname, Port } from '../network/types'; import type { Timer } from '../types'; import type NodeGraph from './NodeGraph'; -import type Queue from './Queue'; +import type TaskManager from '../tasks/TaskManager'; import type { NodeAddress, NodeData, @@ -13,6 +13,7 @@ import type { SeedNodes, } from './types'; import type NodeManager from './NodeManager'; +import type { TaskHandler, TaskHandlerId } from 'tasks/types'; import { withF } from '@matrixai/resources'; import Logger from '@matrixai/logger'; import { ready, StartStop } from '@matrixai/async-init/dist/StartStop'; @@ -57,7 +58,7 @@ class NodeConnectionManager { protected nodeGraph: NodeGraph; protected keyManager: KeyManager; protected proxy: Proxy; - protected queue: Queue; + protected taskManager: TaskManager; // NodeManager has to be passed in during start to allow co-dependency protected nodeManager: NodeManager | undefined; protected seedNodes: SeedNodes; @@ -74,11 +75,28 @@ class NodeConnectionManager { protected connections: Map = new Map(); protected connectionLocks: LockBox = new LockBox(); + protected pingAndSetNodeHandlerId: TaskHandlerId = + 'NodeConnectionManager.pingAndSetNodeHandler' as TaskHandlerId; + // TODO: make cancelable + protected pingAndSetNodeHandler: TaskHandler = async ( + context, + taskInfo, + nodeIdEncoded: string, + host: Host, + port: Port, + ) => { + const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; + const host_ = await networkUtils.resolveHost(host); + if (await this.pingNode(nodeId, host_, port)) { + await this.nodeManager!.setNode(nodeId, { host: host_, port }, true); + } + }; + public constructor({ keyManager, nodeGraph, proxy, - queue, + taskManager, seedNodes = {}, initialClosestNodes = 3, connConnectTime = 20000, @@ -88,7 +106,7 @@ class NodeConnectionManager { nodeGraph: NodeGraph; keyManager: KeyManager; proxy: Proxy; - queue: Queue; + taskManager: TaskManager; seedNodes?: SeedNodes; initialClosestNodes?: number; connConnectTime?: number; @@ -99,7 +117,7 @@ class NodeConnectionManager { this.keyManager = keyManager; this.nodeGraph = nodeGraph; this.proxy = proxy; - this.queue = queue; + this.taskManager = taskManager; this.seedNodes = seedNodes; this.initialClosestNodes = initialClosestNodes; this.connConnectTime = connConnectTime; @@ -109,6 +127,12 @@ class NodeConnectionManager { public async start({ nodeManager }: { nodeManager: NodeManager }) { this.logger.info(`Starting ${this.constructor.name}`); this.nodeManager = nodeManager; + // Setting handlers + this.taskManager.registerHandler( + this.pingAndSetNodeHandlerId, + this.pingAndSetNodeHandler, + ); + // Adding seed nodes for (const nodeIdEncoded in this.seedNodes) { const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; await this.nodeManager.setNode( @@ -130,6 +154,8 @@ class NodeConnectionManager { // It exists so we want to destroy it await this.destroyConnection(IdInternal.fromString(nodeId)); } + // Removing handlers + this.taskManager.deregisterHandler(this.pingAndSetNodeHandlerId); this.logger.info(`Stopped ${this.constructor.name}`); } @@ -605,26 +631,29 @@ class NodeConnectionManager { if (e instanceof nodesErrors.ErrorNodeConnectionTimeout) continue; throw e; } - const nodes = await this.getRemoteNodeClosestNodes( + const closestNodes = await this.getRemoteNodeClosestNodes( seedNodeId, this.keyManager.getNodeId(), timer, ); - for (const [nodeId, nodeData] of nodes) { + for (const [nodeId, nodeData] of closestNodes) { if (!nodeId.equals(this.keyManager.getNodeId())) { - const pingAndAddNode = async () => { - const port = nodeData.address.port; - const host = await networkUtils.resolveHost(nodeData.address.host); - if (await this.pingNode(nodeId, host, port)) { - await this.nodeManager!.setNode(nodeId, nodeData.address, true); - } - }; - - if (!block) { - this.queue.push(pingAndAddNode); - } else { + const pingAndSetTask = await this.taskManager.scheduleTask({ + delay: 0, + handlerId: this.pingAndSetNodeHandlerId, + lazy: !block, + parameters: [ + nodesUtils.encodeNodeId(nodeId), + nodeData.address.host, + nodeData.address.port, + ], + path: ['pingAndSetNode'], + // Need to be somewhat active so high priority + priority: 100, + }); + if (block) { try { - await pingAndAddNode(); + await pingAndSetTask.promise(); } catch (e) { if (!(e instanceof nodesErrors.ErrorNodeGraphSameNodeId)) throw e; } @@ -632,19 +661,24 @@ class NodeConnectionManager { } } // Refreshing every bucket above the closest node - const refreshBuckets = async () => { - const [closestNode] = ( - await this.nodeGraph.getClosestNodes(this.keyManager.getNodeId(), 1) - ).pop()!; + let closestNodeInfo = closestNodes.pop()!; + if (this.keyManager.getNodeId().equals(closestNodeInfo[0])) { + // Skip our nodeId if it exists + closestNodeInfo = closestNodes.pop()!; + } + let index = 0; + if (closestNodeInfo != null) { + const [closestNode] = closestNodeInfo; const [bucketIndex] = this.nodeGraph.bucketIndex(closestNode); - for (let i = bucketIndex; i < this.nodeGraph.nodeIdBits; i++) { - this.nodeManager?.refreshBucketQueueAdd(i); - } - }; - if (!block) { - this.queue.push(refreshBuckets); - } else { - await refreshBuckets(); + index = bucketIndex; + } + for (let i = index; i < this.nodeGraph.nodeIdBits; i++) { + const task = await this.nodeManager!.updateRefreshBucketDelay( + i, + 0, + !block, + ); + if (block) await task.promise(); } } } diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index cb7b79992..6aa7a1ce3 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -727,16 +727,19 @@ class NodeManager { count += 1; if (count <= 1) { foundTask = task; + // If already running then don't update + if (task.status !== 'scheduled') continue; // Update the first one // total delay is refreshBucketDelay + time since task creation - const delay = + // time since task creation = now - creation time; + const delayNew = performance.now() + performance.timeOrigin - task.created.getTime() + - this.refreshBucketDelay; - await this.taskManager.updateTask(task.id, { delay }, tran); + delay; + await this.taskManager.updateTask(task.id, { delay: delayNew }, tran); this.logger.debug( - 'Updating refreshBucket task for bucket ${bucketIndex}', + `Updating refreshBucket task for bucket ${bucketIndex}`, ); } else { // These are extra, so we cancel them diff --git a/tests/nodes/NodeConnection.test.ts b/tests/nodes/NodeConnection.test.ts index 3afb53aa1..228fd5b1a 100644 --- a/tests/nodes/NodeConnection.test.ts +++ b/tests/nodes/NodeConnection.test.ts @@ -10,6 +10,7 @@ import fs from 'fs'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { destroyed } from '@matrixai/async-init'; +import TaskManager from '@/tasks/TaskManager'; import Proxy from '@/network/Proxy'; import NodeConnection from '@/nodes/NodeConnection'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; @@ -33,7 +34,6 @@ import * as nodesUtils from '@/nodes/utils'; import * as agentErrors from '@/agent/errors'; import * as grpcUtils from '@/grpc/utils'; import { timerStart } from '@/utils'; -import Queue from '@/nodes/Queue'; import * as testNodesUtils from './utils'; import * as grpcTestUtils from '../grpc/utils'; import * as agentTestUtils from '../agent/utils'; @@ -85,7 +85,6 @@ describe(`${NodeConnection.name} test`, () => { let serverKeyManager: KeyManager; let serverVaultManager: VaultManager; let serverNodeGraph: NodeGraph; - let serverQueue: Queue; let serverNodeConnectionManager: NodeConnectionManager; let serverNodeManager: NodeManager; let serverSigchain: Sigchain; @@ -111,6 +110,7 @@ describe(`${NodeConnection.name} test`, () => { let sourcePort: Port; let serverTLSConfig: TLSConfig; + let serverTaskManager: TaskManager; /** * Mock TCP server @@ -240,13 +240,16 @@ describe(`${NodeConnection.name} test`, () => { keyManager: serverKeyManager, logger, }); - - serverQueue = new Queue({ logger }); + serverTaskManager = await TaskManager.createTaskManager({ + db: serverDb, + lazy: true, + logger, + }); serverNodeConnectionManager = new NodeConnectionManager({ keyManager: serverKeyManager, nodeGraph: serverNodeGraph, proxy: serverProxy, - queue: serverQueue, + taskManager: serverTaskManager, logger, }); serverNodeManager = new NodeManager({ @@ -255,10 +258,9 @@ describe(`${NodeConnection.name} test`, () => { keyManager: serverKeyManager, nodeGraph: serverNodeGraph, nodeConnectionManager: serverNodeConnectionManager, - queue: serverQueue, + taskManager: serverTaskManager, logger: logger, }); - await serverQueue.start(); await serverNodeManager.start(); await serverNodeConnectionManager.start({ nodeManager: serverNodeManager }); serverVaultManager = await VaultManager.createVaultManager({ @@ -372,7 +374,6 @@ describe(`${NodeConnection.name} test`, () => { await serverNodeGraph.destroy(); await serverNodeConnectionManager.stop(); await serverNodeManager.stop(); - await serverQueue.stop(); await serverNotificationsManager.stop(); await serverNotificationsManager.destroy(); await agentTestUtils.closeTestAgentServer(agentServer); diff --git a/tests/nodes/NodeConnectionManager.general.test.ts b/tests/nodes/NodeConnectionManager.general.test.ts index 28423dde9..fcaf3c211 100644 --- a/tests/nodes/NodeConnectionManager.general.test.ts +++ b/tests/nodes/NodeConnectionManager.general.test.ts @@ -1,13 +1,13 @@ import type { NodeAddress, NodeBucket, NodeId, SeedNodes } from '@/nodes/types'; import type { Host, Port } from '@/network/types'; import type NodeManager from '@/nodes/NodeManager'; +import type TaskManager from '@/tasks/TaskManager'; import fs from 'fs'; import path from 'path'; import os from 'os'; import { DB } from '@matrixai/db'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { IdInternal } from '@matrixai/id'; -import Queue from '@/nodes/Queue'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; import NodeGraph from '@/nodes/NodeGraph'; @@ -76,7 +76,6 @@ describe(`${NodeConnectionManager.name} general test`, () => { let db: DB; let proxy: Proxy; let nodeGraph: NodeGraph; - let queue: Queue; let remoteNode1: PolykeyAgent; let remoteNode2: PolykeyAgent; @@ -123,6 +122,10 @@ describe(`${NodeConnectionManager.name} general test`, () => { }; const dummyNodeManager = { setNode: jest.fn() } as unknown as NodeManager; + const dummyTaskManager: TaskManager = { + registerHandler: jest.fn(), + deregisterHandler: jest.fn(), + } as unknown as TaskManager; beforeAll(async () => { dataDir2 = await fs.promises.mkdtemp( @@ -197,10 +200,6 @@ describe(`${NodeConnectionManager.name} general test`, () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), - }); - await queue.start(); const tlsConfig = { keyPrivatePem: keyManager.getRootKeyPairPem().privateKey, certChainPem: keysUtils.certToPem(keyManager.getRootCert()), @@ -226,7 +225,6 @@ describe(`${NodeConnectionManager.name} general test`, () => { }); afterEach(async () => { - await queue.stop(); await nodeGraph.stop(); await nodeGraph.destroy(); await db.stop(); @@ -243,7 +241,7 @@ describe(`${NodeConnectionManager.name} general test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager: dummyTaskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); @@ -276,7 +274,7 @@ describe(`${NodeConnectionManager.name} general test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager: dummyTaskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); @@ -325,7 +323,7 @@ describe(`${NodeConnectionManager.name} general test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager: dummyTaskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); @@ -391,7 +389,7 @@ describe(`${NodeConnectionManager.name} general test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager: dummyTaskManager, logger: logger.getChild('NodeConnectionManager'), }); @@ -463,7 +461,7 @@ describe(`${NodeConnectionManager.name} general test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager: dummyTaskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); @@ -501,7 +499,7 @@ describe(`${NodeConnectionManager.name} general test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager: dummyTaskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); diff --git a/tests/nodes/NodeConnectionManager.lifecycle.test.ts b/tests/nodes/NodeConnectionManager.lifecycle.test.ts index c9ff18cff..a904c7ef3 100644 --- a/tests/nodes/NodeConnectionManager.lifecycle.test.ts +++ b/tests/nodes/NodeConnectionManager.lifecycle.test.ts @@ -8,7 +8,7 @@ import { DB } from '@matrixai/db'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { withF } from '@matrixai/resources'; import { IdInternal } from '@matrixai/id'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; import NodeGraph from '@/nodes/NodeGraph'; @@ -77,7 +77,7 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { let proxy: Proxy; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let remoteNode1: PolykeyAgent; let remoteNode2: PolykeyAgent; @@ -155,10 +155,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + lazy: true, + logger, }); - await queue.start(); const tlsConfig = { keyPrivatePem: keyManager.getRootKeyPairPem().privateKey, certChainPem: keysUtils.certToPem(keyManager.getRootCert()), @@ -184,7 +185,7 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { }); afterEach(async () => { - await queue.stop(); + await taskManager.stop(); await nodeGraph.stop(); await nodeGraph.destroy(); await db.stop(); @@ -203,10 +204,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // @ts-ignore: kidnap connections const connections = nodeConnectionManager.connections; // @ts-ignore: kidnap connectionLocks @@ -229,10 +231,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // @ts-ignore: kidnap connections const connections = nodeConnectionManager.connections; // @ts-ignore: kidnap connectionLocks @@ -264,10 +267,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // @ts-ignore: kidnap connections const connections = nodeConnectionManager.connections; // @ts-ignore: kidnap connectionLocks @@ -293,11 +297,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); - + await taskManager.startProcessing(); // @ts-ignore: kidnap connections const connections = nodeConnectionManager.connections; // @ts-ignore: kidnap connectionLocks @@ -346,11 +350,12 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 500, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // Add the dummy node await nodeGraph.setNode(dummyNodeId, { host: '125.0.0.1' as Host, @@ -388,10 +393,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // @ts-ignore accessing protected NodeConnectionMap const connections = nodeConnectionManager.connections; expect(connections.size).toBe(0); @@ -415,10 +421,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // @ts-ignore accessing protected NodeConnectionMap const connections = nodeConnectionManager.connections; // @ts-ignore: kidnap connectionLocks @@ -449,10 +456,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // @ts-ignore: kidnap connections const connections = nodeConnectionManager.connections; // @ts-ignore: kidnap connectionLocks @@ -483,10 +491,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); // Do testing // set up connections await nodeConnectionManager.withConnF(remoteNodeId1, nop); @@ -526,10 +535,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); await nodeConnectionManager.pingNode( remoteNodeId1, remoteNode1.proxy.getProxyHost(), @@ -547,11 +557,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); - + await taskManager.startProcessing(); // Pinging node expect( await nodeConnectionManager.pingNode( @@ -573,11 +583,11 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { keyManager, nodeGraph, proxy, - queue, + taskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); - + await taskManager.startProcessing(); expect( await nodeConnectionManager.pingNode( remoteNodeId1, diff --git a/tests/nodes/NodeConnectionManager.seednodes.test.ts b/tests/nodes/NodeConnectionManager.seednodes.test.ts index 4c8d62440..aaa72c9cf 100644 --- a/tests/nodes/NodeConnectionManager.seednodes.test.ts +++ b/tests/nodes/NodeConnectionManager.seednodes.test.ts @@ -16,13 +16,14 @@ import Proxy from '@/network/Proxy'; import * as nodesUtils from '@/nodes/utils'; import * as keysUtils from '@/keys/utils'; import * as grpcUtils from '@/grpc/utils'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; +import { sleep } from '@/utils/index'; import { globalRootKeyPems } from '../fixtures/globalRootKeyPems'; describe(`${NodeConnectionManager.name} seed nodes test`, () => { const logger = new Logger( `${NodeConnectionManager.name} test`, - LogLevel.WARN, + LogLevel.DEBUG, [new StreamHandler()], ); grpcUtils.setLogger(logger.getChild('grpc')); @@ -76,6 +77,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { let remoteNodeId1: NodeId; let remoteNodeId2: NodeId; + let taskManager: TaskManager; const dummyNodeManager = { setNode: jest.fn(), refreshBucketQueueAdd: jest.fn(), @@ -150,6 +152,11 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { }, }, }); + taskManager = await TaskManager.createTaskManager({ + db, + lazy: true, + logger: logger.getChild('taskManager'), + }); nodeGraph = await NodeGraph.createNodeGraph({ db, keyManager, @@ -187,6 +194,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { await keyManager.stop(); await keyManager.destroy(); await proxy.stop(); + await taskManager.stop(); }); // Seed nodes @@ -198,9 +206,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { keyManager, nodeGraph, proxy, - queue: new Queue({ - logger: logger.getChild('queue'), - }), + taskManager, seedNodes: dummySeedNodes, logger: logger, }); @@ -210,7 +216,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { logger, nodeConnectionManager, nodeGraph, - queue: {} as Queue, + taskManager, sigchain: {} as Sigchain, }); await nodeManager.start(); @@ -235,9 +241,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { keyManager, nodeGraph, proxy, - queue: new Queue({ - logger: logger.getChild('queue'), - }), + taskManager, seedNodes: dummySeedNodes, logger: logger, }); @@ -255,7 +259,6 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { test('should synchronise nodeGraph', async () => { let nodeConnectionManager: NodeConnectionManager | undefined; let nodeManager: NodeManager | undefined; - let queue: Queue | undefined; const mockedRefreshBucket = jest.spyOn( NodeManager.prototype, 'refreshBucket', @@ -276,12 +279,11 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { host: remoteNode2.proxy.getProxyHost(), port: remoteNode2.proxy.getProxyPort(), }; - queue = new Queue({ logger }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, seedNodes, logger: logger, }); @@ -291,10 +293,9 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { logger, nodeConnectionManager, nodeGraph, - queue, + taskManager, sigchain: {} as Sigchain, }); - await queue.start(); await nodeManager.start(); await remoteNode1.nodeGraph.setNode(nodeId1, { host: serverHost, @@ -305,6 +306,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { port: serverPort, }); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); await nodeConnectionManager.syncNodeGraph(); expect(await nodeGraph.getNode(nodeId1)).toBeDefined(); expect(await nodeGraph.getNode(nodeId2)).toBeDefined(); @@ -314,13 +316,11 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { mockedPingNode.mockRestore(); await nodeManager?.stop(); await nodeConnectionManager?.stop(); - await queue?.stop(); } }); test('should call refreshBucket when syncing nodeGraph', async () => { let nodeConnectionManager: NodeConnectionManager | undefined; let nodeManager: NodeManager | undefined; - let queue: Queue | undefined; const mockedRefreshBucket = jest.spyOn( NodeManager.prototype, 'refreshBucket', @@ -341,12 +341,11 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { host: remoteNode2.proxy.getProxyHost(), port: remoteNode2.proxy.getProxyPort(), }; - queue = new Queue({ logger }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, seedNodes, logger: logger, }); @@ -357,9 +356,8 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { nodeConnectionManager, nodeGraph, sigchain: {} as Sigchain, - queue, + taskManager, }); - await queue.start(); await nodeManager.start(); await remoteNode1.nodeGraph.setNode(nodeId1, { host: serverHost, @@ -370,21 +368,20 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { port: serverPort, }); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); await nodeConnectionManager.syncNodeGraph(); - await nodeManager.refreshBucketQueueDrained(); + await sleep(1000); expect(mockedRefreshBucket).toHaveBeenCalled(); } finally { mockedRefreshBucket.mockRestore(); mockedPingNode.mockRestore(); await nodeManager?.stop(); await nodeConnectionManager?.stop(); - await queue?.stop(); } }); test('should handle an offline seed node when synchronising nodeGraph', async () => { let nodeConnectionManager: NodeConnectionManager | undefined; let nodeManager: NodeManager | undefined; - let queue: Queue | undefined; const mockedRefreshBucket = jest.spyOn( NodeManager.prototype, 'refreshBucket', @@ -418,12 +415,11 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { host: serverHost, port: serverPort, }); - queue = new Queue({ logger }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, seedNodes, connConnectTime: 500, logger: logger, @@ -435,11 +431,11 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { nodeConnectionManager, nodeGraph, sigchain: {} as Sigchain, - queue, + taskManager, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); // This should complete without error await nodeConnectionManager.syncNodeGraph(); // Information on remotes are found @@ -450,7 +446,6 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { mockedPingNode.mockRestore(); await nodeConnectionManager?.stop(); await nodeManager?.stop(); - await queue?.stop(); } }); test( @@ -507,10 +502,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { logger, }); - await node1.queue.drained(); - await node1.nodeManager.refreshBucketQueueDrained(); - await node2.queue.drained(); - await node2.nodeManager.refreshBucketQueueDrained(); + await sleep(1000); const getAllNodes = async (node: PolykeyAgent) => { const nodes: Array = []; diff --git a/tests/nodes/NodeConnectionManager.termination.test.ts b/tests/nodes/NodeConnectionManager.termination.test.ts index 5436a9fbb..87b237d62 100644 --- a/tests/nodes/NodeConnectionManager.termination.test.ts +++ b/tests/nodes/NodeConnectionManager.termination.test.ts @@ -2,7 +2,7 @@ import type { AddressInfo } from 'net'; import type { NodeId, NodeIdString, SeedNodes } from '@/nodes/types'; import type { Host, Port, TLSConfig } from '@/network/types'; import type NodeManager from '@/nodes/NodeManager'; -import type Queue from '@/nodes/Queue'; +import type TaskManager from 'tasks/TaskManager'; import net from 'net'; import fs from 'fs'; import path from 'path'; @@ -84,6 +84,10 @@ describe(`${NodeConnectionManager.name} termination test`, () => { let tlsConfig2: TLSConfig; const dummyNodeManager = { setNode: jest.fn() } as unknown as NodeManager; + const dummyTaskManager: TaskManager = { + registerHandler: jest.fn(), + deregisterHandler: jest.fn(), + } as unknown as TaskManager; beforeEach(async () => { dataDir = await fs.promises.mkdtemp( @@ -240,7 +244,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -281,7 +285,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -325,7 +329,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -372,7 +376,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy: defaultProxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -433,7 +437,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy: defaultProxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -516,7 +520,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy: defaultProxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -592,7 +596,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy: defaultProxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -673,7 +677,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy: defaultProxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); @@ -754,7 +758,7 @@ describe(`${NodeConnectionManager.name} termination test`, () => { keyManager, nodeGraph, proxy: defaultProxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: logger, connConnectTime: 2000, }); diff --git a/tests/nodes/NodeConnectionManager.timeout.test.ts b/tests/nodes/NodeConnectionManager.timeout.test.ts index d356f1f55..d06d2a019 100644 --- a/tests/nodes/NodeConnectionManager.timeout.test.ts +++ b/tests/nodes/NodeConnectionManager.timeout.test.ts @@ -1,7 +1,7 @@ import type { NodeId, NodeIdString, SeedNodes } from '@/nodes/types'; import type { Host, Port } from '@/network/types'; import type NodeManager from 'nodes/NodeManager'; -import type Queue from '@/nodes/Queue'; +import type TaskManager from '@/tasks/TaskManager'; import fs from 'fs'; import path from 'path'; import os from 'os'; @@ -77,6 +77,10 @@ describe(`${NodeConnectionManager.name} timeout test`, () => { let remoteNodeId2: NodeId; const dummyNodeManager = { setNode: jest.fn() } as unknown as NodeManager; + const dummyTaskManager: TaskManager = { + registerHandler: jest.fn(), + deregisterHandler: jest.fn(), + } as unknown as TaskManager; beforeAll(async () => { dataDir2 = await fs.promises.mkdtemp( @@ -188,7 +192,7 @@ describe(`${NodeConnectionManager.name} timeout test`, () => { keyManager, nodeGraph, proxy, - queue: {} as Queue, + taskManager: dummyTaskManager, connTimeoutTime: 500, logger: nodeConnectionManagerLogger, }); @@ -226,7 +230,7 @@ describe(`${NodeConnectionManager.name} timeout test`, () => { keyManager, nodeGraph, proxy, - queue: {} as Queue, + taskManager: dummyTaskManager, connTimeoutTime: 1000, logger: nodeConnectionManagerLogger, }); @@ -280,7 +284,7 @@ describe(`${NodeConnectionManager.name} timeout test`, () => { keyManager, nodeGraph, proxy, - queue: {} as Queue, + taskManager: dummyTaskManager, logger: nodeConnectionManagerLogger, }); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index 3c0650742..269c2b019 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -1,6 +1,7 @@ import type { CertificatePem, KeyPairPem, PublicKeyPem } from '@/keys/types'; import type { Host, Port } from '@/network/types'; import type { NodeId, NodeAddress } from '@/nodes/types'; +import type { Task } from '@/tasks/types'; import os from 'os'; import path from 'path'; import fs from 'fs'; @@ -20,12 +21,9 @@ import * as claimsUtils from '@/claims/utils'; import { never, promise, promisify, sleep } from '@/utils'; import * as nodesUtils from '@/nodes/utils'; import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb'; -import * as nodesErrors from '@/nodes/errors'; import * as nodesTestUtils from './utils'; import { generateNodeIdForBucket } from './utils'; import { globalRootKeyPems } from '../fixtures/globalRootKeyPems'; -import { before } from 'cheerio/lib/api/manipulation'; -import { Task } from '@/tasks/types'; describe(`${NodeManager.name} test`, () => { const password = 'password'; @@ -933,7 +931,10 @@ describe(`${NodeManager.name} test`, () => { // Getting starting value const bucketIndex = 100; let refreshBucketTask: Task | undefined; - for await (const task of taskManager.getTasks('asc', true, ['refreshBucket', `${bucketIndex}`])){ + for await (const task of taskManager.getTasks('asc', true, [ + 'refreshBucket', + `${bucketIndex}`, + ])) { refreshBucketTask = task; } if (refreshBucketTask == null) never(); @@ -945,11 +946,16 @@ describe(`${NodeManager.name} test`, () => { await nodeManager.setNode(nodeId, {} as NodeAddress); // Deadline should be updated let refreshBucketTaskUpdated: Task | undefined; - for await (const task of taskManager.getTasks('asc', true, ['refreshBucket', `${bucketIndex}`])){ + for await (const task of taskManager.getTasks('asc', true, [ + 'refreshBucket', + `${bucketIndex}`, + ])) { refreshBucketTaskUpdated = task; } if (refreshBucketTaskUpdated == null) never(); - expect(refreshBucketTaskUpdated.delay).not.toEqual(refreshBucketTask.delay); + expect(refreshBucketTaskUpdated.delay).not.toEqual( + refreshBucketTask.delay, + ); } finally { mockRefreshBucket.mockRestore(); await nodeManager.stop(); From eb1b5575637bbe420fc19a2de96c4f8372438df3 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 13 Sep 2022 17:13:27 +1000 Subject: [PATCH 04/40] feat: adding cancellability to `NodeManager` handlers --- src/nodes/NodeConnectionManager.ts | 20 +++++++++++++-- src/nodes/NodeManager.ts | 40 ++++++++++++++---------------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 2068e6aaf..78e0449c3 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -507,6 +507,8 @@ class NodeConnectionManager { nextNodeId, nextNodeAddress.address.host, nextNodeAddress.address.port, + undefined, + { signal }, ) ) { await this.nodeManager!.setNode(nextNodeId, nextNodeAddress.address); @@ -523,7 +525,7 @@ class NodeConnectionManager { // Check to see if any of these are the target node. At the same time, add // them to the shortlist for (const [nodeId, nodeData] of foundClosest) { - if (signal?.aborted) throw new nodesErrors.ErrorNodeAborted(); + signal?.throwIfAborted(); // Ignore any nodes that have been contacted or our own node if (contacted[nodeId] || localNodeId.equals(nodeId)) { continue; @@ -534,6 +536,8 @@ class NodeConnectionManager { nodeId, nodeData.address.host, nodeData.address.port, + undefined, + { signal }, )) ) { await this.nodeManager!.setNode(nodeId, nodeData.address); @@ -773,6 +777,7 @@ class NodeConnectionManager { * @param host - Host of the target node * @param port - Port of the target node * @param timer Connection timeout timer + * @param options */ @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) public async pingNode( @@ -780,7 +785,9 @@ class NodeConnectionManager { host: Host | Hostname, port: Port, timer?: Timer, + options: { signal?: AbortSignal } = {}, ): Promise { + const { signal } = { ...options }; host = await networkUtils.resolveHost(host); // If we can create a connection then we have punched though the NAT, // authenticated and confirmed the nodeId matches @@ -791,6 +798,7 @@ class NodeConnectionManager { const signature = await this.keyManager.signWithRootKeyPair( Buffer.from(proxyAddress), ); + signal?.throwIfAborted(); // FIXME: this needs to handle aborting const holePunchPromises = Array.from(this.getSeedNodes(), (seedNodeId) => { return this.sendHolePunchMessage( @@ -808,8 +816,16 @@ class NodeConnectionManager { timer, ); + const abortPromise = new Promise((_resolve, reject) => { + signal?.addEventListener('abort', () => reject(signal.reason)); + }); + try { - await Promise.any([forwardPunchPromise, ...holePunchPromises]); + await Promise.any([ + forwardPunchPromise, + ...holePunchPromises, + abortPromise, + ]); } catch (e) { return false; } diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 6aa7a1ce3..5ccee1038 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -68,7 +68,9 @@ class NodeManager { timeout: number, ) => { const nodeId: NodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; - await this.setNode(nodeId, nodeAddress, true, false, timeout); + await this.setNode(nodeId, nodeAddress, true, false, timeout, undefined, { + signal: context.signal, + }); }; constructor({ @@ -134,11 +136,12 @@ class NodeManager { nodeId: NodeId, address?: NodeAddress, timer?: Timer, + options: { signal?: AbortSignal } = {}, ): Promise { // We need to attempt a connection using the proxies // For now we will just do a forward connect + relay message const targetAddress = - address ?? (await this.nodeConnectionManager.findNode(nodeId)); + address ?? (await this.nodeConnectionManager.findNode(nodeId, options)); if (targetAddress == null) { throw new nodesErrors.ErrorNodeGraphNodeIdNotFound(); } @@ -441,6 +444,7 @@ class NodeManager { force: boolean = false, timeout?: number, tran?: DBTransaction, + options: { signal?: AbortSignal } = {}, ): Promise { // We don't want to add our own node if (nodeId.equals(this.keyManager.getNodeId())) { @@ -515,6 +519,7 @@ class NodeManager { nodeId, nodeAddress, timeout, + options, ); } else { this.logger.debug( @@ -542,27 +547,20 @@ class NodeManager { nodeId: NodeId, nodeAddress: NodeAddress, timeout?: number, + options: { signal?: AbortSignal } = {}, ) { + const { signal } = { ...options }; const oldestNodeIds = await this.nodeGraph.getOldestNode(bucketIndex, 3); - // We want to concurrently ping the nodes - // Fixme, remove concurrency? we'd want to stick to 1 active connection per - // background task - const pingPromises = oldestNodeIds.map((nodeId) => { - const doPing = async (): Promise<{ - nodeId: NodeId; - success: boolean; - }> => { - // This needs to return nodeId and ping result - const data = await this.nodeGraph.getNode(nodeId); - if (data == null) return { nodeId, success: false }; - const timer = timeout != null ? timerStart(timeout) : undefined; - const result = await this.pingNode(nodeId, nodeAddress, timer); - return { nodeId, success: result }; - }; - return doPing(); - }); - const pingResults = await Promise.all(pingPromises); - for (const { nodeId, success } of pingResults) { + for (const nodeId of oldestNodeIds) { + signal?.throwIfAborted(); + // This needs to return nodeId and ping result + const data = await this.nodeGraph.getNode(nodeId); + if (data == null) return { nodeId, success: false }; + const timer = timeout != null ? timerStart(timeout) : undefined; + const success = await this.pingNode(nodeId, nodeAddress, timer, { + signal, + }); + if (success) { // Ping succeeded, update the node this.logger.debug( From 6e4322bcd1309e9d2ed41329c14f5dfdfc929c98 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 14 Sep 2022 13:55:14 +1000 Subject: [PATCH 05/40] fix: small bug with `pingNode` --- src/nodes/NodeConnectionManager.ts | 5 ++--- src/nodes/NodeManager.ts | 1 + tests/nodes/NodeManager.test.ts | 23 ++++++++++++++++------- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 78e0449c3..94debf3ce 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -821,9 +821,8 @@ class NodeConnectionManager { }); try { - await Promise.any([ - forwardPunchPromise, - ...holePunchPromises, + await Promise.race([ + Promise.any([forwardPunchPromise, ...holePunchPromises]), abortPromise, ]); } catch (e) { diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 5ccee1038..f33d174b4 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -131,6 +131,7 @@ class NodeManager { * @param nodeId - NodeId of the node we're pinging * @param address - Optional Host and Port we want to ping * @param timer Connection timeout timer + * @param options */ public async pingNode( nodeId: NodeId, diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index 269c2b019..8e81081a2 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -18,7 +18,7 @@ import NodeManager from '@/nodes/NodeManager'; import Proxy from '@/network/Proxy'; import Sigchain from '@/sigchain/Sigchain'; import * as claimsUtils from '@/claims/utils'; -import { never, promise, promisify, sleep } from '@/utils'; +import { never, promise, promisify, sleep, timerStart } from '@/utils'; import * as nodesUtils from '@/nodes/utils'; import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb'; import * as nodesTestUtils from './utils'; @@ -184,7 +184,11 @@ describe(`${NodeManager.name} test`, () => { await server.stop(); // Check if active // Case 1: cannot establish new connection, so offline - const active1 = await nodeManager.pingNode(serverNodeId); + const active1 = await nodeManager.pingNode( + serverNodeId, + undefined, + timerStart(1000), + ); expect(active1).toBe(false); // Bring server node online await server.start({ @@ -201,17 +205,22 @@ describe(`${NodeManager.name} test`, () => { await nodeGraph.setNode(serverNodeId, serverNodeAddress); // Check if active // Case 2: can establish new connection, so online - const active2 = await nodeManager.pingNode(serverNodeId); + const active2 = await nodeManager.pingNode( + serverNodeId, + undefined, + timerStart(1000), + ); expect(active2).toBe(true); // Turn server node offline again await server.stop(); await server.destroy(); - // Give time for the ping buffers to send and wait for timeout on - // existing connection - await sleep(30000); // FIXME: remove this sleep // Check if active // Case 3: pre-existing connection no longer active, so offline - const active3 = await nodeManager.pingNode(serverNodeId); + const active3 = await nodeManager.pingNode( + serverNodeId, + undefined, + timerStart(1000), + ); expect(active3).toBe(false); } finally { // Clean up From 27316f94f035dc2d8d295ad15115a7934aa996e8 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 14 Sep 2022 17:07:13 +1000 Subject: [PATCH 06/40] fix: bugs with `nodeConnectionManager.syncNodeGraph` --- src/nodes/NodeConnectionManager.ts | 6 +++--- src/nodes/NodeGraph.ts | 9 +++++++++ src/nodes/NodeManager.ts | 7 +++++-- tests/nodes/NodeConnectionManager.seednodes.test.ts | 7 ++++--- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 94debf3ce..353d3ba2d 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -525,7 +525,7 @@ class NodeConnectionManager { // Check to see if any of these are the target node. At the same time, add // them to the shortlist for (const [nodeId, nodeData] of foundClosest) { - signal?.throwIfAborted(); + if (signal?.aborted) throw signal.reason; // Ignore any nodes that have been contacted or our own node if (contacted[nodeId] || localNodeId.equals(nodeId)) { continue; @@ -670,7 +670,7 @@ class NodeConnectionManager { // Skip our nodeId if it exists closestNodeInfo = closestNodes.pop()!; } - let index = 0; + let index = this.nodeGraph.nodeIdBits; if (closestNodeInfo != null) { const [closestNode] = closestNodeInfo; const [bucketIndex] = this.nodeGraph.bucketIndex(closestNode); @@ -798,7 +798,7 @@ class NodeConnectionManager { const signature = await this.keyManager.signWithRootKeyPair( Buffer.from(proxyAddress), ); - signal?.throwIfAborted(); + if (signal?.aborted) throw signal.reason; // FIXME: this needs to handle aborting const holePunchPromises = Array.from(this.getSeedNodes(), (seedNodeId) => { return this.sendHolePunchMessage( diff --git a/src/nodes/NodeGraph.ts b/src/nodes/NodeGraph.ts index fda9caba1..5f65db114 100644 --- a/src/nodes/NodeGraph.ts +++ b/src/nodes/NodeGraph.ts @@ -151,6 +151,15 @@ class NodeGraph { return space; } + @ready(new nodesErrors.ErrorNodeGraphNotRunning()) + public async lockBucket(bucketIndex: number, tran: DBTransaction) { + const keyPath = [ + ...this.nodeGraphMetaDbPath, + nodesUtils.bucketKey(bucketIndex), + ]; + return await tran.lock(keyPath.join('')); + } + @ready(new nodesErrors.ErrorNodeGraphNotRunning()) public async getNode( nodeId: NodeId, diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index f33d174b4..4b209b45b 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -436,6 +436,7 @@ class NodeManager { * This will drop the oldest node in favor of the new. * @param timeout Connection timeout * @param tran + * @param options */ @ready(new nodesErrors.ErrorNodeManagerNotRunning()) public async setNode( @@ -467,9 +468,11 @@ class NodeManager { // We need to ping the oldest node. If the ping succeeds we need to update // the lastUpdated of the oldest node and drop the new one. If the ping // fails we delete the old node and add in the new one. + const [bucketIndex] = this.nodeGraph.bucketIndex(nodeId); + // To avoid conflict we want to lock on the bucket index + await this.nodeGraph.lockBucket(bucketIndex, tran); const nodeData = await this.nodeGraph.getNode(nodeId, tran); // If this is a new entry, check the bucket limit - const [bucketIndex] = this.nodeGraph.bucketIndex(nodeId); const count = await this.nodeGraph.getBucketMetaProp( bucketIndex, 'count', @@ -553,7 +556,7 @@ class NodeManager { const { signal } = { ...options }; const oldestNodeIds = await this.nodeGraph.getOldestNode(bucketIndex, 3); for (const nodeId of oldestNodeIds) { - signal?.throwIfAborted(); + if (signal?.aborted) throw signal.reason; // This needs to return nodeId and ping result const data = await this.nodeGraph.getNode(nodeId); if (data == null) return { nodeId, success: false }; diff --git a/tests/nodes/NodeConnectionManager.seednodes.test.ts b/tests/nodes/NodeConnectionManager.seednodes.test.ts index aaa72c9cf..24f499cb1 100644 --- a/tests/nodes/NodeConnectionManager.seednodes.test.ts +++ b/tests/nodes/NodeConnectionManager.seednodes.test.ts @@ -23,7 +23,7 @@ import { globalRootKeyPems } from '../fixtures/globalRootKeyPems'; describe(`${NodeConnectionManager.name} seed nodes test`, () => { const logger = new Logger( `${NodeConnectionManager.name} test`, - LogLevel.DEBUG, + LogLevel.WARN, [new StreamHandler()], ); grpcUtils.setLogger(logger.getChild('grpc')); @@ -437,7 +437,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { await nodeConnectionManager.start({ nodeManager }); await taskManager.startProcessing(); // This should complete without error - await nodeConnectionManager.syncNodeGraph(); + await nodeConnectionManager.syncNodeGraph(true); // Information on remotes are found expect(await nodeGraph.getNode(nodeId1)).toBeDefined(); expect(await nodeGraph.getNode(nodeId2)).toBeDefined(); @@ -502,7 +502,8 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { logger, }); - await sleep(1000); + await node1.nodeConnectionManager.syncNodeGraph(true); + await node2.nodeConnectionManager.syncNodeGraph(true); const getAllNodes = async (node: PolykeyAgent) => { const nodes: Array = []; From f1ab40b0e71bcb1ad88cd7ee57172e9301fbd72d Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 14 Sep 2022 18:22:42 +1000 Subject: [PATCH 07/40] tests: cleaning up dependencies in tests --- src/bootstrap/utils.ts | 13 +++++++--- tests/agent/GRPCClientAgent.test.ts | 19 ++++++++------ tests/agent/service/notificationsSend.test.ts | 19 ++++++++------ .../gestaltsDiscoveryByIdentity.test.ts | 19 ++++++++------ .../service/gestaltsDiscoveryByNode.test.ts | 19 ++++++++------ .../gestaltsGestaltTrustByIdentity.test.ts | 19 ++++++++------ .../gestaltsGestaltTrustByNode.test.ts | 19 ++++++++------ tests/client/service/identitiesClaim.test.ts | 17 +++++++------ tests/client/service/nodesAdd.test.ts | 19 ++++++++------ tests/client/service/nodesClaim.test.ts | 19 ++++++++------ tests/client/service/nodesFind.test.ts | 17 +++++++------ tests/client/service/nodesPing.test.ts | 19 ++++++++------ .../client/service/notificationsClear.test.ts | 19 ++++++++------ .../client/service/notificationsRead.test.ts | 19 ++++++++------ .../client/service/notificationsSend.test.ts | 19 ++++++++------ tests/discovery/Discovery.test.ts | 19 ++++++++------ .../NotificationsManager.test.ts | 19 ++++++++------ tests/vaults/VaultManager.test.ts | 25 +++++++++++++++---- 18 files changed, 205 insertions(+), 133 deletions(-) diff --git a/src/bootstrap/utils.ts b/src/bootstrap/utils.ts index 9eece1244..72c06de83 100644 --- a/src/bootstrap/utils.ts +++ b/src/bootstrap/utils.ts @@ -4,7 +4,7 @@ import path from 'path'; import Logger from '@matrixai/logger'; import { DB } from '@matrixai/db'; import * as bootstrapErrors from './errors'; -import Queue from '../nodes/Queue'; +import TaskManager from '../tasks/TaskManager'; import { IdentitiesManager } from '../identities'; import { SessionManager } from '../sessions'; import { Status } from '../status'; @@ -143,12 +143,16 @@ async function bootstrapState({ keyManager, logger: logger.getChild(NodeGraph.name), }); - const queue = new Queue({ logger }); + const taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, + }); const nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, logger: logger.getChild(NodeConnectionManager.name), }); const nodeManager = new NodeManager({ @@ -157,7 +161,7 @@ async function bootstrapState({ nodeGraph, nodeConnectionManager, sigchain, - queue, + taskManager, logger: logger.getChild(NodeManager.name), }); const notificationsManager = @@ -196,6 +200,7 @@ async function bootstrapState({ await acl.stop(); await sigchain.stop(); await identitiesManager.stop(); + await taskManager.stop(); await db.stop(); await keyManager.stop(); await schema.stop(); diff --git a/tests/agent/GRPCClientAgent.test.ts b/tests/agent/GRPCClientAgent.test.ts index c7f710295..6719ac6ce 100644 --- a/tests/agent/GRPCClientAgent.test.ts +++ b/tests/agent/GRPCClientAgent.test.ts @@ -6,7 +6,7 @@ import path from 'path'; import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import GestaltGraph from '@/gestalts/GestaltGraph'; import ACL from '@/acl/ACL'; import KeyManager from '@/keys/KeyManager'; @@ -41,7 +41,7 @@ describe(GRPCClientAgent.name, () => { let keyManager: KeyManager; let vaultManager: VaultManager; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let sigchain: Sigchain; @@ -104,12 +104,16 @@ describe(GRPCClientAgent.name, () => { keyManager, logger, }); - queue = new Queue({ logger }); + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, + }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, logger, }); nodeManager = new NodeManager({ @@ -118,12 +122,12 @@ describe(GRPCClientAgent.name, () => { keyManager: keyManager, nodeGraph: nodeGraph, nodeConnectionManager: nodeConnectionManager, - queue, + taskManager, logger: logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); notificationsManager = await NotificationsManager.createNotificationsManager({ acl: acl, @@ -169,6 +173,7 @@ describe(GRPCClientAgent.name, () => { }); }, globalThis.defaultTimeout); afterEach(async () => { + await taskManager.stopProcessing(); await testAgentUtils.closeTestAgentClient(client); await testAgentUtils.closeTestAgentServer(server); await vaultManager.stop(); @@ -176,13 +181,13 @@ describe(GRPCClientAgent.name, () => { await sigchain.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await nodeGraph.stop(); await gestaltGraph.stop(); await acl.stop(); await proxy.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/agent/service/notificationsSend.test.ts b/tests/agent/service/notificationsSend.test.ts index 506941396..21d3d1aeb 100644 --- a/tests/agent/service/notificationsSend.test.ts +++ b/tests/agent/service/notificationsSend.test.ts @@ -8,7 +8,7 @@ import { createPrivateKey, createPublicKey } from 'crypto'; import { exportJWK, SignJWT } from 'jose'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import GRPCServer from '@/grpc/GRPCServer'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; @@ -39,7 +39,7 @@ describe('notificationsSend', () => { let senderKeyManager: KeyManager; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let notificationsManager: NotificationsManager; @@ -102,14 +102,16 @@ describe('notificationsSend', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -120,12 +122,12 @@ describe('notificationsSend', () => { nodeGraph, nodeConnectionManager, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); notificationsManager = await NotificationsManager.createNotificationsManager({ acl, @@ -156,11 +158,11 @@ describe('notificationsSend', () => { }); }, globalThis.defaultTimeout); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); await nodeConnectionManager.stop(); - await queue.stop(); await nodeManager.stop(); await sigchain.stop(); await sigchain.stop(); @@ -169,6 +171,7 @@ describe('notificationsSend', () => { await db.stop(); await senderKeyManager.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/gestaltsDiscoveryByIdentity.test.ts b/tests/client/service/gestaltsDiscoveryByIdentity.test.ts index 0b9dd8c44..38176072d 100644 --- a/tests/client/service/gestaltsDiscoveryByIdentity.test.ts +++ b/tests/client/service/gestaltsDiscoveryByIdentity.test.ts @@ -6,7 +6,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import GestaltGraph from '@/gestalts/GestaltGraph'; import ACL from '@/acl/ACL'; import KeyManager from '@/keys/KeyManager'; @@ -45,7 +45,7 @@ describe('gestaltsDiscoveryByIdentity', () => { let gestaltGraph: GestaltGraph; let identitiesManager: IdentitiesManager; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let sigchain: Sigchain; @@ -113,14 +113,16 @@ describe('gestaltsDiscoveryByIdentity', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -131,12 +133,12 @@ describe('gestaltsDiscoveryByIdentity', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); discovery = await Discovery.createDiscovery({ db, keyManager, @@ -167,13 +169,13 @@ describe('gestaltsDiscoveryByIdentity', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); await nodeGraph.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await sigchain.stop(); await proxy.stop(); await identitiesManager.stop(); @@ -181,6 +183,7 @@ describe('gestaltsDiscoveryByIdentity', () => { await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/gestaltsDiscoveryByNode.test.ts b/tests/client/service/gestaltsDiscoveryByNode.test.ts index d0d77b431..d88e5d475 100644 --- a/tests/client/service/gestaltsDiscoveryByNode.test.ts +++ b/tests/client/service/gestaltsDiscoveryByNode.test.ts @@ -6,7 +6,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import GestaltGraph from '@/gestalts/GestaltGraph'; import ACL from '@/acl/ACL'; import KeyManager from '@/keys/KeyManager'; @@ -46,7 +46,7 @@ describe('gestaltsDiscoveryByNode', () => { let gestaltGraph: GestaltGraph; let identitiesManager: IdentitiesManager; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let sigchain: Sigchain; @@ -114,14 +114,16 @@ describe('gestaltsDiscoveryByNode', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -132,12 +134,12 @@ describe('gestaltsDiscoveryByNode', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.start(); discovery = await Discovery.createDiscovery({ db, keyManager, @@ -168,13 +170,13 @@ describe('gestaltsDiscoveryByNode', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); await nodeGraph.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await sigchain.stop(); await proxy.stop(); await identitiesManager.stop(); @@ -182,6 +184,7 @@ describe('gestaltsDiscoveryByNode', () => { await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts b/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts index 052295ed7..8a6a3d03a 100644 --- a/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts +++ b/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts @@ -9,7 +9,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; import Discovery from '@/discovery/Discovery'; @@ -58,7 +58,7 @@ describe('gestaltsGestaltTrustByIdentity', () => { let discovery: Discovery; let gestaltGraph: GestaltGraph; let identitiesManager: IdentitiesManager; - let queue: Queue; + let taskManager: TaskManager; let nodeManager: NodeManager; let nodeConnectionManager: NodeConnectionManager; let nodeGraph: NodeGraph; @@ -173,14 +173,16 @@ describe('gestaltsGestaltTrustByIdentity', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -191,12 +193,12 @@ describe('gestaltsGestaltTrustByIdentity', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); await nodeManager.setNode(nodesUtils.decodeNodeId(nodeId)!, { host: node.proxy.getProxyHost(), port: node.proxy.getProxyPort(), @@ -233,12 +235,12 @@ describe('gestaltsGestaltTrustByIdentity', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await nodeGraph.stop(); await proxy.stop(); await sigchain.stop(); @@ -247,6 +249,7 @@ describe('gestaltsGestaltTrustByIdentity', () => { await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/gestaltsGestaltTrustByNode.test.ts b/tests/client/service/gestaltsGestaltTrustByNode.test.ts index b32462ff5..fd6a2f8d1 100644 --- a/tests/client/service/gestaltsGestaltTrustByNode.test.ts +++ b/tests/client/service/gestaltsGestaltTrustByNode.test.ts @@ -10,7 +10,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; import Discovery from '@/discovery/Discovery'; @@ -103,7 +103,7 @@ describe('gestaltsGestaltTrustByNode', () => { let discovery: Discovery; let gestaltGraph: GestaltGraph; let identitiesManager: IdentitiesManager; - let queue: Queue; + let taskManager: TaskManager; let nodeManager: NodeManager; let nodeConnectionManager: NodeConnectionManager; let nodeGraph: NodeGraph; @@ -181,14 +181,16 @@ describe('gestaltsGestaltTrustByNode', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -199,12 +201,12 @@ describe('gestaltsGestaltTrustByNode', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); await nodeManager.setNode(nodesUtils.decodeNodeId(nodeId)!, { host: node.proxy.getProxyHost(), port: node.proxy.getProxyPort(), @@ -241,12 +243,12 @@ describe('gestaltsGestaltTrustByNode', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await nodeGraph.stop(); await proxy.stop(); await sigchain.stop(); @@ -255,6 +257,7 @@ describe('gestaltsGestaltTrustByNode', () => { await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/identitiesClaim.test.ts b/tests/client/service/identitiesClaim.test.ts index 1dcba0893..39a23ec3e 100644 --- a/tests/client/service/identitiesClaim.test.ts +++ b/tests/client/service/identitiesClaim.test.ts @@ -9,7 +9,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import IdentitiesManager from '@/identities/IdentitiesManager'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; @@ -75,7 +75,7 @@ describe('identitiesClaim', () => { let testProvider: TestProvider; let identitiesManager: IdentitiesManager; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let sigchain: Sigchain; let proxy: Proxy; @@ -128,19 +128,21 @@ describe('identitiesClaim', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ connConnectTime: 2000, proxy, keyManager, nodeGraph, - queue, + taskManager, logger: logger.getChild('NodeConnectionManager'), }); - await queue.start(); await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + await taskManager.startProcessing(); const clientService = { identitiesClaim: identitiesClaim({ authenticate, @@ -165,16 +167,17 @@ describe('identitiesClaim', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await nodeConnectionManager.stop(); - await queue.stop(); await nodeGraph.stop(); await sigchain.stop(); await proxy.stop(); await identitiesManager.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/nodesAdd.test.ts b/tests/client/service/nodesAdd.test.ts index fe28906de..e3eebd810 100644 --- a/tests/client/service/nodesAdd.test.ts +++ b/tests/client/service/nodesAdd.test.ts @@ -5,7 +5,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; import NodeGraph from '@/nodes/NodeGraph'; @@ -34,7 +34,7 @@ describe('nodesAdd', () => { const authToken = 'abc123'; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let sigchain: Sigchain; @@ -82,14 +82,16 @@ describe('nodesAdd', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -100,12 +102,12 @@ describe('nodesAdd', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const clientService = { nodesAdd: nodesAdd({ authenticate, @@ -128,16 +130,17 @@ describe('nodesAdd', () => { }); }); afterEach(async () => { + await taskManager.startProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await nodeGraph.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await sigchain.stop(); await proxy.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/nodesClaim.test.ts b/tests/client/service/nodesClaim.test.ts index 55fe371d7..21f812fea 100644 --- a/tests/client/service/nodesClaim.test.ts +++ b/tests/client/service/nodesClaim.test.ts @@ -7,7 +7,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import NotificationsManager from '@/notifications/NotificationsManager'; import ACL from '@/acl/ACL'; @@ -65,7 +65,7 @@ describe('nodesClaim', () => { const authToken = 'abc123'; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let notificationsManager: NotificationsManager; @@ -118,14 +118,16 @@ describe('nodesClaim', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -136,12 +138,12 @@ describe('nodesClaim', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); notificationsManager = await NotificationsManager.createNotificationsManager({ acl, @@ -174,11 +176,11 @@ describe('nodesClaim', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await nodeGraph.stop(); await notificationsManager.stop(); await sigchain.stop(); @@ -186,6 +188,7 @@ describe('nodesClaim', () => { await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/nodesFind.test.ts b/tests/client/service/nodesFind.test.ts index f8dd24b27..9ef517816 100644 --- a/tests/client/service/nodesFind.test.ts +++ b/tests/client/service/nodesFind.test.ts @@ -6,7 +6,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; import NodeGraph from '@/nodes/NodeGraph'; @@ -44,7 +44,7 @@ describe('nodesFind', () => { const authToken = 'abc123'; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let sigchain: Sigchain; let proxy: Proxy; @@ -91,20 +91,22 @@ describe('nodesFind', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), }); - await queue.start(); await nodeConnectionManager.start({ nodeManager: {} as NodeManager }); + await taskManager.startProcessing(); const clientService = { nodesFind: nodesFind({ authenticate, @@ -126,15 +128,16 @@ describe('nodesFind', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await sigchain.stop(); await nodeGraph.stop(); await nodeConnectionManager.stop(); - await queue.stop(); await proxy.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/nodesPing.test.ts b/tests/client/service/nodesPing.test.ts index 5874207df..652d0c6ae 100644 --- a/tests/client/service/nodesPing.test.ts +++ b/tests/client/service/nodesPing.test.ts @@ -5,7 +5,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { Metadata } from '@grpc/grpc-js'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; import NodeGraph from '@/nodes/NodeGraph'; @@ -43,7 +43,7 @@ describe('nodesPing', () => { const authToken = 'abc123'; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let sigchain: Sigchain; @@ -91,14 +91,16 @@ describe('nodesPing', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -109,11 +111,11 @@ describe('nodesPing', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const clientService = { nodesPing: nodesPing({ authenticate, @@ -135,15 +137,16 @@ describe('nodesPing', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await sigchain.stop(); await nodeGraph.stop(); await nodeConnectionManager.stop(); - await queue.stop(); await proxy.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/notificationsClear.test.ts b/tests/client/service/notificationsClear.test.ts index 64aa78eb8..a6546bd3a 100644 --- a/tests/client/service/notificationsClear.test.ts +++ b/tests/client/service/notificationsClear.test.ts @@ -5,7 +5,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { Metadata } from '@grpc/grpc-js'; import { DB } from '@matrixai/db'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import GRPCServer from '@/grpc/GRPCServer'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; @@ -41,7 +41,7 @@ describe('notificationsClear', () => { const authToken = 'abc123'; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let notificationsManager: NotificationsManager; @@ -95,14 +95,16 @@ describe('notificationsClear', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -113,12 +115,12 @@ describe('notificationsClear', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); notificationsManager = await NotificationsManager.createNotificationsManager({ acl, @@ -150,18 +152,19 @@ describe('notificationsClear', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); await nodeGraph.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await sigchain.stop(); await proxy.stop(); await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/notificationsRead.test.ts b/tests/client/service/notificationsRead.test.ts index a39860841..125276cd7 100644 --- a/tests/client/service/notificationsRead.test.ts +++ b/tests/client/service/notificationsRead.test.ts @@ -6,7 +6,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { Metadata } from '@grpc/grpc-js'; import { DB } from '@matrixai/db'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import GRPCServer from '@/grpc/GRPCServer'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; @@ -116,7 +116,7 @@ describe('notificationsRead', () => { const authToken = 'abc123'; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let notificationsManager: NotificationsManager; @@ -170,14 +170,16 @@ describe('notificationsRead', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -188,12 +190,12 @@ describe('notificationsRead', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.start(); notificationsManager = await NotificationsManager.createNotificationsManager({ acl, @@ -225,6 +227,7 @@ describe('notificationsRead', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); @@ -232,11 +235,11 @@ describe('notificationsRead', () => { await nodeGraph.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await proxy.stop(); await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/client/service/notificationsSend.test.ts b/tests/client/service/notificationsSend.test.ts index 3c5aecbce..7e2e7b40e 100644 --- a/tests/client/service/notificationsSend.test.ts +++ b/tests/client/service/notificationsSend.test.ts @@ -6,7 +6,7 @@ import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { Metadata } from '@grpc/grpc-js'; import { DB } from '@matrixai/db'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import KeyManager from '@/keys/KeyManager'; import GRPCServer from '@/grpc/GRPCServer'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; @@ -52,7 +52,7 @@ describe('notificationsSend', () => { const authToken = 'abc123'; let dataDir: string; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let notificationsManager: NotificationsManager; @@ -105,14 +105,16 @@ describe('notificationsSend', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -123,12 +125,12 @@ describe('notificationsSend', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); notificationsManager = await NotificationsManager.createNotificationsManager({ acl, @@ -159,18 +161,19 @@ describe('notificationsSend', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); await nodeGraph.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await sigchain.stop(); await proxy.stop(); await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/discovery/Discovery.test.ts b/tests/discovery/Discovery.test.ts index 2e59779b1..ab380c175 100644 --- a/tests/discovery/Discovery.test.ts +++ b/tests/discovery/Discovery.test.ts @@ -6,7 +6,7 @@ import path from 'path'; import os from 'os'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import Discovery from '@/discovery/Discovery'; import GestaltGraph from '@/gestalts/GestaltGraph'; @@ -46,7 +46,7 @@ describe('Discovery', () => { let gestaltGraph: GestaltGraph; let identitiesManager: IdentitiesManager; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let db: DB; @@ -124,14 +124,16 @@ describe('Discovery', () => { keyManager, logger: logger.getChild('NodeGraph'), }); - queue = new Queue({ - logger: logger.getChild('queue'), + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue, + taskManager, connConnectTime: 2000, connTimeoutTime: 2000, logger: logger.getChild('NodeConnectionManager'), @@ -142,12 +144,12 @@ describe('Discovery', () => { nodeConnectionManager, nodeGraph, sigchain, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); // Set up other gestalt nodeA = await PolykeyAgent.createPolykeyAgent({ password: password, @@ -200,11 +202,11 @@ describe('Discovery', () => { await testProvider.publishClaim(identityId, claim); }); afterEach(async () => { + await taskManager.stopProcessing(); await nodeA.stop(); await nodeB.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); - await queue.stop(); await nodeGraph.stop(); await proxy.stop(); await sigchain.stop(); @@ -213,6 +215,7 @@ describe('Discovery', () => { await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/notifications/NotificationsManager.test.ts b/tests/notifications/NotificationsManager.test.ts index 0a4d23f3e..103364e9e 100644 --- a/tests/notifications/NotificationsManager.test.ts +++ b/tests/notifications/NotificationsManager.test.ts @@ -8,7 +8,7 @@ import path from 'path'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import { IdInternal } from '@matrixai/id'; -import Queue from '@/nodes/Queue'; +import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import ACL from '@/acl/ACL'; import Sigchain from '@/sigchain/Sigchain'; @@ -49,7 +49,7 @@ describe('NotificationsManager', () => { let acl: ACL; let db: DB; let nodeGraph: NodeGraph; - let queue: Queue; + let taskManager: TaskManager; let nodeConnectionManager: NodeConnectionManager; let nodeManager: NodeManager; let keyManager: KeyManager; @@ -106,12 +106,16 @@ describe('NotificationsManager', () => { keyManager, logger, }); - queue = new Queue({ logger }); + taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, + }); nodeConnectionManager = new NodeConnectionManager({ nodeGraph, keyManager, proxy, - queue, + taskManager, logger, }); nodeManager = new NodeManager({ @@ -120,12 +124,12 @@ describe('NotificationsManager', () => { sigchain, nodeConnectionManager, nodeGraph, - queue, + taskManager, logger, }); - await queue.start(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.start(); // Set up node for receiving notifications receiver = await PolykeyAgent.createPolykeyAgent({ password: password, @@ -144,8 +148,8 @@ describe('NotificationsManager', () => { }); }, globalThis.defaultTimeout); afterEach(async () => { + await taskManager.stopProcessing(); await receiver.stop(); - await queue.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); await nodeGraph.stop(); @@ -154,6 +158,7 @@ describe('NotificationsManager', () => { await acl.stop(); await db.stop(); await keyManager.stop(); + await taskManager.stop(); await fs.promises.rm(dataDir, { force: true, recursive: true, diff --git a/tests/vaults/VaultManager.test.ts b/tests/vaults/VaultManager.test.ts index 762010273..76ddb6fdf 100644 --- a/tests/vaults/VaultManager.test.ts +++ b/tests/vaults/VaultManager.test.ts @@ -8,7 +8,6 @@ import type { import type NotificationsManager from '@/notifications/NotificationsManager'; import type { Host, Port, TLSConfig } from '@/network/types'; import type NodeManager from '@/nodes/NodeManager'; -import type Queue from '@/nodes/Queue'; import fs from 'fs'; import os from 'os'; import path from 'path'; @@ -18,6 +17,7 @@ import { DB } from '@matrixai/db'; import { destroyed, running } from '@matrixai/async-init'; import git from 'isomorphic-git'; import { RWLockWriter } from '@matrixai/async-locks'; +import TaskManager from '@/tasks/TaskManager'; import ACL from '@/acl/ACL'; import GestaltGraph from '@/gestalts/GestaltGraph'; import NodeConnectionManager from '@/nodes/NodeConnectionManager'; @@ -480,6 +480,7 @@ describe('VaultManager', () => { let remoteKeynode1: PolykeyAgent, remoteKeynode2: PolykeyAgent; let localNodeId: NodeId; let localNodeIdEncoded: NodeIdEncoded; + let taskManager: TaskManager; beforeAll(async () => { // Creating agents @@ -580,18 +581,22 @@ describe('VaultManager', () => { serverHost: localHost, serverPort: port, }); - + taskManager = await TaskManager.createTaskManager({ + db, + lazy: true, + logger, + }); nodeConnectionManager = new NodeConnectionManager({ keyManager, nodeGraph, proxy, - queue: {} as Queue, + taskManager, logger, }); await nodeConnectionManager.start({ nodeManager: { setNode: jest.fn() } as unknown as NodeManager, }); - + await taskManager.startProcessing(); await nodeGraph.setNode(remoteKeynode1Id, { host: remoteKeynode1.proxy.getProxyHost(), port: remoteKeynode1.proxy.getProxyPort(), @@ -602,6 +607,7 @@ describe('VaultManager', () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); await remoteKeynode1.vaultManager.destroyVault(remoteVaultId); await nodeConnectionManager.stop(); await proxy.stop(); @@ -609,6 +615,7 @@ describe('VaultManager', () => { await nodeGraph.destroy(); await keyManager.stop(); await keyManager.destroy(); + await taskManager.stop(); }); test('clone vaults from a remote keynode using a vault name', async () => { @@ -1510,17 +1517,23 @@ describe('VaultManager', () => { serverHost: localHost, serverPort: port, }); + const taskManager = await TaskManager.createTaskManager({ + db, + logger, + lazy: true, + }); const nodeConnectionManager = new NodeConnectionManager({ keyManager, logger, nodeGraph, proxy, - queue: {} as Queue, + taskManager, connConnectTime: 1000, }); await nodeConnectionManager.start({ nodeManager: { setNode: jest.fn() } as unknown as NodeManager, }); + await taskManager.startProcessing(); const vaultManager = await VaultManager.createVaultManager({ vaultsPath, keyManager, @@ -1602,6 +1615,7 @@ describe('VaultManager', () => { ]); expect(vaults[vaultsUtils.encodeVaultId(vault3)]).toBeUndefined(); } finally { + await taskManager.stopProcessing(); await vaultManager.stop(); await vaultManager.destroy(); await nodeConnectionManager.stop(); @@ -1614,6 +1628,7 @@ describe('VaultManager', () => { await acl.destroy(); await remoteAgent.stop(); await remoteAgent.destroy(); + await taskManager.stop(); } }); test('stopping respects locks', async () => { From a4470adcff24bc3a7aa942d7e3432fc20021cd35 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 14 Sep 2022 19:40:34 +1000 Subject: [PATCH 08/40] fix: `getRemoteNodeClosestNodes` shouldn't throw connection errors `getRemoteNodeClosestNodes` was throwing an connection error in certain conditions. If it failed to connect to a node it should've just skipped that node. #418 --- src/nodes/NodeConnectionManager.ts | 92 +++++++++++++++--------------- src/nodes/utils.ts | 12 ++++ 2 files changed, 59 insertions(+), 45 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 353d3ba2d..72d3911af 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -26,8 +26,6 @@ import * as nodesErrors from './errors'; import GRPCClientAgent from '../agent/GRPCClientAgent'; import * as validationUtils from '../validation/utils'; import * as networkUtils from '../network/utils'; -import * as agentErrors from '../agent/errors'; -import * as grpcErrors from '../grpc/errors'; import * as nodesPB from '../proto/js/polykey/v1/nodes/nodes_pb'; import { timerStart } from '../utils'; @@ -190,11 +188,7 @@ class NodeConnectionManager { return [ async (e) => { await release(); - if ( - e instanceof nodesErrors.ErrorNodeConnectionDestroyed || - e instanceof grpcErrors.ErrorGRPC || - e instanceof agentErrors.ErrorAgentClientDestroyed - ) { + if (nodesUtils.isConnectionError(e)) { // Error with connection, shutting connection down await this.destroyConnection(targetNodeId); } @@ -467,9 +461,6 @@ class NodeConnectionManager { // Let foundTarget: boolean = false; let foundAddress: NodeAddress | undefined = undefined; // Get the closest alpha nodes to the target node (set as shortlist) - // FIXME? this is an array. Shouldn't it be a set? - // It's possible for this to grow faster than we can consume it, - // doubly so if we allow duplicates const shortlist = await this.nodeGraph.getClosestNodes( targetNodeId, this.initialClosestNodes, @@ -484,11 +475,10 @@ class NodeConnectionManager { // Not sufficient to simply check if there's already a pre-existing connection // in nodeConnections - what if there's been more than 1 invocation of // getClosestGlobalNodes()? - const contacted: { [nodeId: string]: boolean } = {}; + const contacted: Record = {}; // Iterate until we've found and contacted k nodes while (Object.keys(contacted).length <= this.nodeGraph.nodeBucketLimit) { - if (signal?.aborted) throw new nodesErrors.ErrorNodeAborted(); - // While (!foundTarget) { + if (signal?.aborted) throw signal.reason; // Remove the node from the front of the array const nextNode = shortlist.shift(); // If we have no nodes left in the shortlist, then stop @@ -522,6 +512,7 @@ class NodeConnectionManager { targetNodeId, timer, ); + if (foundClosest.length === 0) continue; // Check to see if any of these are the target node. At the same time, add // them to the shortlist for (const [nodeId, nodeData] of foundClosest) { @@ -585,36 +576,43 @@ class NodeConnectionManager { // Construct the message const nodeIdMessage = new nodesPB.Node(); nodeIdMessage.setNodeId(nodesUtils.encodeNodeId(targetNodeId)); - // Send through client - return this.withConnF( - nodeId, - async (connection) => { - const client = connection.getClient(); - const response = await client.nodesClosestLocalNodesGet(nodeIdMessage); - const nodes: Array<[NodeId, NodeData]> = []; - // Loop over each map element (from the returned response) and populate nodes - response.getNodeTableMap().forEach((address, nodeIdString: string) => { - const nodeId = nodesUtils.decodeNodeId(nodeIdString); - // If the nodeId is not valid we don't add it to the list of nodes - if (nodeId != null) { - nodes.push([ - nodeId, - { - address: { - host: address.getHost() as Host | Hostname, - port: address.getPort() as Port, - }, - // Not really needed - // But if it's needed then we need to add the information to the proto definition - lastUpdated: 0, + try { + // Send through client + const response = await this.withConnF( + nodeId, + async (connection) => { + const client = connection.getClient(); + return await client.nodesClosestLocalNodesGet(nodeIdMessage); + }, + timer, + ); + const nodes: Array<[NodeId, NodeData]> = []; + // Loop over each map element (from the returned response) and populate nodes + response.getNodeTableMap().forEach((address, nodeIdString: string) => { + const nodeId = nodesUtils.decodeNodeId(nodeIdString); + // If the nodeId is not valid we don't add it to the list of nodes + if (nodeId != null) { + nodes.push([ + nodeId, + { + address: { + host: address.getHost() as Host | Hostname, + port: address.getPort() as Port, }, - ]); - } - }); - return nodes; - }, - timer, - ); + // Not really needed + // But if it's needed then we need to add the information to the proto definition + lastUpdated: 0, + }, + ]); + } + }); + return nodes; + } catch (e) { + if (nodesUtils.isConnectionError(e)) { + return []; + } + throw e; + } } /** @@ -625,7 +623,10 @@ class NodeConnectionManager { * non-blocking */ @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) - public async syncNodeGraph(block: boolean = true, timer?: Timer) { + public async syncNodeGraph( + block: boolean = true, + timer?: Timer, + ): Promise { this.logger.info('Syncing nodeGraph'); for (const seedNodeId of this.getSeedNodes()) { // Check if the connection is viable @@ -640,8 +641,9 @@ class NodeConnectionManager { this.keyManager.getNodeId(), timer, ); + const localNodeId = this.keyManager.getNodeId(); for (const [nodeId, nodeData] of closestNodes) { - if (!nodeId.equals(this.keyManager.getNodeId())) { + if (!localNodeId.equals(nodeId)) { const pingAndSetTask = await this.taskManager.scheduleTask({ delay: 0, handlerId: this.pingAndSetNodeHandlerId, @@ -798,7 +800,6 @@ class NodeConnectionManager { const signature = await this.keyManager.signWithRootKeyPair( Buffer.from(proxyAddress), ); - if (signal?.aborted) throw signal.reason; // FIXME: this needs to handle aborting const holePunchPromises = Array.from(this.getSeedNodes(), (seedNodeId) => { return this.sendHolePunchMessage( @@ -817,6 +818,7 @@ class NodeConnectionManager { ); const abortPromise = new Promise((_resolve, reject) => { + if (signal?.aborted) throw signal.reason; signal?.addEventListener('abort', () => reject(signal.reason)); }); diff --git a/src/nodes/utils.ts b/src/nodes/utils.ts index 1fe3c799d..544b7bc55 100644 --- a/src/nodes/utils.ts +++ b/src/nodes/utils.ts @@ -8,8 +8,11 @@ import type { KeyPath } from '@matrixai/db'; import { IdInternal } from '@matrixai/id'; import lexi from 'lexicographic-integer'; import { utils as dbUtils } from '@matrixai/db'; +import * as nodesErrors from './errors'; import { bytes2BigInt } from '../utils'; import * as keysUtils from '../keys/utils'; +import * as grpcErrors from '../grpc/errors'; +import * as agentErrors from '../agent/errors'; const sepBuffer = dbUtils.sep; @@ -310,6 +313,14 @@ function generateRandomNodeIdForBucket( return xOrNodeId(nodeId, randomDistanceForBucket); } +function isConnectionError(e): boolean { + return ( + e instanceof nodesErrors.ErrorNodeConnectionDestroyed || + e instanceof grpcErrors.ErrorGRPC || + e instanceof agentErrors.ErrorAgentClientDestroyed + ); +} + export { sepBuffer, encodeNodeId, @@ -330,4 +341,5 @@ export { generateRandomDistanceForBucket, xOrNodeId, generateRandomNodeIdForBucket, + isConnectionError, }; From ca5e67552639f555b649ecf62013b1dca6270d8c Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 15 Sep 2022 16:01:18 +1000 Subject: [PATCH 09/40] fix: excessive connections from `refreshBuckets` Removing excessive logging for using connections. We don't need a 3 log messages for each time we use an existing connection. Adding 'jitter' or spacing to the `refreshBuckets` delays so that they don't run all at once. This is implemented with a `refreshBucketDelaySpread` paramater that specifies the multiple of the delay to spread across. defaults to 0.5 for 50% Adding a 'heuristic' to `refreshBucket` to prevent it from contacting the same nodes repeatably. Currently this is just a check in `getClosestGlobalNodes` where if we find less than `nodeBucketLimit` nodes we just reset the timer on all `refreshBucket` tasks. Adding tests for checking the spread of `refreshBucket` delays. Another test for resetting the timer on `refreshBucket` tasks if a `findNode` finds less than 20 nodes. #415 --- src/nodes/NodeConnectionManager.ts | 40 +++++------ src/nodes/NodeManager.ts | 44 ++++++++---- .../NodeConnectionManager.seednodes.test.ts | 68 ++++++++++++++++++- tests/nodes/NodeManager.test.ts | 53 +++++++++++++++ 4 files changed, 168 insertions(+), 37 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 72d3911af..897bcc3eb 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -215,14 +215,7 @@ class NodeConnectionManager { ): Promise { return await withF( [await this.acquireConnection(targetNodeId, timer)], - async ([conn]) => { - this.logger.info( - `withConnF calling function with connection to ${nodesUtils.encodeNodeId( - targetNodeId, - )}`, - ); - return await f(conn); - }, + async ([conn]) => await f(conn), ); } @@ -268,25 +261,12 @@ class NodeConnectionManager { targetNodeId: NodeId, timer?: Timer, ): Promise { - this.logger.info( - `Getting connection to ${nodesUtils.encodeNodeId(targetNodeId)}`, - ); const targetNodeIdString = targetNodeId.toString() as NodeIdString; return await this.connectionLocks.withF( [targetNodeIdString, RWLockWriter, 'write'], async () => { const connAndTimer = this.connections.get(targetNodeIdString); - if (connAndTimer != null) { - this.logger.info( - `existing entry found for ${nodesUtils.encodeNodeId(targetNodeId)}`, - ); - return connAndTimer; - } - this.logger.info( - `no existing entry, creating connection to ${nodesUtils.encodeNodeId( - targetNodeId, - )}`, - ); + if (connAndTimer != null) return connAndTimer; // Creating the connection and set in map const targetAddress = await this.findNode(targetNodeId); if (targetAddress == null) { @@ -556,6 +536,22 @@ class NodeConnectionManager { } }); } + // If the found nodes are less than nodeBucketLimit then + // we expect that refresh buckets won't find anything new + if (Object.keys(contacted).length < this.nodeGraph.nodeBucketLimit) { + // Reset the delay on all refresh bucket tasks + for ( + let bucketIndex = 0; + bucketIndex < this.nodeGraph.nodeIdBits; + bucketIndex++ + ) { + await this.nodeManager?.updateRefreshBucketDelay( + bucketIndex, + undefined, + true, + ); + } + } return foundAddress; } diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 4b209b45b..fae495da3 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -38,6 +38,7 @@ class NodeManager { protected nodeGraph: NodeGraph; protected taskManager: TaskManager; protected refreshBucketDelay: number; + protected refreshBucketDelaySpread: number; public readonly setNodeHandlerId = 'NodeManager.setNodeHandler' as TaskHandlerId; public readonly refreshBucketHandlerId = @@ -50,8 +51,12 @@ class NodeManager { ) => { await this.refreshBucket(bucketIndex, { signal: context.signal }); // When completed reschedule the task + const spread = + (Math.random() - 0.5) * + this.refreshBucketDelay * + this.refreshBucketDelaySpread; await this.taskManager.scheduleTask({ - delay: this.refreshBucketDelay, + delay: this.refreshBucketDelay + spread, handlerId: this.refreshBucketHandlerId, lazy: true, parameters: [bucketIndex], @@ -81,6 +86,7 @@ class NodeManager { nodeGraph, taskManager, refreshBucketDelay = 3600000, // 1 hour in milliseconds + refreshBucketDelaySpread = 0.5, // Multiple of refreshBucketDelay to spread by logger, }: { db: DB; @@ -90,6 +96,7 @@ class NodeManager { nodeGraph: NodeGraph; taskManager: TaskManager; refreshBucketDelay?: number; + refreshBucketDelaySpread?: number; logger?: Logger; }) { this.logger = logger ?? new Logger(this.constructor.name); @@ -100,6 +107,11 @@ class NodeManager { this.nodeGraph = nodeGraph; this.taskManager = taskManager; this.refreshBucketDelay = refreshBucketDelay; + // Clamped from 0 to 1 inclusive + this.refreshBucketDelaySpread = Math.max( + 0, + Math.min(refreshBucketDelaySpread, 1), + ); } public async start() { @@ -639,7 +651,6 @@ class NodeManager { } this.logger.info('Setting up refreshBucket tasks'); - // 1. Iterate over existing tasks and reset the delay const existingTasks: Array = new Array(this.nodeGraph.nodeIdBits); for await (const task of this.taskManager.getTasks( @@ -654,30 +665,30 @@ class NodeManager { { // If it's scheduled then reset delay existingTasks[bucketIndex] = true; - this.logger.debug( - `Updating refreshBucket delay for bucket ${bucketIndex}`, - ); // Total delay is refreshBucketDelay + time since task creation + const spread = + (Math.random() - 0.5) * + this.refreshBucketDelay * + this.refreshBucketDelaySpread; const delay = performance.now() + performance.timeOrigin - task.created.getTime() + - this.refreshBucketDelay; + this.refreshBucketDelay + + spread; await this.taskManager.updateTask(task.id, { delay }, tran); } break; case 'queued': case 'active': // If it's running then leave it - this.logger.debug( - `RefreshBucket task for bucket ${bucketIndex} is already active, ignoring`, - ); existingTasks[bucketIndex] = true; break; default: - // Otherwise ignore it, should be re-created + // Otherwise, ignore it, should be re-created existingTasks[bucketIndex] = false; } + this.logger.info('Set up refreshBucket tasks'); } // 2. Recreate any missing tasks for buckets @@ -692,9 +703,13 @@ class NodeManager { this.logger.debug( `Creating refreshBucket task for bucket ${bucketIndex}`, ); + const spread = + (Math.random() - 0.5) * + this.refreshBucketDelay * + this.refreshBucketDelaySpread; await this.taskManager.scheduleTask({ handlerId: this.refreshBucketHandlerId, - delay: this.refreshBucketDelay, + delay: this.refreshBucketDelay + spread, lazy: true, parameters: [bucketIndex], path: ['refreshBucket', `${bucketIndex}`], @@ -718,6 +733,8 @@ class NodeManager { ); } + const spread = + (Math.random() - 0.5) * delay * this.refreshBucketDelaySpread; let foundTask: Task | undefined; let count = 0; for await (const task of this.taskManager.getTasks( @@ -738,7 +755,8 @@ class NodeManager { performance.now() + performance.timeOrigin - task.created.getTime() + - delay; + delay + + spread; await this.taskManager.updateTask(task.id, { delay: delayNew }, tran); this.logger.debug( `Updating refreshBucket task for bucket ${bucketIndex}`, @@ -757,7 +775,7 @@ class NodeManager { `No refreshBucket task for bucket ${bucketIndex}, new one was created`, ); foundTask = await this.taskManager.scheduleTask({ - delay: this.refreshBucketDelay, + delay: delay + spread, handlerId: this.refreshBucketHandlerId, lazy: true, parameters: [bucketIndex], diff --git a/tests/nodes/NodeConnectionManager.seednodes.test.ts b/tests/nodes/NodeConnectionManager.seednodes.test.ts index 24f499cb1..b79964525 100644 --- a/tests/nodes/NodeConnectionManager.seednodes.test.ts +++ b/tests/nodes/NodeConnectionManager.seednodes.test.ts @@ -470,7 +470,6 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { ); mockedPingNode.mockImplementation(async () => true); try { - logger.setLevel(LogLevel.WARN); node1 = await PolykeyAgent.createPolykeyAgent({ nodePath: path.join(dataDir, 'node1'), password: 'password', @@ -533,7 +532,6 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { expect(node2Nodes).toContain(nodeId1); } finally { mockedPingNode.mockRestore(); - logger.setLevel(LogLevel.WARN); await node1?.stop(); await node1?.destroy(); await node2?.stop(); @@ -542,4 +540,70 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { }, globalThis.defaultTimeout * 2, ); + test( + 'refreshBucket delays should be reset after finding less than 20 nodes', + async () => { + // Using a single seed node we need to check that each entering node adds itself to the seed node. + // Also need to check that the new nodes can be seen in the network. + let node1: PolykeyAgent | undefined; + const seedNodes: SeedNodes = {}; + seedNodes[nodesUtils.encodeNodeId(remoteNodeId1)] = { + host: remoteNode1.proxy.getProxyHost(), + port: remoteNode1.proxy.getProxyPort(), + }; + seedNodes[nodesUtils.encodeNodeId(remoteNodeId2)] = { + host: remoteNode2.proxy.getProxyHost(), + port: remoteNode2.proxy.getProxyPort(), + }; + const mockedPingNode = jest.spyOn( + NodeConnectionManager.prototype, + 'pingNode', + ); + mockedPingNode.mockImplementation(async () => true); + try { + node1 = await PolykeyAgent.createPolykeyAgent({ + nodePath: path.join(dataDir, 'node1'), + password: 'password', + networkConfig: { + proxyHost: localHost, + agentHost: localHost, + clientHost: localHost, + forwardHost: localHost, + }, + keysConfig: { + privateKeyPemOverride: globalRootKeyPems[3], + }, + seedNodes, + logger, + }); + + // Reset all the refresh bucket timers to a distinct time + for ( + let bucketIndex = 0; + bucketIndex < node1.nodeGraph.nodeIdBits; + bucketIndex++ + ) { + await node1.nodeManager.updateRefreshBucketDelay( + bucketIndex, + 10000, + true, + ); + } + + // Trigger a refreshBucket + await node1.nodeManager.refreshBucket(1); + + for await (const task of node1.taskManager.getTasks('asc', true, [ + 'refreshBucket', + ])) { + expect(task.delay).toBeGreaterThanOrEqual(50000); + } + } finally { + mockedPingNode.mockRestore(); + await node1?.stop(); + await node1?.destroy(); + } + }, + globalThis.defaultTimeout * 2, + ); }); diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index 8e81081a2..35a90d636 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -989,4 +989,57 @@ describe(`${NodeManager.name} test`, () => { await nodeManager.stop(); } }); + test('refreshBucket tasks should have spread delays', async () => { + const refreshBucketTimeout = 100000; + const nodeManager = new NodeManager({ + db, + sigchain: {} as Sigchain, + keyManager, + nodeGraph, + nodeConnectionManager: dummyNodeConnectionManager, + taskManager, + refreshBucketDelay: refreshBucketTimeout, + logger, + }); + const mockRefreshBucket = jest.spyOn( + NodeManager.prototype, + 'refreshBucket', + ); + try { + mockRefreshBucket.mockImplementation(async () => {}); + await taskManager.startProcessing(); + await nodeManager.start(); + await nodeConnectionManager.start({ nodeManager }); + // Getting starting value + const startingDelay = new Set(); + for await (const task of taskManager.getTasks('asc', true, [ + 'refreshBucket', + ])) { + startingDelay.add(task.delay); + } + expect(startingDelay.size).not.toBe(1); + // Updating delays should have spread + for ( + let bucketIndex = 0; + bucketIndex < nodeGraph.nodeIdBits; + bucketIndex++ + ) { + await nodeManager.updateRefreshBucketDelay( + bucketIndex, + undefined, + true, + ); + } + const updatedDelay = new Set(); + for await (const task of taskManager.getTasks('asc', true, [ + 'refreshBucket', + ])) { + updatedDelay.add(task.delay); + } + expect(updatedDelay.size).not.toBe(1); + } finally { + mockRefreshBucket.mockRestore(); + await nodeManager.stop(); + } + }); }); From 206dceba70534c260d705fe24dca58acd59257ae Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Fri, 16 Sep 2022 13:25:56 +1000 Subject: [PATCH 10/40] feat: `nodeConnectionManager.getClosestGlobalNodes` can optionally skip recently offline nodes This is done with an in-memory map of `nodeIdstring` to some data tracking the backoff period. it defaults to 5 min and doubles each failure. #413 --- src/nodes/NodeConnectionManager.ts | 61 ++++++++++++++++--- src/nodes/NodeManager.ts | 7 ++- .../NodeConnectionManager.general.test.ts | 60 +++++++++++++++++- 3 files changed, 117 insertions(+), 11 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 897bcc3eb..5aa90c657 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -72,6 +72,13 @@ class NodeConnectionManager { */ protected connections: Map = new Map(); protected connectionLocks: LockBox = new LockBox(); + // Tracks the backoff period for offline nodes + protected nodesBackoffMap: Map< + string, + { lastAttempt: number; delay: number } + > = new Map(); + protected backoffDefault: number = 300; // 5 min + protected backoffMultiplier: number = 2; // Doubles every failure protected pingAndSetNodeHandlerId: TaskHandlerId = 'NodeConnectionManager.pingAndSetNodeHandler' as TaskHandlerId; @@ -394,11 +401,13 @@ class NodeConnectionManager { * Retrieves the node address. If an entry doesn't exist in the db, then * proceeds to locate it using Kademlia. * @param targetNodeId Id of the node we are tying to find + * @param ignoreRecentOffline skips nodes that are within their backoff period * @param options */ @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) public async findNode( targetNodeId: NodeId, + ignoreRecentOffline: boolean = false, options: { signal?: AbortSignal } = {}, ): Promise { const { signal } = { ...options }; @@ -407,9 +416,14 @@ class NodeConnectionManager { // Otherwise, attempt to locate it by contacting network address = address ?? - (await this.getClosestGlobalNodes(targetNodeId, undefined, { - signal, - })); + (await this.getClosestGlobalNodes( + targetNodeId, + ignoreRecentOffline, + undefined, + { + signal, + }, + )); // TODO: This currently just does one iteration return address; } @@ -426,6 +440,7 @@ class NodeConnectionManager { * port). * @param targetNodeId ID of the node attempting to be found (i.e. attempting * to find its IP address and port) + * @param ignoreRecentOffline skips nodes that are within their backoff period * @param timer Connection timeout timer * @param options * @returns whether the target node was located in the process @@ -433,6 +448,7 @@ class NodeConnectionManager { @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) public async getClosestGlobalNodes( targetNodeId: NodeId, + ignoreRecentOffline: boolean = false, timer?: Timer, options: { signal?: AbortSignal } = {}, ): Promise { @@ -455,9 +471,9 @@ class NodeConnectionManager { // Not sufficient to simply check if there's already a pre-existing connection // in nodeConnections - what if there's been more than 1 invocation of // getClosestGlobalNodes()? - const contacted: Record = {}; + const contacted: Set = new Set(); // Iterate until we've found and contacted k nodes - while (Object.keys(contacted).length <= this.nodeGraph.nodeBucketLimit) { + while (contacted.size <= this.nodeGraph.nodeBucketLimit) { if (signal?.aborted) throw signal.reason; // Remove the node from the front of the array const nextNode = shortlist.shift(); @@ -467,9 +483,8 @@ class NodeConnectionManager { } const [nextNodeId, nextNodeAddress] = nextNode; // Skip if the node has already been contacted - if (contacted[nextNodeId]) { - continue; - } + if (contacted.has(nextNodeId.toString())) continue; + if (ignoreRecentOffline && this.hasBackoff(nextNodeId)) continue; // Connect to the node (check if pre-existing connection exists, otherwise // create a new one) if ( @@ -482,7 +497,9 @@ class NodeConnectionManager { ) ) { await this.nodeManager!.setNode(nextNodeId, nextNodeAddress.address); + this.removeBackoff(nextNodeId); } else { + this.increaseBackoff(nextNodeId); continue; } contacted[nextNodeId] = true; @@ -828,6 +845,34 @@ class NodeConnectionManager { } return true; } + + protected hasBackoff(nodeId: NodeId): boolean { + const backoff = this.nodesBackoffMap.get(nodeId.toString()); + if (backoff == null) return false; + const currentTime = performance.now() + performance.timeOrigin; + const backOffDeadline = backoff.lastAttempt + backoff.delay; + return currentTime < backOffDeadline; + } + + protected increaseBackoff(nodeId: NodeId): void { + const backoff = this.nodesBackoffMap.get(nodeId.toString()); + const currentTime = performance.now() + performance.timeOrigin; + if (backoff == null) { + this.nodesBackoffMap.set(nodeId.toString(), { + lastAttempt: currentTime, + delay: this.backoffDefault, + }); + } else { + this.nodesBackoffMap.set(nodeId.toString(), { + lastAttempt: currentTime, + delay: backoff.delay * this.backoffMultiplier, + }); + } + } + + protected removeBackoff(nodeId: NodeId): void { + this.nodesBackoffMap.delete(nodeId.toString()); + } } export default NodeConnectionManager; diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index fae495da3..6362ce3e3 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -154,7 +154,8 @@ class NodeManager { // We need to attempt a connection using the proxies // For now we will just do a forward connect + relay message const targetAddress = - address ?? (await this.nodeConnectionManager.findNode(nodeId, options)); + address ?? + (await this.nodeConnectionManager.findNode(nodeId, false, options)); if (targetAddress == null) { throw new nodesErrors.ErrorNodeGraphNodeIdNotFound(); } @@ -640,7 +641,9 @@ class NodeManager { bucketIndex, ); // We then need to start a findNode procedure - await this.nodeConnectionManager.findNode(bucketRandomNodeId, { signal }); + await this.nodeConnectionManager.findNode(bucketRandomNodeId, true, { + signal, + }); } private async setupRefreshBucketTasks(tran?: DBTransaction) { diff --git a/tests/nodes/NodeConnectionManager.general.test.ts b/tests/nodes/NodeConnectionManager.general.test.ts index fcaf3c211..a80e6b309 100644 --- a/tests/nodes/NodeConnectionManager.general.test.ts +++ b/tests/nodes/NodeConnectionManager.general.test.ts @@ -121,7 +121,10 @@ describe(`${NodeConnectionManager.name} general test`, () => { return IdInternal.create(idArray); }; - const dummyNodeManager = { setNode: jest.fn() } as unknown as NodeManager; + const dummyNodeManager = { + setNode: jest.fn(), + updateRefreshBucketDelay: jest.fn(), + } as unknown as NodeManager; const dummyTaskManager: TaskManager = { registerHandler: jest.fn(), deregisterHandler: jest.fn(), @@ -520,4 +523,59 @@ describe(`${NodeConnectionManager.name} general test`, () => { await nodeConnectionManager?.stop(); } }); + test('getClosestGlobalNodes should skip recent offline nodes', async () => { + let nodeConnectionManager: NodeConnectionManager | undefined; + const mockedPingNode = jest.spyOn( + NodeConnectionManager.prototype, + 'pingNode', + ); + try { + nodeConnectionManager = new NodeConnectionManager({ + keyManager, + nodeGraph, + proxy, + taskManager: dummyTaskManager, + logger: nodeConnectionManagerLogger, + }); + await nodeConnectionManager.start({ nodeManager: dummyNodeManager }); + // Check two things, + // 1. existence of a node in the backoff map + // 2. getClosestGlobalNodes doesn't try to connect to offline node + + // Add fake data to `NodeGraph` + await nodeGraph.setNode(nodeId1, { + host: serverHost, + port: serverPort, + }); + await nodeGraph.setNode(nodeId2, { + host: serverHost, + port: serverPort, + }); + + // Making pings fail + mockedPingNode.mockImplementation(async () => false); + await nodeConnectionManager.getClosestGlobalNodes(nodeId3, false); + expect(mockedPingNode).toHaveBeenCalled(); + + // Nodes 1 and 2 should exist in backoff map + // @ts-ignore: kidnap protected property + const backoffMap = nodeConnectionManager.nodesBackoffMap; + expect(backoffMap.has(nodeId1.toString())).toBeTrue(); + expect(backoffMap.has(nodeId2.toString())).toBeTrue(); + expect(backoffMap.has(nodeId3.toString())).toBeFalse(); + + // Next find node should skip offline nodes + mockedPingNode.mockClear(); + await nodeConnectionManager.getClosestGlobalNodes(nodeId3, true); + expect(mockedPingNode).not.toHaveBeenCalled(); + + // We can try connecting anyway + mockedPingNode.mockClear(); + await nodeConnectionManager.getClosestGlobalNodes(nodeId3, false); + expect(mockedPingNode).toHaveBeenCalled(); + } finally { + mockedPingNode.mockRestore(); + await nodeConnectionManager?.stop(); + } + }); }); From c65e7584088cb3e8adb4a97977b11373e8275f6f Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Fri, 16 Sep 2022 14:08:43 +1000 Subject: [PATCH 11/40] feat: added handler to detect promise deadlocks to `ExitHandlers.ts` #307 --- src/bin/errors.ts | 7 +++++++ src/bin/utils/ExitHandlers.ts | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/bin/errors.ts b/src/bin/errors.ts index be6876a65..576fa21a6 100644 --- a/src/bin/errors.ts +++ b/src/bin/errors.ts @@ -49,6 +49,12 @@ class ErrorCLIPolykeyAgentProcess extends ErrorCLI { exitCode = sysexits.OSERR; } +class ErrorCLIPolykeyAsynchronousDeadlock extends ErrorCLI { + static description = + 'PolykeyAgent process exited unexpectedly, likely due to promise deadlock'; + exitCode = sysexits.SOFTWARE; +} + class ErrorNodeFindFailed extends ErrorCLI { static description = 'Failed to find the node in the DHT'; exitCode = 1; @@ -70,6 +76,7 @@ export { ErrorCLIFileRead, ErrorCLIPolykeyAgentStatus, ErrorCLIPolykeyAgentProcess, + ErrorCLIPolykeyAsynchronousDeadlock, ErrorNodeFindFailed, ErrorNodePingFailed, }; diff --git a/src/bin/utils/ExitHandlers.ts b/src/bin/utils/ExitHandlers.ts index 2fdd74f03..24fa27871 100644 --- a/src/bin/utils/ExitHandlers.ts +++ b/src/bin/utils/ExitHandlers.ts @@ -1,6 +1,7 @@ import process from 'process'; import * as binUtils from './utils'; import ErrorPolykey from '../../ErrorPolykey'; +import * as CLIErrors from '../errors'; class ExitHandlers { /** @@ -84,6 +85,19 @@ class ExitHandlers { } }; + protected deadlockHandler = async () => { + if (process.exitCode == null) { + const e = new CLIErrors.ErrorCLIPolykeyAsynchronousDeadlock(); + process.stderr.write( + binUtils.outputFormatter({ + type: this._errFormat, + data: e, + }), + ); + process.exitCode = e.exitCode; + } + }; + /** * Automatically installs all handlers */ @@ -110,6 +124,7 @@ class ExitHandlers { // Both synchronous and asynchronous errors are handled process.once('unhandledRejection', this.errorHandler); process.once('uncaughtException', this.errorHandler); + process.once('beforeExit', this.deadlockHandler); } public uninstall() { @@ -119,6 +134,7 @@ class ExitHandlers { process.removeListener('SIGHUP', this.signalHandler); process.removeListener('unhandledRejection', this.errorHandler); process.removeListener('uncaughtException', this.errorHandler); + process.removeListener('beforeExit', this.deadlockHandler); } /** From f9b1dbe9d7b4c33d66a8497cd28f9d3f858c4132 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Fri, 16 Sep 2022 14:42:55 +1000 Subject: [PATCH 12/40] fix: handerIds are derived from class and handler function names --- src/nodes/NodeConnectionManager.ts | 4 ++-- src/nodes/NodeManager.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 5aa90c657..c21a5021c 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -80,8 +80,6 @@ class NodeConnectionManager { protected backoffDefault: number = 300; // 5 min protected backoffMultiplier: number = 2; // Doubles every failure - protected pingAndSetNodeHandlerId: TaskHandlerId = - 'NodeConnectionManager.pingAndSetNodeHandler' as TaskHandlerId; // TODO: make cancelable protected pingAndSetNodeHandler: TaskHandler = async ( context, @@ -96,6 +94,8 @@ class NodeConnectionManager { await this.nodeManager!.setNode(nodeId, { host: host_, port }, true); } }; + protected pingAndSetNodeHandlerId: TaskHandlerId = + `${this.constructor.name}.${this.pingAndSetNodeHandler.name}` as TaskHandlerId; public constructor({ keyManager, diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 6362ce3e3..e492b41d8 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -39,10 +39,6 @@ class NodeManager { protected taskManager: TaskManager; protected refreshBucketDelay: number; protected refreshBucketDelaySpread: number; - public readonly setNodeHandlerId = - 'NodeManager.setNodeHandler' as TaskHandlerId; - public readonly refreshBucketHandlerId = - 'NodeManager.refreshBucketHandler' as TaskHandlerId; private refreshBucketHandler: TaskHandler = async ( context, @@ -64,6 +60,8 @@ class NodeManager { priority: 0, }); }; + public readonly refreshBucketHandlerId = + `${this.constructor.name}.${this.refreshBucketHandler.name}` as TaskHandlerId; private setNodeHandler: TaskHandler = async ( context, @@ -77,6 +75,8 @@ class NodeManager { signal: context.signal, }); }; + public readonly setNodeHandlerId = + `${this.constructor.name}.${this.setNodeHandler.name}` as TaskHandlerId; constructor({ db, From 20ea3955cde44a46a1a20c42e6e3dfabbafd6324 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Fri, 16 Sep 2022 19:20:34 +1000 Subject: [PATCH 13/40] fix: fixing up `setNode` garbage collection. --- package-lock.json | 14 +++---- package.json | 2 +- src/nodes/NodeManager.ts | 90 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 91 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index d0558ba09..605305c01 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@grpc/grpc-js": "1.6.7", "@matrixai/async-cancellable": "^1.0.2", "@matrixai/async-init": "^1.8.2", - "@matrixai/async-locks": "^3.1.2", + "@matrixai/async-locks": "^3.2.0", "@matrixai/db": "^5.0.3", "@matrixai/errors": "^1.1.3", "@matrixai/id": "^3.3.3", @@ -2638,9 +2638,9 @@ } }, "node_modules/@matrixai/async-locks": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/@matrixai/async-locks/-/async-locks-3.1.2.tgz", - "integrity": "sha512-rIA89EGBNlWV59pLVwx7aqlKWVJRCOsVi6evt8HoN6dyvyyns8//Q8PyBcg5ay0GjLkqsXKQjYXMRif5OB3VSg==", + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/@matrixai/async-locks/-/async-locks-3.2.0.tgz", + "integrity": "sha512-Gl919y3GK2lBCI7M3MabE2u0+XOhKqqgwFEGVaPSI2BrdSI+RY7K3+dzjTSUTujVZwiYskT611CBvlDm9fhsNg==", "dependencies": { "@matrixai/errors": "^1.1.3", "@matrixai/resources": "^1.1.4", @@ -13411,9 +13411,9 @@ } }, "@matrixai/async-locks": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/@matrixai/async-locks/-/async-locks-3.1.2.tgz", - "integrity": "sha512-rIA89EGBNlWV59pLVwx7aqlKWVJRCOsVi6evt8HoN6dyvyyns8//Q8PyBcg5ay0GjLkqsXKQjYXMRif5OB3VSg==", + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/@matrixai/async-locks/-/async-locks-3.2.0.tgz", + "integrity": "sha512-Gl919y3GK2lBCI7M3MabE2u0+XOhKqqgwFEGVaPSI2BrdSI+RY7K3+dzjTSUTujVZwiYskT611CBvlDm9fhsNg==", "requires": { "@matrixai/errors": "^1.1.3", "@matrixai/resources": "^1.1.4", diff --git a/package.json b/package.json index 7d5dfecef..ffd45a1cf 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "@grpc/grpc-js": "1.6.7", "@matrixai/async-cancellable": "^1.0.2", "@matrixai/async-init": "^1.8.2", - "@matrixai/async-locks": "^3.1.2", + "@matrixai/async-locks": "^3.2.0", "@matrixai/db": "^5.0.3", "@matrixai/errors": "^1.1.3", "@matrixai/id": "^3.3.3", diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index e492b41d8..391cd79bb 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -5,18 +5,15 @@ import type KeyManager from '../keys/KeyManager'; import type { PublicKeyPem } from '../keys/types'; import type Sigchain from '../sigchain/Sigchain'; import type { ChainData, ChainDataEncoded } from '../sigchain/types'; -import type { - NodeId, - NodeAddress, - NodeBucket, - NodeBucketIndex, -} from '../nodes/types'; +import type { NodeId, NodeAddress, NodeBucket, NodeBucketIndex } from './types'; import type { ClaimEncoded } from '../claims/types'; import type { Timer } from '../types'; import type TaskManager from '../tasks/TaskManager'; import type { TaskHandler, TaskHandlerId, Task } from '../tasks/types'; import Logger from '@matrixai/logger'; import { StartStop, ready } from '@matrixai/async-init/dist/StartStop'; +import { Semaphore } from '@matrixai/async-locks'; +import { IdInternal } from '@matrixai/id'; import * as nodesErrors from './errors'; import * as nodesUtils from './utils'; import * as networkUtils from '../network/utils'; @@ -39,6 +36,7 @@ class NodeManager { protected taskManager: TaskManager; protected refreshBucketDelay: number; protected refreshBucketDelaySpread: number; + protected pendingNodes: Map> = new Map(); private refreshBucketHandler: TaskHandler = async ( context, @@ -558,7 +556,6 @@ class NodeManager { } } - // FIXME: make cancellable private async garbageCollectOldNode( bucketIndex: number, nodeId: NodeId, @@ -606,6 +603,85 @@ class NodeManager { } } + private async garbageCollectbucket( + bucketIndex: number, + options: { signal?: AbortSignal } = {}, + ): Promise { + const { signal } = { ...options }; + + // This needs to: + // 1. Iterate over every node within the bucket pinging K at a time + // 2. remove any un-responsive nodes until there is room of all pending + // or run out of existing nodes + // 3. fill in the bucket with pending nodes until full + // 4. throw out remaining pending nodes + + const pendingNodes = this.pendingNodes.get(bucketIndex); + // No pending nodes means nothing to do + if (pendingNodes == null || pendingNodes.size === 0) return; + this.pendingNodes.set(bucketIndex, new Map()); + await this.db.withTransactionF(async (tran) => { + // Locking on bucket + await this.nodeGraph.lockBucket(bucketIndex, tran); + const semaphore = new Semaphore(3); + + // Iterating over existing nodes + const bucket = await this.getBucket(bucketIndex, tran); + if (bucket == null) never(); + let removedNodes = 0; + for (const [nodeId, nodeData] of bucket) { + if (removedNodes >= pendingNodes.size) break; + const [semaphoreReleaser] = await semaphore.lock()(); + void (async () => { + // Ping and remove or update node in bucket + if ( + await this.pingNode(nodeId, nodeData.address, undefined, { signal }) + ) { + // Succeeded so update + await this.setNode( + nodeId, + nodeData.address, + true, + false, + undefined, + tran, + { signal }, + ); + } else { + await this.unsetNode(nodeId, tran); + removedNodes += 1; + } + // Releasing semaphore + await semaphoreReleaser(); + })().then(); + // Wait for pending pings to complete + await semaphore.waitForUnlock(); + // Fill in bucket with pending nodes + for (const [nodeIdString, address] of pendingNodes) { + if (removedNodes <= 0) break; + const nodeId = IdInternal.fromString(nodeIdString); + await this.setNode(nodeId, address, true, false, undefined, tran, { + signal, + }); + removedNodes -= 1; + } + } + }); + } + + protected addPendingNode( + bucketIndex: number, + nodeId: NodeId, + nodeAddress: NodeAddress, + ): void { + if (!this.pendingNodes.has(bucketIndex)) { + this.pendingNodes.set(bucketIndex, new Map()); + } + const pendingNodes = this.pendingNodes.get(bucketIndex); + pendingNodes!.set(nodeId.toString(), nodeAddress); + // No need to re-set it in the map, Maps are by reference + } + /** * Removes a node from the NodeGraph */ From 26695b55ff31bd22007e5aebccbb6b604a33a421 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 16:17:25 +1000 Subject: [PATCH 14/40] fix: refactored `nodeManager.setNode` garbage collection --- src/PolykeyAgent.ts | 12 +- src/client/service/nodesAdd.ts | 1 - src/nodes/NodeConnectionManager.ts | 5 +- src/nodes/NodeManager.ts | 244 ++++++++++++----------------- src/tasks/TaskManager.ts | 1 + tests/nodes/NodeManager.test.ts | 106 ++++++------- 6 files changed, 158 insertions(+), 211 deletions(-) diff --git a/src/PolykeyAgent.ts b/src/PolykeyAgent.ts index 377f816bc..7259ab384 100644 --- a/src/PolykeyAgent.ts +++ b/src/PolykeyAgent.ts @@ -582,14 +582,10 @@ class PolykeyAgent { ); // Reverse connection was established and authenticated, // add it to the node graph - await this.nodeManager.setNode( - data.remoteNodeId, - { - host: data.remoteHost, - port: data.remotePort, - }, - false, - ); + await this.nodeManager.setNode(data.remoteNodeId, { + host: data.remoteHost, + port: data.remotePort, + }); } }, ); diff --git a/src/client/service/nodesAdd.ts b/src/client/service/nodesAdd.ts index 87b356b7f..64e1cc34a 100644 --- a/src/client/service/nodesAdd.ts +++ b/src/client/service/nodesAdd.ts @@ -79,7 +79,6 @@ function nodesAdd({ host, port, } as NodeAddress, - true, request.getForce(), undefined, tran, diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index c21a5021c..b97969883 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -91,7 +91,7 @@ class NodeConnectionManager { const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; const host_ = await networkUtils.resolveHost(host); if (await this.pingNode(nodeId, host_, port)) { - await this.nodeManager!.setNode(nodeId, { host: host_, port }, true); + await this.nodeManager!.setNode(nodeId, { host: host_, port }); } }; protected pingAndSetNodeHandlerId: TaskHandlerId = @@ -144,7 +144,6 @@ class NodeConnectionManager { nodeId, this.seedNodes[nodeIdEncoded], true, - true, ); } this.logger.info(`Started ${this.constructor.name}`); @@ -318,7 +317,7 @@ class NodeConnectionManager { }); // We can assume connection was established and destination was valid, // we can add the target to the nodeGraph - await this.nodeManager?.setNode(targetNodeId, targetAddress, false); + await this.nodeManager?.setNode(targetNodeId, targetAddress); // Creating TTL timeout const timeToLiveTimer = setTimeout(async () => { await this.destroyConnection(targetNodeId); diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 391cd79bb..07d11845c 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -12,7 +12,7 @@ import type TaskManager from '../tasks/TaskManager'; import type { TaskHandler, TaskHandlerId, Task } from '../tasks/types'; import Logger from '@matrixai/logger'; import { StartStop, ready } from '@matrixai/async-init/dist/StartStop'; -import { Semaphore } from '@matrixai/async-locks'; +import { Semaphore, Lock } from '@matrixai/async-locks'; import { IdInternal } from '@matrixai/id'; import * as nodesErrors from './errors'; import * as nodesUtils from './utils'; @@ -22,7 +22,7 @@ import * as utilsPB from '../proto/js/polykey/v1/utils/utils_pb'; import * as claimsErrors from '../claims/errors'; import * as sigchainUtils from '../sigchain/utils'; import * as claimsUtils from '../claims/utils'; -import { timerStart, never } from '../utils/utils'; +import { never } from '../utils/utils'; interface NodeManager extends StartStop {} @StartStop() @@ -38,6 +38,7 @@ class NodeManager { protected refreshBucketDelaySpread: number; protected pendingNodes: Map> = new Map(); + public readonly basePath = this.constructor.name; private refreshBucketHandler: TaskHandler = async ( context, taskInfo, @@ -54,27 +55,22 @@ class NodeManager { handlerId: this.refreshBucketHandlerId, lazy: true, parameters: [bucketIndex], - path: ['refreshBucket', `${bucketIndex}`], + path: [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], priority: 0, }); }; public readonly refreshBucketHandlerId = - `${this.constructor.name}.${this.refreshBucketHandler.name}` as TaskHandlerId; - - private setNodeHandler: TaskHandler = async ( - context, - taskInfo, - nodeIdEncoded, - nodeAddress: NodeAddress, - timeout: number, + `${this.basePath}.${this.refreshBucketHandler.name}` as TaskHandlerId; + private gcBucketHandler: TaskHandler = async ( + ctx, + _taskInfo, + bucketIndex: number, ) => { - const nodeId: NodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; - await this.setNode(nodeId, nodeAddress, true, false, timeout, undefined, { - signal: context.signal, - }); + this.logger.info('RUNNING GARBAGE COLELCT'); + await this.garbageCollectBucket(bucketIndex, { signal: ctx.signal }); }; - public readonly setNodeHandlerId = - `${this.constructor.name}.${this.setNodeHandler.name}` as TaskHandlerId; + public readonly gcBucketHandlerId = + `${this.basePath}.${this.gcBucketHandler.name}` as TaskHandlerId; constructor({ db, @@ -115,14 +111,14 @@ class NodeManager { public async start() { this.logger.info(`Starting ${this.constructor.name}`); this.logger.info(`Registering handler for setNode`); - this.taskManager.registerHandler( - this.setNodeHandlerId, - this.setNodeHandler, - ); this.taskManager.registerHandler( this.refreshBucketHandlerId, this.refreshBucketHandler, ); + this.taskManager.registerHandler( + this.gcBucketHandlerId, + this.gcBucketHandler, + ); await this.setupRefreshBucketTasks(); this.logger.info(`Started ${this.constructor.name}`); } @@ -130,8 +126,8 @@ class NodeManager { public async stop() { this.logger.info(`Stopping ${this.constructor.name}`); this.logger.info(`Unregistering handler for setNode`); - this.taskManager.deregisterHandler(this.setNodeHandlerId); this.taskManager.deregisterHandler(this.refreshBucketHandlerId); + this.taskManager.deregisterHandler(this.gcBucketHandlerId); this.logger.info(`Stopped ${this.constructor.name}`); } @@ -442,22 +438,18 @@ class NodeManager { * This operation is blocking by default - set `block` 2qto false to make it non-blocking * @param nodeId - Id of the node we wish to add * @param nodeAddress - Expected address of the node we want to add - * @param block - Flag for if the operation should block or utilize the async queue * @param force - Flag for if we want to add the node without authenticating or if the bucket is full. * This will drop the oldest node in favor of the new. * @param timeout Connection timeout * @param tran - * @param options */ @ready(new nodesErrors.ErrorNodeManagerNotRunning()) public async setNode( nodeId: NodeId, nodeAddress: NodeAddress, - block: boolean = true, force: boolean = false, timeout?: number, tran?: DBTransaction, - options: { signal?: AbortSignal } = {}, ): Promise { // We don't want to add our own node if (nodeId.equals(this.keyManager.getNodeId())) { @@ -467,7 +459,7 @@ class NodeManager { if (tran == null) { return this.db.withTransactionF((tran) => - this.setNode(nodeId, nodeAddress, block, force, timeout, tran), + this.setNode(nodeId, nodeAddress, force, timeout, tran), ); } @@ -502,7 +494,6 @@ class NodeManager { ); } else { // We want to add a node but the bucket is full - // We need to ping the oldest node if (force) { // We just add the new node anyway without checking the old one const oldNodeId = ( @@ -523,87 +514,18 @@ class NodeManager { tran, ); return; - } else if (block) { - this.logger.debug( - `Bucket was full and blocking was true, garbage collecting old nodes to add ${nodesUtils.encodeNodeId( - nodeId, - )}`, - ); - await this.garbageCollectOldNode( - bucketIndex, - nodeId, - nodeAddress, - timeout, - options, - ); - } else { - this.logger.debug( - `Bucket was full and blocking was false, adding ${nodesUtils.encodeNodeId( - nodeId, - )} to queue`, - ); - // Re-attempt this later asynchronously by adding to the scheduler - await this.taskManager.scheduleTask( - { - handlerId: this.setNodeHandlerId, - parameters: [nodesUtils.toString(), nodeAddress, timeout], - path: ['setNode'], - lazy: true, - }, - tran, - ); - } - } - } - - private async garbageCollectOldNode( - bucketIndex: number, - nodeId: NodeId, - nodeAddress: NodeAddress, - timeout?: number, - options: { signal?: AbortSignal } = {}, - ) { - const { signal } = { ...options }; - const oldestNodeIds = await this.nodeGraph.getOldestNode(bucketIndex, 3); - for (const nodeId of oldestNodeIds) { - if (signal?.aborted) throw signal.reason; - // This needs to return nodeId and ping result - const data = await this.nodeGraph.getNode(nodeId); - if (data == null) return { nodeId, success: false }; - const timer = timeout != null ? timerStart(timeout) : undefined; - const success = await this.pingNode(nodeId, nodeAddress, timer, { - signal, - }); - - if (success) { - // Ping succeeded, update the node - this.logger.debug( - `Ping succeeded for ${nodesUtils.encodeNodeId(nodeId)}`, - ); - const node = (await this.nodeGraph.getNode(nodeId))!; - await this.nodeGraph.setNode(nodeId, node.address); - // Updating the refreshBucket timer - await this.updateRefreshBucketDelay( - bucketIndex, - this.refreshBucketDelay, - ); - } else { - this.logger.debug(`Ping failed for ${nodesUtils.encodeNodeId(nodeId)}`); - // Otherwise, we remove the node - await this.nodeGraph.unsetNode(nodeId); } - } - // Check if we now have room and add the new node - const count = await this.nodeGraph.getBucketMetaProp(bucketIndex, 'count'); - if (count < this.nodeGraph.nodeBucketLimit) { - this.logger.debug(`Bucket ${bucketIndex} now has room, adding new node`); - await this.nodeGraph.setNode(nodeId, nodeAddress); - // Updating the refreshBucket timer - await this.updateRefreshBucketDelay(bucketIndex, this.refreshBucketDelay); + this.logger.debug( + `Bucket was full, adding ${nodesUtils.encodeNodeId( + nodeId, + )} to pending list`, + ); + // Add the node to the pending nodes list + await this.addPendingNode(bucketIndex, nodeId, nodeAddress); } } - private async garbageCollectbucket( + private async garbageCollectBucket( bucketIndex: number, options: { signal?: AbortSignal } = {}, ): Promise { @@ -617,7 +539,7 @@ class NodeManager { // 4. throw out remaining pending nodes const pendingNodes = this.pendingNodes.get(bucketIndex); - // No pending nodes means nothing to do + // No nodes mean nothing to do if (pendingNodes == null || pendingNodes.size === 0) return; this.pendingNodes.set(bucketIndex, new Map()); await this.db.withTransactionF(async (tran) => { @@ -629,57 +551,90 @@ class NodeManager { const bucket = await this.getBucket(bucketIndex, tran); if (bucket == null) never(); let removedNodes = 0; + const unsetLock = new Lock(); + const pendingPromises: Array> = []; for (const [nodeId, nodeData] of bucket) { if (removedNodes >= pendingNodes.size) break; - const [semaphoreReleaser] = await semaphore.lock()(); - void (async () => { - // Ping and remove or update node in bucket - if ( - await this.pingNode(nodeId, nodeData.address, undefined, { signal }) - ) { - // Succeeded so update - await this.setNode( - nodeId, - nodeData.address, - true, - false, - undefined, - tran, - { signal }, - ); - } else { - await this.unsetNode(nodeId, tran); - removedNodes += 1; - } - // Releasing semaphore - await semaphoreReleaser(); - })().then(); - // Wait for pending pings to complete await semaphore.waitForUnlock(); - // Fill in bucket with pending nodes - for (const [nodeIdString, address] of pendingNodes) { - if (removedNodes <= 0) break; - const nodeId = IdInternal.fromString(nodeIdString); - await this.setNode(nodeId, address, true, false, undefined, tran, { - signal, - }); - removedNodes -= 1; - } + if (signal?.aborted === true) break; + const [semaphoreReleaser] = await semaphore.lock()(); + pendingPromises.push( + (async () => { + // Ping and remove or update node in bucket + if ( + await this.pingNode(nodeId, nodeData.address, undefined, { + signal, + }) + ) { + // Succeeded so update + await this.setNode( + nodeId, + nodeData.address, + false, + undefined, + tran, + ); + } else { + // We need to lock this since it's concurrent + // and shares the transaction + await unsetLock.withF(async () => { + await this.unsetNode(nodeId, tran); + removedNodes += 1; + }); + } + // Releasing semaphore + await semaphoreReleaser(); + })(), + ); + } + // Wait for pending pings to complete + await Promise.all(pendingPromises); + // Fill in bucket with pending nodes + for (const [nodeIdString, address] of pendingNodes) { + if (removedNodes <= 0) break; + const nodeId = IdInternal.fromString(nodeIdString); + await this.setNode(nodeId, address, false, undefined, tran); + removedNodes -= 1; } }); } - protected addPendingNode( + protected async addPendingNode( bucketIndex: number, nodeId: NodeId, nodeAddress: NodeAddress, - ): void { + ): Promise { if (!this.pendingNodes.has(bucketIndex)) { this.pendingNodes.set(bucketIndex, new Map()); } const pendingNodes = this.pendingNodes.get(bucketIndex); pendingNodes!.set(nodeId.toString(), nodeAddress); // No need to re-set it in the map, Maps are by reference + + // Check and start a 'garbageCollect` bucket task + let first: boolean = true; + for await (const task of this.taskManager.getTasks('asc', true, [ + this.basePath, + this.gcBucketHandlerId, + `${bucketIndex}`, + ])) { + if (first) { + // Just ignore it. + first = false; + continue; + } + // There shouldn't be duplicates, we'll remove extra + task.cancel('Removing extra task'); + } + if (first) { + // If none were found, schedule a new one + await this.taskManager.scheduleTask({ + handlerId: this.gcBucketHandlerId, + parameters: [bucketIndex], + path: [this.basePath, this.gcBucketHandlerId, `${bucketIndex}`], + lazy: true, + }); + } } /** @@ -767,7 +722,6 @@ class NodeManager { // Otherwise, ignore it, should be re-created existingTasks[bucketIndex] = false; } - this.logger.info('Set up refreshBucket tasks'); } // 2. Recreate any missing tasks for buckets @@ -791,7 +745,7 @@ class NodeManager { delay: this.refreshBucketDelay + spread, lazy: true, parameters: [bucketIndex], - path: ['refreshBucket', `${bucketIndex}`], + path: [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], priority: 0, }); } @@ -850,7 +804,7 @@ class NodeManager { } } if (count === 0) { - this.logger.warn( + this.logger.debug( `No refreshBucket task for bucket ${bucketIndex}, new one was created`, ); foundTask = await this.taskManager.scheduleTask({ @@ -858,7 +812,7 @@ class NodeManager { handlerId: this.refreshBucketHandlerId, lazy: true, parameters: [bucketIndex], - path: ['refreshBucket', `${bucketIndex}`], + path: [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], priority: 0, }); } diff --git a/src/tasks/TaskManager.ts b/src/tasks/TaskManager.ts index 6dc221def..9e3da47d5 100644 --- a/src/tasks/TaskManager.ts +++ b/src/tasks/TaskManager.ts @@ -1148,6 +1148,7 @@ class TaskManager { this.logger.debug(`Requeued Task ${taskIdEncoded}`); } + @ready(new tasksErrors.ErrorTaskManagerNotRunning()) protected async cancelTask(taskId: TaskId, cancelReason: any): Promise { const taskIdEncoded = tasksUtils.encodeTaskId(taskId); this.logger.debug(`Cancelling Task ${taskIdEncoded}`); diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index 35a90d636..8fc010ea9 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -123,14 +123,16 @@ describe(`${NodeManager.name} test`, () => { }); }); afterEach(async () => { + await taskManager.stopProcessing(); + await taskManager.stopTasks(); mockedPingNode.mockClear(); mockedPingNode.mockImplementation(async (_) => true); await nodeConnectionManager.stop(); - await taskManager.stop(); await nodeGraph.stop(); await nodeGraph.destroy(); await sigchain.stop(); await sigchain.destroy(); + await taskManager.stop(); await db.stop(); await db.destroy(); await keyManager.stop(); @@ -551,10 +553,10 @@ describe(`${NodeManager.name} test`, () => { taskManager, logger, }); + const nodeManagerPingMock = jest.spyOn(NodeManager.prototype, 'pingNode'); try { await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); - await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; // Creating 20 nodes in bucket @@ -571,14 +573,22 @@ describe(`${NodeManager.name} test`, () => { bucketIndex, ); // Mocking ping - const nodeManagerPingMock = jest.spyOn(NodeManager.prototype, 'pingNode'); nodeManagerPingMock.mockResolvedValue(true); const oldestNodeId = (await nodeGraph.getOldestNode(bucketIndex)).pop(); const oldestNode = await nodeGraph.getNode(oldestNodeId!); // Waiting for a second to tick over await sleep(1500); // Adding a new node with bucket full - await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress, true); + await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress); + const tasks: Array> = []; + for await (const task of taskManager.getTasks('asc', false, [ + nodeManager.basePath, + nodeManager.gcBucketHandlerId, + ])) { + tasks.push(task.promise()); + } + await taskManager.startProcessing(); + await Promise.all(tasks); // Bucket still contains max nodes const bucket = await nodeManager.getBucket(bucketIndex); expect(bucket).toHaveLength(nodeGraph.nodeBucketLimit); @@ -588,9 +598,9 @@ describe(`${NodeManager.name} test`, () => { // Oldest node was updated const oldestNodeNew = await nodeGraph.getNode(oldestNodeId!); expect(oldestNodeNew!.lastUpdated).not.toEqual(oldestNode!.lastUpdated); - nodeManagerPingMock.mockRestore(); } finally { await nodeManager.stop(); + nodeManagerPingMock.mockRestore(); } }); test('should add node if bucket is full, old node is alive and force is set', async () => { @@ -627,12 +637,7 @@ describe(`${NodeManager.name} test`, () => { nodeManagerPingMock.mockResolvedValue(true); const oldestNodeId = (await nodeGraph.getOldestNode(bucketIndex)).pop(); // Adding a new node with bucket full - await nodeManager.setNode( - nodeId, - { port: 55555 } as NodeAddress, - false, - true, - ); + await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress, true); // Bucket still contains max nodes const bucket = await nodeManager.getBucket(bucketIndex); expect(bucket).toHaveLength(nodeGraph.nodeBucketLimit); @@ -660,7 +665,6 @@ describe(`${NodeManager.name} test`, () => { try { await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); - await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; // Creating 20 nodes in bucket @@ -681,7 +685,16 @@ describe(`${NodeManager.name} test`, () => { nodeManagerPingMock.mockResolvedValue(false); const oldestNodeId = (await nodeGraph.getOldestNode(bucketIndex)).pop(); // Adding a new node with bucket full - await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress, true); + await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress); + const tasks: Array> = []; + for await (const task of taskManager.getTasks('asc', false, [ + nodeManager.basePath, + nodeManager.gcBucketHandlerId, + ])) { + tasks.push(task.promise()); + } + await taskManager.startProcessing(); + await Promise.all(tasks); // New node was added const node = await nodeGraph.getNode(nodeId); expect(node).toBeDefined(); @@ -764,7 +777,6 @@ describe(`${NodeManager.name} test`, () => { try { await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); - await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -780,9 +792,18 @@ describe(`${NodeManager.name} test`, () => { }; // Pings succeed, node not added - mockedPingNode.mockImplementation(async (_) => true); + mockedPingNode.mockImplementation(async () => true); const newNode = generateNodeIdForBucket(nodeId, 100, 21); await nodeManager.setNode(newNode, address); + const tasks: Array> = []; + for await (const task of taskManager.getTasks('asc', false, [ + nodeManager.basePath, + nodeManager.gcBucketHandlerId, + ])) { + tasks.push(task.promise()); + } + await taskManager.startProcessing(); + await Promise.all(tasks); expect(await listBucket(100)).not.toContain( nodesUtils.encodeNodeId(newNode), ); @@ -804,7 +825,6 @@ describe(`${NodeManager.name} test`, () => { await nodeManager.start(); try { await nodeConnectionManager.start({ nodeManager }); - await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -827,6 +847,15 @@ describe(`${NodeManager.name} test`, () => { await nodeManager.setNode(newNode1, address); await nodeManager.setNode(newNode2, address); await nodeManager.setNode(newNode3, address); + const tasks: Array> = []; + for await (const task of taskManager.getTasks('asc', false, [ + nodeManager.basePath, + nodeManager.gcBucketHandlerId, + ])) { + tasks.push(task.promise()); + } + await taskManager.startProcessing(); + await Promise.all(tasks); const list = await listBucket(100); expect(list).toContain(nodesUtils.encodeNodeId(newNode1)); expect(list).toContain(nodesUtils.encodeNodeId(newNode2)); @@ -872,7 +901,7 @@ describe(`${NodeManager.name} test`, () => { const newNode4 = generateNodeIdForBucket(nodeId, 100, 25); // Set manually to non-blocking await expect( - nodeManager.setNode(newNode4, address, false), + nodeManager.setNode(newNode4, address), ).resolves.toBeUndefined(); delayPing.resolveP(); } finally { @@ -881,41 +910,6 @@ describe(`${NodeManager.name} test`, () => { await tempNodeGraph.destroy(); } }); - test('should block when blocking is set to true', async () => { - mockedPingNode.mockImplementation(async (_) => true); - const nodeManager = new NodeManager({ - db, - sigchain: {} as Sigchain, - keyManager, - nodeGraph, - nodeConnectionManager: dummyNodeConnectionManager, - taskManager, - logger, - }); - await nodeManager.start(); - try { - await nodeConnectionManager.start({ nodeManager }); - await taskManager.startProcessing(); - const nodeId = keyManager.getNodeId(); - const address = { host: localhost, port }; - // Let's fill a bucket - for (let i = 0; i < nodeGraph.nodeBucketLimit; i++) { - const newNode = generateNodeIdForBucket(nodeId, 100, i); - await nodeManager.setNode(newNode, address); - } - - // Set node can block - mockedPingNode.mockClear(); - mockedPingNode.mockImplementation(async () => true); - const newNode5 = generateNodeIdForBucket(nodeId, 100, 25); - await expect( - nodeManager.setNode(newNode5, address, true), - ).resolves.toBeUndefined(); - expect(mockedPingNode).toBeCalled(); - } finally { - await nodeManager.stop(); - } - }); test('should update deadline when updating a bucket', async () => { const refreshBucketTimeout = 100000; const nodeManager = new NodeManager({ @@ -941,7 +935,8 @@ describe(`${NodeManager.name} test`, () => { const bucketIndex = 100; let refreshBucketTask: Task | undefined; for await (const task of taskManager.getTasks('asc', true, [ - 'refreshBucket', + nodeManager.basePath, + nodeManager.refreshBucketHandlerId, `${bucketIndex}`, ])) { refreshBucketTask = task; @@ -956,7 +951,8 @@ describe(`${NodeManager.name} test`, () => { // Deadline should be updated let refreshBucketTaskUpdated: Task | undefined; for await (const task of taskManager.getTasks('asc', true, [ - 'refreshBucket', + nodeManager.basePath, + nodeManager.refreshBucketHandlerId, `${bucketIndex}`, ])) { refreshBucketTaskUpdated = task; @@ -966,6 +962,8 @@ describe(`${NodeManager.name} test`, () => { refreshBucketTask.delay, ); } finally { + await taskManager.stopProcessing(); + await taskManager.stopTasks(); mockRefreshBucket.mockRestore(); await nodeManager.stop(); } From ca9af189509403bd71e0c1938c15165681c0019f Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 16:25:12 +1000 Subject: [PATCH 15/40] tests: proper stopping of `taskManager` in tests --- tests/agent/GRPCClientAgent.test.ts | 1 + tests/agent/service/notificationsSend.test.ts | 1 + tests/client/service/gestaltsDiscoveryByIdentity.test.ts | 1 + tests/client/service/gestaltsDiscoveryByNode.test.ts | 1 + tests/client/service/gestaltsGestaltTrustByIdentity.test.ts | 1 + tests/client/service/gestaltsGestaltTrustByNode.test.ts | 1 + tests/client/service/identitiesClaim.test.ts | 1 + tests/client/service/nodesAdd.test.ts | 3 ++- tests/client/service/nodesClaim.test.ts | 1 + tests/client/service/nodesFind.test.ts | 1 + tests/client/service/nodesPing.test.ts | 1 + tests/client/service/notificationsClear.test.ts | 1 + tests/client/service/notificationsRead.test.ts | 1 + tests/client/service/notificationsSend.test.ts | 1 + tests/discovery/Discovery.test.ts | 1 + tests/notifications/NotificationsManager.test.ts | 1 + tests/vaults/VaultManager.test.ts | 2 ++ 17 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/agent/GRPCClientAgent.test.ts b/tests/agent/GRPCClientAgent.test.ts index 6719ac6ce..2a932aede 100644 --- a/tests/agent/GRPCClientAgent.test.ts +++ b/tests/agent/GRPCClientAgent.test.ts @@ -174,6 +174,7 @@ describe(GRPCClientAgent.name, () => { }, globalThis.defaultTimeout); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await testAgentUtils.closeTestAgentClient(client); await testAgentUtils.closeTestAgentServer(server); await vaultManager.stop(); diff --git a/tests/agent/service/notificationsSend.test.ts b/tests/agent/service/notificationsSend.test.ts index 21d3d1aeb..22d5eea14 100644 --- a/tests/agent/service/notificationsSend.test.ts +++ b/tests/agent/service/notificationsSend.test.ts @@ -159,6 +159,7 @@ describe('notificationsSend', () => { }, globalThis.defaultTimeout); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); diff --git a/tests/client/service/gestaltsDiscoveryByIdentity.test.ts b/tests/client/service/gestaltsDiscoveryByIdentity.test.ts index 38176072d..d4c64807e 100644 --- a/tests/client/service/gestaltsDiscoveryByIdentity.test.ts +++ b/tests/client/service/gestaltsDiscoveryByIdentity.test.ts @@ -170,6 +170,7 @@ describe('gestaltsDiscoveryByIdentity', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); diff --git a/tests/client/service/gestaltsDiscoveryByNode.test.ts b/tests/client/service/gestaltsDiscoveryByNode.test.ts index d88e5d475..0354ed66f 100644 --- a/tests/client/service/gestaltsDiscoveryByNode.test.ts +++ b/tests/client/service/gestaltsDiscoveryByNode.test.ts @@ -171,6 +171,7 @@ describe('gestaltsDiscoveryByNode', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); diff --git a/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts b/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts index 8a6a3d03a..ea0bc370d 100644 --- a/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts +++ b/tests/client/service/gestaltsGestaltTrustByIdentity.test.ts @@ -236,6 +236,7 @@ describe('gestaltsGestaltTrustByIdentity', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); diff --git a/tests/client/service/gestaltsGestaltTrustByNode.test.ts b/tests/client/service/gestaltsGestaltTrustByNode.test.ts index fd6a2f8d1..200f45eb6 100644 --- a/tests/client/service/gestaltsGestaltTrustByNode.test.ts +++ b/tests/client/service/gestaltsGestaltTrustByNode.test.ts @@ -244,6 +244,7 @@ describe('gestaltsGestaltTrustByNode', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await discovery.stop(); diff --git a/tests/client/service/identitiesClaim.test.ts b/tests/client/service/identitiesClaim.test.ts index 39a23ec3e..5be95e093 100644 --- a/tests/client/service/identitiesClaim.test.ts +++ b/tests/client/service/identitiesClaim.test.ts @@ -168,6 +168,7 @@ describe('identitiesClaim', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await nodeConnectionManager.stop(); diff --git a/tests/client/service/nodesAdd.test.ts b/tests/client/service/nodesAdd.test.ts index e3eebd810..0d8ccb29f 100644 --- a/tests/client/service/nodesAdd.test.ts +++ b/tests/client/service/nodesAdd.test.ts @@ -130,7 +130,8 @@ describe('nodesAdd', () => { }); }); afterEach(async () => { - await taskManager.startProcessing(); + await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await nodeGraph.stop(); diff --git a/tests/client/service/nodesClaim.test.ts b/tests/client/service/nodesClaim.test.ts index 21f812fea..824161c99 100644 --- a/tests/client/service/nodesClaim.test.ts +++ b/tests/client/service/nodesClaim.test.ts @@ -177,6 +177,7 @@ describe('nodesClaim', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await nodeConnectionManager.stop(); diff --git a/tests/client/service/nodesFind.test.ts b/tests/client/service/nodesFind.test.ts index 9ef517816..c58123a38 100644 --- a/tests/client/service/nodesFind.test.ts +++ b/tests/client/service/nodesFind.test.ts @@ -129,6 +129,7 @@ describe('nodesFind', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await sigchain.stop(); diff --git a/tests/client/service/nodesPing.test.ts b/tests/client/service/nodesPing.test.ts index 652d0c6ae..1e05faf36 100644 --- a/tests/client/service/nodesPing.test.ts +++ b/tests/client/service/nodesPing.test.ts @@ -138,6 +138,7 @@ describe('nodesPing', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await sigchain.stop(); diff --git a/tests/client/service/notificationsClear.test.ts b/tests/client/service/notificationsClear.test.ts index a6546bd3a..45551e501 100644 --- a/tests/client/service/notificationsClear.test.ts +++ b/tests/client/service/notificationsClear.test.ts @@ -153,6 +153,7 @@ describe('notificationsClear', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); diff --git a/tests/client/service/notificationsRead.test.ts b/tests/client/service/notificationsRead.test.ts index 125276cd7..07faca128 100644 --- a/tests/client/service/notificationsRead.test.ts +++ b/tests/client/service/notificationsRead.test.ts @@ -228,6 +228,7 @@ describe('notificationsRead', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); diff --git a/tests/client/service/notificationsSend.test.ts b/tests/client/service/notificationsSend.test.ts index 7e2e7b40e..0841ef7c2 100644 --- a/tests/client/service/notificationsSend.test.ts +++ b/tests/client/service/notificationsSend.test.ts @@ -162,6 +162,7 @@ describe('notificationsSend', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await grpcClient.destroy(); await grpcServer.stop(); await notificationsManager.stop(); diff --git a/tests/discovery/Discovery.test.ts b/tests/discovery/Discovery.test.ts index ab380c175..3a5ebf34e 100644 --- a/tests/discovery/Discovery.test.ts +++ b/tests/discovery/Discovery.test.ts @@ -203,6 +203,7 @@ describe('Discovery', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await nodeA.stop(); await nodeB.stop(); await nodeConnectionManager.stop(); diff --git a/tests/notifications/NotificationsManager.test.ts b/tests/notifications/NotificationsManager.test.ts index 103364e9e..a01a577db 100644 --- a/tests/notifications/NotificationsManager.test.ts +++ b/tests/notifications/NotificationsManager.test.ts @@ -149,6 +149,7 @@ describe('NotificationsManager', () => { }, globalThis.defaultTimeout); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await receiver.stop(); await nodeConnectionManager.stop(); await nodeManager.stop(); diff --git a/tests/vaults/VaultManager.test.ts b/tests/vaults/VaultManager.test.ts index 76ddb6fdf..0e9ff57e5 100644 --- a/tests/vaults/VaultManager.test.ts +++ b/tests/vaults/VaultManager.test.ts @@ -608,6 +608,7 @@ describe('VaultManager', () => { }); afterEach(async () => { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await remoteKeynode1.vaultManager.destroyVault(remoteVaultId); await nodeConnectionManager.stop(); await proxy.stop(); @@ -1616,6 +1617,7 @@ describe('VaultManager', () => { expect(vaults[vaultsUtils.encodeVaultId(vault3)]).toBeUndefined(); } finally { await taskManager.stopProcessing(); + await taskManager.stopTasks(); await vaultManager.stop(); await vaultManager.destroy(); await nodeConnectionManager.stop(); From 3be12d7c367c9320ebf040bee2341c5a5c4f910d Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 16:33:30 +1000 Subject: [PATCH 16/40] fix: updating task paths `TaskPaths` should take the form of `[basePath, handlerId, ...extra]`. basePath is the `this.constructor.name` for the domain the handler is registered for. --- src/nodes/NodeConnectionManager.ts | 5 +++-- src/nodes/NodeManager.ts | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index b97969883..1d7cd5f4b 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -81,6 +81,7 @@ class NodeConnectionManager { protected backoffMultiplier: number = 2; // Doubles every failure // TODO: make cancelable + public readonly basePath = this.constructor.name; protected pingAndSetNodeHandler: TaskHandler = async ( context, taskInfo, @@ -95,7 +96,7 @@ class NodeConnectionManager { } }; protected pingAndSetNodeHandlerId: TaskHandlerId = - `${this.constructor.name}.${this.pingAndSetNodeHandler.name}` as TaskHandlerId; + `${this.basePath}.${this.pingAndSetNodeHandler.name}` as TaskHandlerId; public constructor({ keyManager, @@ -665,7 +666,7 @@ class NodeConnectionManager { nodeData.address.host, nodeData.address.port, ], - path: ['pingAndSetNode'], + path: [this.basePath, this.pingAndSetNodeHandlerId], // Need to be somewhat active so high priority priority: 100, }); diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 07d11845c..9b085307a 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -690,7 +690,7 @@ class NodeManager { for await (const task of this.taskManager.getTasks( 'asc', true, - ['refreshBucket'], + [this.basePath, this.refreshBucketHandlerId], tran, )) { const bucketIndex = parseInt(task.path[0]); @@ -773,7 +773,7 @@ class NodeManager { for await (const task of this.taskManager.getTasks( 'asc', true, - ['refreshBucket', `${bucketIndex}`], + [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], tran, )) { count += 1; From 19bce20e653e548c1d54a9e63de5e4c5dcdefb18 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 20:09:32 +1000 Subject: [PATCH 17/40] feat: task handlers are now `timedCancellable` --- src/network/Proxy.ts | 20 +++- src/nodes/NodeConnectionManager.ts | 98 +++++++++++-------- src/nodes/NodeManager.ts | 63 ++++++------ tests/network/Proxy.test.ts | 7 +- .../NodeConnectionManager.general.test.ts | 9 +- .../NodeConnectionManager.lifecycle.test.ts | 8 +- .../NodeConnectionManager.seednodes.test.ts | 25 +++-- tests/nodes/NodeManager.test.ts | 36 +++---- 8 files changed, 155 insertions(+), 111 deletions(-) diff --git a/src/network/Proxy.ts b/src/network/Proxy.ts index 973c7f525..dd71631bf 100644 --- a/src/network/Proxy.ts +++ b/src/network/Proxy.ts @@ -12,6 +12,8 @@ import type { NodeId } from '../nodes/types'; import type { Timer } from '../types'; import type UTPConnection from 'utp-native/lib/connection'; import type { ConnectionsReverse } from './ConnectionReverse'; +import type { PromiseCancellable } from '@matrixai/async-cancellable'; +import type { ContextTimed } from 'contexts/types'; import http from 'http'; import UTP from 'utp-native'; import Logger from '@matrixai/logger'; @@ -22,6 +24,7 @@ import ConnectionReverse from './ConnectionReverse'; import ConnectionForward from './ConnectionForward'; import * as networkUtils from './utils'; import * as networkErrors from './errors'; +import { context, timedCancellable } from '../contexts'; import * as nodesUtils from '../nodes/utils'; import { promisify, timerStart, timerStop } from '../utils'; @@ -314,15 +317,22 @@ class Proxy { * It will only stop the timer if using the default timer * Set timer to `null` explicitly to wait forever */ + public openConnectionForward( + nodeId: NodeId, + proxyHost: Host, + proxyPort: Port, + ctx?: Partial, + ): PromiseCancellable; + @timedCancellable(true, 20000) @ready(new networkErrors.ErrorProxyNotRunning(), true) public async openConnectionForward( nodeId: NodeId, proxyHost: Host, proxyPort: Port, - timer?: Timer, + @context ctx?: ContextTimed, ): Promise { - let timer_ = timer; - if (timer === undefined) { + let timer_: Timer | undefined; + if (ctx?.timer != null) { timer_ = timerStart(this.connConnectTime); } const proxyAddress = networkUtils.buildAddress(proxyHost, proxyPort); @@ -340,8 +350,8 @@ class Proxy { timer_, ); } finally { - if (timer === undefined) { - timerStop(timer_!); + if (timer_ != null) { + timerStop(timer_); } this.connectionLocksForward.delete(proxyAddress); } diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 1d7cd5f4b..7901ff319 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -14,6 +14,8 @@ import type { } from './types'; import type NodeManager from './NodeManager'; import type { TaskHandler, TaskHandlerId } from 'tasks/types'; +import type { ContextTimed } from 'contexts/types'; +import type { PromiseCancellable } from '@matrixai/async-cancellable'; import { withF } from '@matrixai/resources'; import Logger from '@matrixai/logger'; import { ready, StartStop } from '@matrixai/async-init/dist/StartStop'; @@ -23,6 +25,7 @@ import { LockBox, RWLockWriter } from '@matrixai/async-locks'; import NodeConnection from './NodeConnection'; import * as nodesUtils from './utils'; import * as nodesErrors from './errors'; +import { context, timedCancellable } from '../contexts'; import GRPCClientAgent from '../agent/GRPCClientAgent'; import * as validationUtils from '../validation/utils'; import * as networkUtils from '../network/utils'; @@ -80,18 +83,17 @@ class NodeConnectionManager { protected backoffDefault: number = 300; // 5 min protected backoffMultiplier: number = 2; // Doubles every failure - // TODO: make cancelable public readonly basePath = this.constructor.name; protected pingAndSetNodeHandler: TaskHandler = async ( - context, - taskInfo, + ctx, + _taskInfo, nodeIdEncoded: string, host: Host, port: Port, ) => { const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; const host_ = await networkUtils.resolveHost(host); - if (await this.pingNode(nodeId, host_, port)) { + if (await this.pingNode(nodeId, host_, port, ctx)) { await this.nodeManager!.setNode(nodeId, { host: host_, port }); } }; @@ -386,15 +388,15 @@ class NodeConnectionManager { * @param nodeId Node ID of the node we are connecting to * @param proxyHost Proxy host of the reverse proxy * @param proxyPort Proxy port of the reverse proxy - * @param timer Connection timeout timer + * @param ctx */ public async holePunchForward( nodeId: NodeId, proxyHost: Host, proxyPort: Port, - timer?: Timer, + ctx?: ContextTimed, ): Promise { - await this.proxy.openConnectionForward(nodeId, proxyHost, proxyPort, timer); + await this.proxy.openConnectionForward(nodeId, proxyHost, proxyPort, ctx); } /** @@ -402,15 +404,20 @@ class NodeConnectionManager { * proceeds to locate it using Kademlia. * @param targetNodeId Id of the node we are tying to find * @param ignoreRecentOffline skips nodes that are within their backoff period - * @param options + * @param ctx */ + public findNode( + targetNodeId: NodeId, + ignoreRecentOffline?: boolean, + ctx?: Partial, + ): PromiseCancellable; @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) + @timedCancellable(true, 20000) public async findNode( targetNodeId: NodeId, ignoreRecentOffline: boolean = false, - options: { signal?: AbortSignal } = {}, + @context ctx: ContextTimed, ): Promise { - const { signal } = { ...options }; // First check if we already have an existing ID -> address record let address = (await this.nodeGraph.getNode(targetNodeId))?.address; // Otherwise, attempt to locate it by contacting network @@ -419,10 +426,7 @@ class NodeConnectionManager { (await this.getClosestGlobalNodes( targetNodeId, ignoreRecentOffline, - undefined, - { - signal, - }, + ctx, )); // TODO: This currently just does one iteration return address; @@ -441,19 +445,22 @@ class NodeConnectionManager { * @param targetNodeId ID of the node attempting to be found (i.e. attempting * to find its IP address and port) * @param ignoreRecentOffline skips nodes that are within their backoff period - * @param timer Connection timeout timer - * @param options + * @param ctx * @returns whether the target node was located in the process */ + public getClosestGlobalNodes( + targetNodeId: NodeId, + ignoreRecentOffline?: boolean, + ctx?: Partial, + ): PromiseCancellable; @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) + @timedCancellable(true, 20000) public async getClosestGlobalNodes( targetNodeId: NodeId, ignoreRecentOffline: boolean = false, - timer?: Timer, - options: { signal?: AbortSignal } = {}, + @context ctx?: ContextTimed, ): Promise { const localNodeId = this.keyManager.getNodeId(); - const { signal } = { ...options }; // Let foundTarget: boolean = false; let foundAddress: NodeAddress | undefined = undefined; // Get the closest alpha nodes to the target node (set as shortlist) @@ -474,7 +481,7 @@ class NodeConnectionManager { const contacted: Set = new Set(); // Iterate until we've found and contacted k nodes while (contacted.size <= this.nodeGraph.nodeBucketLimit) { - if (signal?.aborted) throw signal.reason; + if (ctx!.signal?.aborted) return; // Remove the node from the front of the array const nextNode = shortlist.shift(); // If we have no nodes left in the shortlist, then stop @@ -492,8 +499,7 @@ class NodeConnectionManager { nextNodeId, nextNodeAddress.address.host, nextNodeAddress.address.port, - undefined, - { signal }, + ctx, ) ) { await this.nodeManager!.setNode(nextNodeId, nextNodeAddress.address); @@ -504,16 +510,24 @@ class NodeConnectionManager { } contacted[nextNodeId] = true; // Ask the node to get their own closest nodes to the target - const foundClosest = await this.getRemoteNodeClosestNodes( - nextNodeId, - targetNodeId, - timer, - ); + let foundClosest: Array<[NodeId, NodeData]>; + try { + foundClosest = await this.getRemoteNodeClosestNodes( + nextNodeId, + targetNodeId, + ctx!.timer.getTimeout() === Infinity + ? undefined + : timerStart(ctx!.timer.getTimeout()), + ); + } catch (e) { + if (e instanceof nodesErrors.ErrorNodeConnectionTimeout) return; + throw e; + } if (foundClosest.length === 0) continue; // Check to see if any of these are the target node. At the same time, add // them to the shortlist for (const [nodeId, nodeData] of foundClosest) { - if (signal?.aborted) throw signal.reason; + if (ctx!.signal?.aborted) return; // Ignore any nodes that have been contacted or our own node if (contacted[nodeId] || localNodeId.equals(nodeId)) { continue; @@ -524,8 +538,7 @@ class NodeConnectionManager { nodeId, nodeData.address.host, nodeData.address.port, - undefined, - { signal }, + ctx, )) ) { await this.nodeManager!.setNode(nodeId, nodeData.address); @@ -791,18 +804,22 @@ class NodeConnectionManager { * @param nodeId - NodeId of the target * @param host - Host of the target node * @param port - Port of the target node - * @param timer Connection timeout timer - * @param options + * @param ctx */ + public pingNode( + nodeId: NodeId, + host: Host | Hostname, + port: Port, + ctx?: Partial, + ): PromiseCancellable; @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) + @timedCancellable(true, 20000) public async pingNode( nodeId: NodeId, host: Host | Hostname, port: Port, - timer?: Timer, - options: { signal?: AbortSignal } = {}, + @context ctx: ContextTimed, ): Promise { - const { signal } = { ...options }; host = await networkUtils.resolveHost(host); // If we can create a connection then we have punched though the NAT, // authenticated and confirmed the nodeId matches @@ -823,16 +840,11 @@ class NodeConnectionManager { signature, ); }); - const forwardPunchPromise = this.holePunchForward( - nodeId, - host, - port, - timer, - ); + const forwardPunchPromise = this.holePunchForward(nodeId, host, port, ctx); const abortPromise = new Promise((_resolve, reject) => { - if (signal?.aborted) throw signal.reason; - signal?.addEventListener('abort', () => reject(signal.reason)); + if (ctx.signal.aborted) throw ctx.signal.reason; + ctx.signal.addEventListener('abort', () => reject(ctx.signal.reason)); }); try { diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 9b085307a..2306f026a 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -7,15 +7,17 @@ import type Sigchain from '../sigchain/Sigchain'; import type { ChainData, ChainDataEncoded } from '../sigchain/types'; import type { NodeId, NodeAddress, NodeBucket, NodeBucketIndex } from './types'; import type { ClaimEncoded } from '../claims/types'; -import type { Timer } from '../types'; import type TaskManager from '../tasks/TaskManager'; import type { TaskHandler, TaskHandlerId, Task } from '../tasks/types'; +import type { ContextTimed } from 'contexts/types'; +import type { PromiseCancellable } from '@matrixai/async-cancellable'; import Logger from '@matrixai/logger'; import { StartStop, ready } from '@matrixai/async-init/dist/StartStop'; import { Semaphore, Lock } from '@matrixai/async-locks'; import { IdInternal } from '@matrixai/id'; import * as nodesErrors from './errors'; import * as nodesUtils from './utils'; +import { timedCancellable, context } from '../contexts'; import * as networkUtils from '../network/utils'; import * as validationUtils from '../validation/utils'; import * as utilsPB from '../proto/js/polykey/v1/utils/utils_pb'; @@ -40,11 +42,11 @@ class NodeManager { public readonly basePath = this.constructor.name; private refreshBucketHandler: TaskHandler = async ( - context, - taskInfo, + ctx, + _taskInfo, bucketIndex, ) => { - await this.refreshBucket(bucketIndex, { signal: context.signal }); + await this.refreshBucket(bucketIndex, ctx); // When completed reschedule the task const spread = (Math.random() - 0.5) * @@ -57,6 +59,7 @@ class NodeManager { parameters: [bucketIndex], path: [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], priority: 0, + deadline: ctx.timer.delay, }); }; public readonly refreshBucketHandlerId = @@ -66,8 +69,7 @@ class NodeManager { _taskInfo, bucketIndex: number, ) => { - this.logger.info('RUNNING GARBAGE COLELCT'); - await this.garbageCollectBucket(bucketIndex, { signal: ctx.signal }); + await this.garbageCollectBucket(bucketIndex, ctx); }; public readonly gcBucketHandlerId = `${this.basePath}.${this.gcBucketHandler.name}` as TaskHandlerId; @@ -136,20 +138,24 @@ class NodeManager { * @return true if online, false if offline * @param nodeId - NodeId of the node we're pinging * @param address - Optional Host and Port we want to ping - * @param timer Connection timeout timer - * @param options + * @param ctx */ + public pingNode( + nodeId: NodeId, + address?: NodeAddress, + ctx?: Partial, + ): PromiseCancellable; + @timedCancellable(true, 20000) public async pingNode( nodeId: NodeId, address?: NodeAddress, - timer?: Timer, - options: { signal?: AbortSignal } = {}, + @context ctx?: ContextTimed, ): Promise { // We need to attempt a connection using the proxies // For now we will just do a forward connect + relay message const targetAddress = address ?? - (await this.nodeConnectionManager.findNode(nodeId, false, options)); + (await this.nodeConnectionManager.findNode(nodeId, false, ctx)); if (targetAddress == null) { throw new nodesErrors.ErrorNodeGraphNodeIdNotFound(); } @@ -158,7 +164,7 @@ class NodeManager { nodeId, targetHost, targetAddress.port, - timer, + ctx, ); } @@ -525,12 +531,15 @@ class NodeManager { } } + private garbageCollectBucket( + bucketIndex: number, + ctx?: Partial, + ): PromiseCancellable; + @timedCancellable(true, 20000) private async garbageCollectBucket( bucketIndex: number, - options: { signal?: AbortSignal } = {}, + @context ctx: ContextTimed, ): Promise { - const { signal } = { ...options }; - // This needs to: // 1. Iterate over every node within the bucket pinging K at a time // 2. remove any un-responsive nodes until there is room of all pending @@ -556,16 +565,12 @@ class NodeManager { for (const [nodeId, nodeData] of bucket) { if (removedNodes >= pendingNodes.size) break; await semaphore.waitForUnlock(); - if (signal?.aborted === true) break; + if (ctx.signal?.aborted === true) break; const [semaphoreReleaser] = await semaphore.lock()(); pendingPromises.push( (async () => { // Ping and remove or update node in bucket - if ( - await this.pingNode(nodeId, nodeData.address, undefined, { - signal, - }) - ) { + if (await this.pingNode(nodeId, nodeData.address, ctx)) { // Succeeded so update await this.setNode( nodeId, @@ -658,13 +663,17 @@ class NodeManager { * Connections during the search will will share node information with other * nodes. * @param bucketIndex - * @param options + * @param ctx */ + public refreshBucket( + bucketIndex: number, + ctx?: Partial, + ): PromiseCancellable; + @timedCancellable(true, 20000) public async refreshBucket( bucketIndex: NodeBucketIndex, - options: { signal?: AbortSignal } = {}, - ) { - const { signal } = { ...options }; + @context ctx: ContextTimed, + ): Promise { // We need to generate a random nodeId for this bucket const nodeId = this.keyManager.getNodeId(); const bucketRandomNodeId = nodesUtils.generateRandomNodeIdForBucket( @@ -672,9 +681,7 @@ class NodeManager { bucketIndex, ); // We then need to start a findNode procedure - await this.nodeConnectionManager.findNode(bucketRandomNodeId, true, { - signal, - }); + await this.nodeConnectionManager.findNode(bucketRandomNodeId, true, ctx); } private async setupRefreshBucketTasks(tran?: DBTransaction) { diff --git a/tests/network/Proxy.test.ts b/tests/network/Proxy.test.ts index 5bab753c4..d80881810 100644 --- a/tests/network/Proxy.test.ts +++ b/tests/network/Proxy.test.ts @@ -6,6 +6,7 @@ import http from 'http'; import tls from 'tls'; import UTP from 'utp-native'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; +import { Timer } from '@matrixai/timer'; import Proxy from '@/network/Proxy'; import * as networkUtils from '@/network/utils'; import * as networkErrors from '@/network/errors'; @@ -311,16 +312,16 @@ describe(Proxy.name, () => { ).rejects.toThrow(networkErrors.ErrorConnectionStartTimeout); expect(receivedCount).toBe(1); // Can override the timer - const timer = timerStart(2000); + const timer = new Timer({ delay: 1000 }); await expect(() => proxy.openConnectionForward( nodeIdABC, localHost, utpSocketHangPort as Port, - timer, + { timer }, ), ).rejects.toThrow(networkErrors.ErrorConnectionStartTimeout); - timerStop(timer); + timer.cancel('clean up'); expect(receivedCount).toBe(2); await expect(() => httpConnect( diff --git a/tests/nodes/NodeConnectionManager.general.test.ts b/tests/nodes/NodeConnectionManager.general.test.ts index a80e6b309..e2bd36605 100644 --- a/tests/nodes/NodeConnectionManager.general.test.ts +++ b/tests/nodes/NodeConnectionManager.general.test.ts @@ -8,6 +8,7 @@ import os from 'os'; import { DB } from '@matrixai/db'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { IdInternal } from '@matrixai/id'; +import { PromiseCancellable } from '@matrixai/async-cancellable'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; import NodeGraph from '@/nodes/NodeGraph'; @@ -271,7 +272,9 @@ describe(`${NodeConnectionManager.name} general test`, () => { NodeConnectionManager.prototype, 'pingNode', ); - mockedPingNode.mockImplementation(async () => true); + mockedPingNode.mockImplementation( + () => new PromiseCancellable((resolve) => resolve(true)), + ); // NodeConnectionManager under test const nodeConnectionManager = new NodeConnectionManager({ keyManager, @@ -553,7 +556,9 @@ describe(`${NodeConnectionManager.name} general test`, () => { }); // Making pings fail - mockedPingNode.mockImplementation(async () => false); + mockedPingNode.mockImplementation( + () => new PromiseCancellable((resolve) => resolve(false)), + ); await nodeConnectionManager.getClosestGlobalNodes(nodeId3, false); expect(mockedPingNode).toHaveBeenCalled(); diff --git a/tests/nodes/NodeConnectionManager.lifecycle.test.ts b/tests/nodes/NodeConnectionManager.lifecycle.test.ts index a904c7ef3..1c0792990 100644 --- a/tests/nodes/NodeConnectionManager.lifecycle.test.ts +++ b/tests/nodes/NodeConnectionManager.lifecycle.test.ts @@ -8,6 +8,7 @@ import { DB } from '@matrixai/db'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { withF } from '@matrixai/resources'; import { IdInternal } from '@matrixai/id'; +import { Timer } from '@matrixai/timer'; import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; @@ -18,7 +19,6 @@ import * as nodesUtils from '@/nodes/utils'; import * as nodesErrors from '@/nodes/errors'; import * as keysUtils from '@/keys/utils'; import * as grpcUtils from '@/grpc/utils'; -import { timerStart } from '@/utils'; import { globalRootKeyPems } from '../fixtures/globalRootKeyPems'; describe(`${NodeConnectionManager.name} lifecycle test`, () => { @@ -568,7 +568,7 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { remoteNodeId1, '127.1.2.3' as Host, 55555 as Port, - timerStart(1000), + { timer: new Timer({ delay: 1000 }) }, ), ).toEqual(false); } finally { @@ -593,7 +593,7 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { remoteNodeId1, remoteNode2.proxy.getProxyHost(), remoteNode2.proxy.getProxyPort(), - timerStart(1000), + { timer: new Timer({ delay: 1000 }) }, ), ).toEqual(false); @@ -602,7 +602,7 @@ describe(`${NodeConnectionManager.name} lifecycle test`, () => { remoteNodeId2, remoteNode1.proxy.getProxyHost(), remoteNode1.proxy.getProxyPort(), - timerStart(1000), + { timer: new Timer({ delay: 1000 }) }, ), ).toEqual(false); } finally { diff --git a/tests/nodes/NodeConnectionManager.seednodes.test.ts b/tests/nodes/NodeConnectionManager.seednodes.test.ts index b79964525..30ff0044e 100644 --- a/tests/nodes/NodeConnectionManager.seednodes.test.ts +++ b/tests/nodes/NodeConnectionManager.seednodes.test.ts @@ -7,6 +7,7 @@ import os from 'os'; import { DB } from '@matrixai/db'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { IdInternal } from '@matrixai/id'; +import { PromiseCancellable } from '@matrixai/async-cancellable'; import NodeManager from '@/nodes/NodeManager'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; @@ -83,6 +84,14 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { refreshBucketQueueAdd: jest.fn(), } as unknown as NodeManager; + function createPromiseCancellable(result: T) { + return () => new PromiseCancellable((resolve) => resolve(result)); + } + + function createPromiseCancellableNop() { + return () => new PromiseCancellable((resolve) => resolve()); + } + beforeAll(async () => { dataDir2 = await fs.promises.mkdtemp( path.join(os.tmpdir(), 'polykey-test-'), @@ -263,12 +272,12 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { NodeManager.prototype, 'refreshBucket', ); - mockedRefreshBucket.mockImplementation(async () => {}); + mockedRefreshBucket.mockImplementation(createPromiseCancellableNop()); const mockedPingNode = jest.spyOn( NodeConnectionManager.prototype, 'pingNode', ); - mockedPingNode.mockImplementation(async () => true); + mockedPingNode.mockImplementation(createPromiseCancellable(true)); try { const seedNodes: SeedNodes = {}; seedNodes[nodesUtils.encodeNodeId(remoteNodeId1)] = { @@ -325,12 +334,12 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { NodeManager.prototype, 'refreshBucket', ); - mockedRefreshBucket.mockImplementation(async () => {}); + mockedRefreshBucket.mockImplementation(createPromiseCancellableNop()); const mockedPingNode = jest.spyOn( NodeConnectionManager.prototype, 'pingNode', ); - mockedPingNode.mockImplementation(async () => true); + mockedPingNode.mockImplementation(createPromiseCancellable(true)); try { const seedNodes: SeedNodes = {}; seedNodes[nodesUtils.encodeNodeId(remoteNodeId1)] = { @@ -386,12 +395,12 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { NodeManager.prototype, 'refreshBucket', ); - mockedRefreshBucket.mockImplementation(async () => {}); + mockedRefreshBucket.mockImplementation(createPromiseCancellableNop()); const mockedPingNode = jest.spyOn( NodeConnectionManager.prototype, 'pingNode', ); - mockedPingNode.mockImplementation(async () => true); + mockedPingNode.mockImplementation(createPromiseCancellable(true)); try { const seedNodes: SeedNodes = {}; seedNodes[nodesUtils.encodeNodeId(remoteNodeId1)] = { @@ -468,7 +477,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { NodeConnectionManager.prototype, 'pingNode', ); - mockedPingNode.mockImplementation(async () => true); + mockedPingNode.mockImplementation(createPromiseCancellable(true)); try { node1 = await PolykeyAgent.createPolykeyAgent({ nodePath: path.join(dataDir, 'node1'), @@ -559,7 +568,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { NodeConnectionManager.prototype, 'pingNode', ); - mockedPingNode.mockImplementation(async () => true); + mockedPingNode.mockImplementation(createPromiseCancellable(true)); try { node1 = await PolykeyAgent.createPolykeyAgent({ nodePath: path.join(dataDir, 'node1'), diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index 8fc010ea9..768023875 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -8,6 +8,8 @@ import fs from 'fs'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; import { DB } from '@matrixai/db'; import UTP from 'utp-native'; +import { Timer } from '@matrixai/timer'; +import { PromiseCancellable } from '@matrixai/async-cancellable'; import TaskManager from '@/tasks/TaskManager'; import PolykeyAgent from '@/PolykeyAgent'; import KeyManager from '@/keys/KeyManager'; @@ -18,7 +20,7 @@ import NodeManager from '@/nodes/NodeManager'; import Proxy from '@/network/Proxy'; import Sigchain from '@/sigchain/Sigchain'; import * as claimsUtils from '@/claims/utils'; -import { never, promise, promisify, sleep, timerStart } from '@/utils'; +import { never, promise, promisify, sleep } from '@/utils'; import * as nodesUtils from '@/nodes/utils'; import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb'; import * as nodesTestUtils from './utils'; @@ -186,11 +188,9 @@ describe(`${NodeManager.name} test`, () => { await server.stop(); // Check if active // Case 1: cannot establish new connection, so offline - const active1 = await nodeManager.pingNode( - serverNodeId, - undefined, - timerStart(1000), - ); + const active1 = await nodeManager.pingNode(serverNodeId, undefined, { + timer: new Timer({ delay: 1000 }), + }); expect(active1).toBe(false); // Bring server node online await server.start({ @@ -207,22 +207,18 @@ describe(`${NodeManager.name} test`, () => { await nodeGraph.setNode(serverNodeId, serverNodeAddress); // Check if active // Case 2: can establish new connection, so online - const active2 = await nodeManager.pingNode( - serverNodeId, - undefined, - timerStart(1000), - ); + const active2 = await nodeManager.pingNode(serverNodeId, undefined, { + timer: new Timer({ delay: 1000 }), + }); expect(active2).toBe(true); // Turn server node offline again await server.stop(); await server.destroy(); // Check if active // Case 3: pre-existing connection no longer active, so offline - const active3 = await nodeManager.pingNode( - serverNodeId, - undefined, - timerStart(1000), - ); + const active3 = await nodeManager.pingNode(serverNodeId, undefined, { + timer: new Timer({ delay: 1000 }), + }); expect(active3).toBe(false); } finally { // Clean up @@ -927,7 +923,9 @@ describe(`${NodeManager.name} test`, () => { 'refreshBucket', ); try { - mockRefreshBucket.mockImplementation(async () => {}); + mockRefreshBucket.mockImplementation( + () => new PromiseCancellable((resolve) => resolve()), + ); await taskManager.startProcessing(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); @@ -1004,7 +1002,9 @@ describe(`${NodeManager.name} test`, () => { 'refreshBucket', ); try { - mockRefreshBucket.mockImplementation(async () => {}); + mockRefreshBucket.mockImplementation( + () => new PromiseCancellable((resolve) => resolve()), + ); await taskManager.startProcessing(); await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); From ad7c75177700c2a5a05188e1af3a5df48b7ff7cd Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 20:39:54 +1000 Subject: [PATCH 18/40] fix: depending on return from `updateTask` to check existence --- src/nodes/NodeManager.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 2306f026a..a44519b28 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -17,6 +17,7 @@ import { Semaphore, Lock } from '@matrixai/async-locks'; import { IdInternal } from '@matrixai/id'; import * as nodesErrors from './errors'; import * as nodesUtils from './utils'; +import * as tasksErrors from '../tasks/errors'; import { timedCancellable, context } from '../contexts'; import * as networkUtils from '../network/utils'; import * as validationUtils from '../validation/utils'; @@ -786,8 +787,6 @@ class NodeManager { count += 1; if (count <= 1) { foundTask = task; - // If already running then don't update - if (task.status !== 'scheduled') continue; // Update the first one // total delay is refreshBucketDelay + time since task creation // time since task creation = now - creation time; @@ -797,7 +796,15 @@ class NodeManager { task.created.getTime() + delay + spread; - await this.taskManager.updateTask(task.id, { delay: delayNew }, tran); + try { + await this.taskManager.updateTask(task.id, { delay: delayNew }); + } catch (e) { + if (e instanceof tasksErrors.ErrorTaskMissing) { + count -= 1; + } else if (!(e instanceof tasksErrors.ErrorTaskRunning)) { + throw e; + } + } this.logger.debug( `Updating refreshBucket task for bucket ${bucketIndex}`, ); From 81dbac65cae70460a93d9141a4b086ee81907aad Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 20:53:12 +1000 Subject: [PATCH 19/40] fix: replacing `private` with `protected` --- src/nodes/NodeManager.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index a44519b28..0226e7de2 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -42,7 +42,7 @@ class NodeManager { protected pendingNodes: Map> = new Map(); public readonly basePath = this.constructor.name; - private refreshBucketHandler: TaskHandler = async ( + protected refreshBucketHandler: TaskHandler = async ( ctx, _taskInfo, bucketIndex, @@ -65,7 +65,7 @@ class NodeManager { }; public readonly refreshBucketHandlerId = `${this.basePath}.${this.refreshBucketHandler.name}` as TaskHandlerId; - private gcBucketHandler: TaskHandler = async ( + protected gcBucketHandler: TaskHandler = async ( ctx, _taskInfo, bucketIndex: number, @@ -532,12 +532,12 @@ class NodeManager { } } - private garbageCollectBucket( + protected garbageCollectBucket( bucketIndex: number, ctx?: Partial, ): PromiseCancellable; @timedCancellable(true, 20000) - private async garbageCollectBucket( + protected async garbageCollectBucket( bucketIndex: number, @context ctx: ContextTimed, ): Promise { @@ -685,7 +685,7 @@ class NodeManager { await this.nodeConnectionManager.findNode(bucketRandomNodeId, true, ctx); } - private async setupRefreshBucketTasks(tran?: DBTransaction) { + protected async setupRefreshBucketTasks(tran?: DBTransaction) { if (tran == null) { return this.db.withTransactionF((tran) => this.setupRefreshBucketTasks(tran), From 66181b494a7c6d08c8cef46b19ed6606bdaca185 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 20:58:52 +1000 Subject: [PATCH 20/40] fix: removing `Queue.ts` --- src/nodes/Queue.ts | 91 ---------------------------------------------- 1 file changed, 91 deletions(-) delete mode 100644 src/nodes/Queue.ts diff --git a/src/nodes/Queue.ts b/src/nodes/Queue.ts deleted file mode 100644 index ed2eaa06e..000000000 --- a/src/nodes/Queue.ts +++ /dev/null @@ -1,91 +0,0 @@ -import type { PromiseDeconstructed } from '../types'; -import Logger from '@matrixai/logger'; -import { StartStop, ready } from '@matrixai/async-init/dist/StartStop'; -import * as nodesErrors from './errors'; -import { promise } from '../utils'; - -interface Queue extends StartStop {} -@StartStop() -class Queue { - protected logger: Logger; - protected end: boolean = false; - protected queue: Array<() => Promise> = []; - protected runner: Promise; - protected plug_: PromiseDeconstructed = promise(); - protected drained_: PromiseDeconstructed = promise(); - - constructor({ logger }: { logger?: Logger }) { - this.logger = logger ?? new Logger(this.constructor.name); - } - - public async start() { - this.logger.info(`Starting ${this.constructor.name}`); - const start = async () => { - this.logger.debug('Starting queue'); - this.plug(); - const pace = async () => { - await this.plug_.p; - return !this.end; - }; - // While queue hasn't ended - while (await pace()) { - const job = this.queue.shift(); - if (job == null) { - // If the queue is empty then we pause the queue - this.plug(); - continue; - } - try { - await job(); - } catch (e) { - if (!(e instanceof nodesErrors.ErrorNodeGraphSameNodeId)) throw e; - } - } - this.logger.debug('queue has ended'); - }; - this.runner = start(); - this.logger.info(`Started ${this.constructor.name}`); - } - - public async stop() { - this.logger.info(`Stopping ${this.constructor.name}`); - this.logger.debug('Stopping queue'); - // Tell the queue runner to end - this.end = true; - this.unplug(); - // Wait for runner to finish it's current job - await this.runner; - this.logger.info(`Stopped ${this.constructor.name}`); - } - - /** - * This adds a setNode operation to the queue - */ - public push(f: () => Promise): void { - this.queue.push(f); - this.unplug(); - } - - @ready(new nodesErrors.ErrorQueueNotRunning()) - public async drained(): Promise { - await this.drained_.p; - } - - private plug(): void { - this.logger.debug('Plugging queue'); - // Pausing queue - this.plug_ = promise(); - // Signaling queue is empty - this.drained_.resolveP(); - } - - private unplug(): void { - this.logger.debug('Unplugging queue'); - // Starting queue - this.plug_.resolveP(); - // Signalling queue is running - this.drained_ = promise(); - } -} - -export default Queue; From cd47c7476dff02614de1f792773350d6ac1ac50f Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 21:07:34 +1000 Subject: [PATCH 21/40] fix: removing `@ready` from `TaskManager.cancelTask` --- src/tasks/TaskManager.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tasks/TaskManager.ts b/src/tasks/TaskManager.ts index 9e3da47d5..6dc221def 100644 --- a/src/tasks/TaskManager.ts +++ b/src/tasks/TaskManager.ts @@ -1148,7 +1148,6 @@ class TaskManager { this.logger.debug(`Requeued Task ${taskIdEncoded}`); } - @ready(new tasksErrors.ErrorTaskManagerNotRunning()) protected async cancelTask(taskId: TaskId, cancelReason: any): Promise { const taskIdEncoded = tasksUtils.encodeTaskId(taskId); this.logger.debug(`Cancelling Task ${taskIdEncoded}`); From 34c658e0a3fafdcd34d19d9ccbccc2bc48a2a6da Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 19 Sep 2022 21:20:21 +1000 Subject: [PATCH 22/40] docs: adding description to `isConnectionError` --- src/nodes/utils.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/nodes/utils.ts b/src/nodes/utils.ts index 544b7bc55..4d94f04f7 100644 --- a/src/nodes/utils.ts +++ b/src/nodes/utils.ts @@ -313,6 +313,16 @@ function generateRandomNodeIdForBucket( return xOrNodeId(nodeId, randomDistanceForBucket); } +/** + * This is used to check if the given error is the result of a connection failure. + * Connection failures can happen due to the following. + * Failure to establish a connection, + * an existing connection fails, + * the GRPC client has been destroyed, + * or the NodeConnection has been destroyed. + * This is generally used to check the connection has failed + * before cleaning it up. + */ function isConnectionError(e): boolean { return ( e instanceof nodesErrors.ErrorNodeConnectionDestroyed || From b9d248b6e6b3dbec18b7df249ffa269b4a4b69fb Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 20 Sep 2022 13:09:47 +1000 Subject: [PATCH 23/40] fix: added cancellability and blocking to `nodeManager.setNode` --- src/client/service/nodesAdd.ts | 1 + src/network/Proxy.ts | 6 +- src/nodes/NodeManager.ts | 184 +++++++++++++++++++++----------- tests/nodes/NodeManager.test.ts | 59 +++------- 4 files changed, 139 insertions(+), 111 deletions(-) diff --git a/src/client/service/nodesAdd.ts b/src/client/service/nodesAdd.ts index 64e1cc34a..87b356b7f 100644 --- a/src/client/service/nodesAdd.ts +++ b/src/client/service/nodesAdd.ts @@ -79,6 +79,7 @@ function nodesAdd({ host, port, } as NodeAddress, + true, request.getForce(), undefined, tran, diff --git a/src/network/Proxy.ts b/src/network/Proxy.ts index dd71631bf..ab15f9dd1 100644 --- a/src/network/Proxy.ts +++ b/src/network/Proxy.ts @@ -331,10 +331,8 @@ class Proxy { proxyPort: Port, @context ctx?: ContextTimed, ): Promise { - let timer_: Timer | undefined; - if (ctx?.timer != null) { - timer_ = timerStart(this.connConnectTime); - } + const timerDelay = ctx?.timer.getTimeout() ?? this.connConnectTime; + const timer_: Timer = timerStart(timerDelay); const proxyAddress = networkUtils.buildAddress(proxyHost, proxyPort); let lock = this.connectionLocksForward.get(proxyAddress); if (lock == null) { diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 0226e7de2..da0827c75 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -71,6 +71,13 @@ class NodeManager { bucketIndex: number, ) => { await this.garbageCollectBucket(bucketIndex, ctx); + // Checking for any new pending tasks + const pendingNodesRemaining = this.pendingNodes.get(bucketIndex); + if (pendingNodesRemaining == null || pendingNodesRemaining.size === 0) { + return; + } + // Re-schedule the task + await this.setupGCTask(bucketIndex); }; public readonly gcBucketHandlerId = `${this.basePath}.${this.gcBucketHandler.name}` as TaskHandlerId; @@ -438,24 +445,34 @@ class NodeManager { ); } - // FIXME: make cancelable /** * Adds a node to the node graph. This assumes that you have already authenticated the node * Updates the node if the node already exists * This operation is blocking by default - set `block` 2qto false to make it non-blocking * @param nodeId - Id of the node we wish to add * @param nodeAddress - Expected address of the node we want to add + * @param block - When true it will wait for any garbage collection to finish before returning. * @param force - Flag for if we want to add the node without authenticating or if the bucket is full. * This will drop the oldest node in favor of the new. - * @param timeout Connection timeout + * @param ctx * @param tran */ + public setNode( + nodeId: NodeId, + nodeAddress: NodeAddress, + block?: boolean, + force?: boolean, + ctx?: Partial, + tran?: DBTransaction, + ): PromiseCancellable; @ready(new nodesErrors.ErrorNodeManagerNotRunning()) + @timedCancellable(true, 20000) public async setNode( nodeId: NodeId, nodeAddress: NodeAddress, + block: boolean = false, force: boolean = false, - timeout?: number, + @context ctx?: ContextTimed, tran?: DBTransaction, ): Promise { // We don't want to add our own node @@ -466,7 +483,7 @@ class NodeManager { if (tran == null) { return this.db.withTransactionF((tran) => - this.setNode(nodeId, nodeAddress, force, timeout, tran), + this.setNode(nodeId, nodeAddress, block, force, ctx, tran), ); } @@ -528,19 +545,34 @@ class NodeManager { )} to pending list`, ); // Add the node to the pending nodes list - await this.addPendingNode(bucketIndex, nodeId, nodeAddress); + await this.addPendingNode( + bucketIndex, + nodeId, + nodeAddress, + block, + ctx, + tran, + ); } } protected garbageCollectBucket( bucketIndex: number, ctx?: Partial, + tran?: DBTransaction, ): PromiseCancellable; @timedCancellable(true, 20000) protected async garbageCollectBucket( bucketIndex: number, @context ctx: ContextTimed, + tran?: DBTransaction, ): Promise { + if (tran == null) { + return this.db.withTransactionF((tran) => + this.garbageCollectBucket(bucketIndex, ctx, tran), + ); + } + // This needs to: // 1. Iterate over every node within the bucket pinging K at a time // 2. remove any un-responsive nodes until there is room of all pending @@ -552,63 +584,65 @@ class NodeManager { // No nodes mean nothing to do if (pendingNodes == null || pendingNodes.size === 0) return; this.pendingNodes.set(bucketIndex, new Map()); - await this.db.withTransactionF(async (tran) => { - // Locking on bucket - await this.nodeGraph.lockBucket(bucketIndex, tran); - const semaphore = new Semaphore(3); - - // Iterating over existing nodes - const bucket = await this.getBucket(bucketIndex, tran); - if (bucket == null) never(); - let removedNodes = 0; - const unsetLock = new Lock(); - const pendingPromises: Array> = []; - for (const [nodeId, nodeData] of bucket) { - if (removedNodes >= pendingNodes.size) break; - await semaphore.waitForUnlock(); - if (ctx.signal?.aborted === true) break; - const [semaphoreReleaser] = await semaphore.lock()(); - pendingPromises.push( - (async () => { - // Ping and remove or update node in bucket - if (await this.pingNode(nodeId, nodeData.address, ctx)) { - // Succeeded so update - await this.setNode( - nodeId, - nodeData.address, - false, - undefined, - tran, - ); - } else { - // We need to lock this since it's concurrent - // and shares the transaction - await unsetLock.withF(async () => { - await this.unsetNode(nodeId, tran); - removedNodes += 1; - }); - } - // Releasing semaphore - await semaphoreReleaser(); - })(), - ); - } - // Wait for pending pings to complete - await Promise.all(pendingPromises); - // Fill in bucket with pending nodes - for (const [nodeIdString, address] of pendingNodes) { - if (removedNodes <= 0) break; - const nodeId = IdInternal.fromString(nodeIdString); - await this.setNode(nodeId, address, false, undefined, tran); - removedNodes -= 1; - } - }); + // Locking on bucket + await this.nodeGraph.lockBucket(bucketIndex, tran); + const semaphore = new Semaphore(3); + + // Iterating over existing nodes + const bucket = await this.getBucket(bucketIndex, tran); + if (bucket == null) never(); + let removedNodes = 0; + const unsetLock = new Lock(); + const pendingPromises: Array> = []; + for (const [nodeId, nodeData] of bucket) { + if (removedNodes >= pendingNodes.size) break; + await semaphore.waitForUnlock(); + if (ctx.signal?.aborted === true) break; + const [semaphoreReleaser] = await semaphore.lock()(); + pendingPromises.push( + (async () => { + // Ping and remove or update node in bucket + if (await this.pingNode(nodeId, nodeData.address, ctx)) { + // Succeeded so update + await this.setNode( + nodeId, + nodeData.address, + false, + false, + undefined, + tran, + ); + } else { + // We need to lock this since it's concurrent + // and shares the transaction + await unsetLock.withF(async () => { + await this.unsetNode(nodeId, tran); + removedNodes += 1; + }); + } + // Releasing semaphore + await semaphoreReleaser(); + })(), + ); + } + // Wait for pending pings to complete + await Promise.all(pendingPromises); + // Fill in bucket with pending nodes + for (const [nodeIdString, address] of pendingNodes) { + if (removedNodes <= 0) break; + const nodeId = IdInternal.fromString(nodeIdString); + await this.setNode(nodeId, address, false, false, undefined, tran); + removedNodes -= 1; + } } protected async addPendingNode( bucketIndex: number, nodeId: NodeId, nodeAddress: NodeAddress, + block: boolean = false, + ctx?: ContextTimed, + tran?: DBTransaction, ): Promise { if (!this.pendingNodes.has(bucketIndex)) { this.pendingNodes.set(bucketIndex, new Map()); @@ -617,22 +651,44 @@ class NodeManager { pendingNodes!.set(nodeId.toString(), nodeAddress); // No need to re-set it in the map, Maps are by reference + // If set to blocking we just run the GC operation here + // without setting up a new task + if (block) { + await this.garbageCollectBucket(bucketIndex, ctx, tran); + return; + } + await this.setupGCTask(bucketIndex); + } + + protected async setupGCTask(bucketIndex: number) { // Check and start a 'garbageCollect` bucket task - let first: boolean = true; + let scheduled: boolean = false; for await (const task of this.taskManager.getTasks('asc', true, [ this.basePath, this.gcBucketHandlerId, `${bucketIndex}`, ])) { - if (first) { - // Just ignore it. - first = false; - continue; + switch (task.status) { + case 'queued': + case 'active': + // Ignore active tasks + break; + case 'scheduled': + { + if (scheduled) { + // Duplicate scheduled are removed + task.cancel('Removing extra scheduled task'); + break; + } + scheduled = true; + } + break; + default: + task.cancel('Removing extra task'); + break; } - // There shouldn't be duplicates, we'll remove extra - task.cancel('Removing extra task'); } - if (first) { + if (!scheduled) { // If none were found, schedule a new one await this.taskManager.scheduleTask({ handlerId: this.gcBucketHandlerId, diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index 768023875..e5e1166d3 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -553,6 +553,7 @@ describe(`${NodeManager.name} test`, () => { try { await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; // Creating 20 nodes in bucket @@ -575,16 +576,7 @@ describe(`${NodeManager.name} test`, () => { // Waiting for a second to tick over await sleep(1500); // Adding a new node with bucket full - await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress); - const tasks: Array> = []; - for await (const task of taskManager.getTasks('asc', false, [ - nodeManager.basePath, - nodeManager.gcBucketHandlerId, - ])) { - tasks.push(task.promise()); - } - await taskManager.startProcessing(); - await Promise.all(tasks); + await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress, true); // Bucket still contains max nodes const bucket = await nodeManager.getBucket(bucketIndex); expect(bucket).toHaveLength(nodeGraph.nodeBucketLimit); @@ -633,7 +625,12 @@ describe(`${NodeManager.name} test`, () => { nodeManagerPingMock.mockResolvedValue(true); const oldestNodeId = (await nodeGraph.getOldestNode(bucketIndex)).pop(); // Adding a new node with bucket full - await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress, true); + await nodeManager.setNode( + nodeId, + { port: 55555 } as NodeAddress, + undefined, + true, + ); // Bucket still contains max nodes const bucket = await nodeManager.getBucket(bucketIndex); expect(bucket).toHaveLength(nodeGraph.nodeBucketLimit); @@ -661,6 +658,7 @@ describe(`${NodeManager.name} test`, () => { try { await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const localNodeId = keyManager.getNodeId(); const bucketIndex = 100; // Creating 20 nodes in bucket @@ -681,16 +679,7 @@ describe(`${NodeManager.name} test`, () => { nodeManagerPingMock.mockResolvedValue(false); const oldestNodeId = (await nodeGraph.getOldestNode(bucketIndex)).pop(); // Adding a new node with bucket full - await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress); - const tasks: Array> = []; - for await (const task of taskManager.getTasks('asc', false, [ - nodeManager.basePath, - nodeManager.gcBucketHandlerId, - ])) { - tasks.push(task.promise()); - } - await taskManager.startProcessing(); - await Promise.all(tasks); + await nodeManager.setNode(nodeId, { port: 55555 } as NodeAddress, true); // New node was added const node = await nodeGraph.getNode(nodeId); expect(node).toBeDefined(); @@ -773,6 +762,7 @@ describe(`${NodeManager.name} test`, () => { try { await nodeManager.start(); await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -790,16 +780,7 @@ describe(`${NodeManager.name} test`, () => { // Pings succeed, node not added mockedPingNode.mockImplementation(async () => true); const newNode = generateNodeIdForBucket(nodeId, 100, 21); - await nodeManager.setNode(newNode, address); - const tasks: Array> = []; - for await (const task of taskManager.getTasks('asc', false, [ - nodeManager.basePath, - nodeManager.gcBucketHandlerId, - ])) { - tasks.push(task.promise()); - } - await taskManager.startProcessing(); - await Promise.all(tasks); + await nodeManager.setNode(newNode, address, true); expect(await listBucket(100)).not.toContain( nodesUtils.encodeNodeId(newNode), ); @@ -821,6 +802,7 @@ describe(`${NodeManager.name} test`, () => { await nodeManager.start(); try { await nodeConnectionManager.start({ nodeManager }); + await taskManager.startProcessing(); const nodeId = keyManager.getNodeId(); const address = { host: localhost, port }; // Let's fill a bucket @@ -840,18 +822,9 @@ describe(`${NodeManager.name} test`, () => { const newNode1 = generateNodeIdForBucket(nodeId, 100, 22); const newNode2 = generateNodeIdForBucket(nodeId, 100, 23); const newNode3 = generateNodeIdForBucket(nodeId, 100, 24); - await nodeManager.setNode(newNode1, address); - await nodeManager.setNode(newNode2, address); - await nodeManager.setNode(newNode3, address); - const tasks: Array> = []; - for await (const task of taskManager.getTasks('asc', false, [ - nodeManager.basePath, - nodeManager.gcBucketHandlerId, - ])) { - tasks.push(task.promise()); - } - await taskManager.startProcessing(); - await Promise.all(tasks); + await nodeManager.setNode(newNode1, address, true); + await nodeManager.setNode(newNode2, address, true); + await nodeManager.setNode(newNode3, address, true); const list = await listBucket(100); expect(list).toContain(nodesUtils.encodeNodeId(newNode1)); expect(list).toContain(nodesUtils.encodeNodeId(newNode2)); From fe817a2ee2a3bb9d83df2aea00c1b171cbd83c48 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 20 Sep 2022 13:24:58 +1000 Subject: [PATCH 24/40] fix: extracted `refreshBucketsDelayJitter` into nodes utils --- src/nodes/NodeManager.ts | 49 ++++++++++++++++++++-------------------- src/nodes/utils.ts | 15 ++++++++++++ 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index da0827c75..98652ca97 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -38,7 +38,7 @@ class NodeManager { protected nodeGraph: NodeGraph; protected taskManager: TaskManager; protected refreshBucketDelay: number; - protected refreshBucketDelaySpread: number; + protected refreshBucketDelayJitter: number; protected pendingNodes: Map> = new Map(); public readonly basePath = this.constructor.name; @@ -49,12 +49,12 @@ class NodeManager { ) => { await this.refreshBucket(bucketIndex, ctx); // When completed reschedule the task - const spread = - (Math.random() - 0.5) * - this.refreshBucketDelay * - this.refreshBucketDelaySpread; + const jitter = nodesUtils.refreshBucketsDelayJitter( + this.refreshBucketDelay, + this.refreshBucketDelayJitter, + ); await this.taskManager.scheduleTask({ - delay: this.refreshBucketDelay + spread, + delay: this.refreshBucketDelay + jitter, handlerId: this.refreshBucketHandlerId, lazy: true, parameters: [bucketIndex], @@ -90,7 +90,7 @@ class NodeManager { nodeGraph, taskManager, refreshBucketDelay = 3600000, // 1 hour in milliseconds - refreshBucketDelaySpread = 0.5, // Multiple of refreshBucketDelay to spread by + refreshBucketDelayJitter = 0.5, // Multiple of refreshBucketDelay to jitter by logger, }: { db: DB; @@ -100,7 +100,7 @@ class NodeManager { nodeGraph: NodeGraph; taskManager: TaskManager; refreshBucketDelay?: number; - refreshBucketDelaySpread?: number; + refreshBucketDelayJitter?: number; logger?: Logger; }) { this.logger = logger ?? new Logger(this.constructor.name); @@ -112,9 +112,9 @@ class NodeManager { this.taskManager = taskManager; this.refreshBucketDelay = refreshBucketDelay; // Clamped from 0 to 1 inclusive - this.refreshBucketDelaySpread = Math.max( + this.refreshBucketDelayJitter = Math.max( 0, - Math.min(refreshBucketDelaySpread, 1), + Math.min(refreshBucketDelayJitter, 1), ); } @@ -764,16 +764,15 @@ class NodeManager { // If it's scheduled then reset delay existingTasks[bucketIndex] = true; // Total delay is refreshBucketDelay + time since task creation - const spread = - (Math.random() - 0.5) * - this.refreshBucketDelay * - this.refreshBucketDelaySpread; const delay = performance.now() + performance.timeOrigin - task.created.getTime() + this.refreshBucketDelay + - spread; + nodesUtils.refreshBucketsDelayJitter( + this.refreshBucketDelay, + this.refreshBucketDelayJitter, + ); await this.taskManager.updateTask(task.id, { delay }, tran); } break; @@ -800,13 +799,13 @@ class NodeManager { this.logger.debug( `Creating refreshBucket task for bucket ${bucketIndex}`, ); - const spread = - (Math.random() - 0.5) * - this.refreshBucketDelay * - this.refreshBucketDelaySpread; + const jitter = nodesUtils.refreshBucketsDelayJitter( + this.refreshBucketDelay, + this.refreshBucketDelayJitter, + ); await this.taskManager.scheduleTask({ handlerId: this.refreshBucketHandlerId, - delay: this.refreshBucketDelay + spread, + delay: this.refreshBucketDelay + jitter, lazy: true, parameters: [bucketIndex], path: [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], @@ -830,8 +829,10 @@ class NodeManager { ); } - const spread = - (Math.random() - 0.5) * delay * this.refreshBucketDelaySpread; + const jitter = nodesUtils.refreshBucketsDelayJitter( + delay, + this.refreshBucketDelayJitter, + ); let foundTask: Task | undefined; let count = 0; for await (const task of this.taskManager.getTasks( @@ -851,7 +852,7 @@ class NodeManager { performance.timeOrigin - task.created.getTime() + delay + - spread; + jitter; try { await this.taskManager.updateTask(task.id, { delay: delayNew }); } catch (e) { @@ -878,7 +879,7 @@ class NodeManager { `No refreshBucket task for bucket ${bucketIndex}, new one was created`, ); foundTask = await this.taskManager.scheduleTask({ - delay: delay + spread, + delay: delay + jitter, handlerId: this.refreshBucketHandlerId, lazy: true, parameters: [bucketIndex], diff --git a/src/nodes/utils.ts b/src/nodes/utils.ts index 4d94f04f7..f1c43b658 100644 --- a/src/nodes/utils.ts +++ b/src/nodes/utils.ts @@ -331,6 +331,20 @@ function isConnectionError(e): boolean { ); } +/** + * This generates a random delay based on the given delay and jitter multiplier. + * For example, a delay of 100 and multiplier of 0.5 would result in a delay + * randomly between 50 and 150. + * @param delay - base delay to 'jitter' around + * @param jitterMultiplier - jitter amount as a multiple of the delay + */ +function refreshBucketsDelayJitter( + delay: number, + jitterMultiplier: number, +): number { + return (Math.random() - 0.5) * delay * jitterMultiplier; +} + export { sepBuffer, encodeNodeId, @@ -352,4 +366,5 @@ export { xOrNodeId, generateRandomNodeIdForBucket, isConnectionError, + refreshBucketsDelayJitter, }; From ca2c966669719ca291d9766cbd5c498ca14a8060 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 20 Sep 2022 14:26:20 +1000 Subject: [PATCH 25/40] fix: fixes to errors and adding un-recoverable error handlers Un-recoverable errors include `ErrorBinUncaughtException`, `ErrorBinUnhandledRejection` and `ErrorBinAsynchronousDeadlock`. #414 --- src/bin/errors.ts | 39 ++++++++++----- src/bin/nodes/CommandFind.ts | 2 +- src/bin/nodes/CommandPing.ts | 4 +- src/bin/utils/ExitHandlers.ts | 94 ++++++++++++++++++++--------------- src/errors.ts | 6 --- 5 files changed, 85 insertions(+), 60 deletions(-) diff --git a/src/bin/errors.ts b/src/bin/errors.ts index 576fa21a6..34e76e41d 100644 --- a/src/bin/errors.ts +++ b/src/bin/errors.ts @@ -1,7 +1,25 @@ import ErrorPolykey from '../ErrorPolykey'; import sysexits from '../utils/sysexits'; -class ErrorCLI extends ErrorPolykey {} +class ErrorBin extends ErrorPolykey {} + +class ErrorBinUncaughtException extends ErrorBin { + static description = ''; + exitCode = sysexits.SOFTWARE; +} + +class ErrorBinUnhandledRejection extends ErrorBin { + static description = ''; + exitCode = sysexits.SOFTWARE; +} + +class ErrorBinAsynchronousDeadlock extends ErrorBin { + static description = + 'PolykeyAgent process exited unexpectedly, likely due to promise deadlock'; + exitCode = sysexits.SOFTWARE; +} + +class ErrorCLI extends ErrorBin {} class ErrorCLINodePath extends ErrorCLI { static description = 'Cannot derive default node path from unknown platform'; @@ -49,23 +67,21 @@ class ErrorCLIPolykeyAgentProcess extends ErrorCLI { exitCode = sysexits.OSERR; } -class ErrorCLIPolykeyAsynchronousDeadlock extends ErrorCLI { - static description = - 'PolykeyAgent process exited unexpectedly, likely due to promise deadlock'; - exitCode = sysexits.SOFTWARE; -} - -class ErrorNodeFindFailed extends ErrorCLI { +class ErrorCLINodeFindFailed extends ErrorCLI { static description = 'Failed to find the node in the DHT'; exitCode = 1; } -class ErrorNodePingFailed extends ErrorCLI { +class ErrorCLINodePingFailed extends ErrorCLI { static description = 'Node was not online or not found.'; exitCode = 1; } export { + ErrorBin, + ErrorBinUncaughtException, + ErrorBinUnhandledRejection, + ErrorBinAsynchronousDeadlock, ErrorCLI, ErrorCLINodePath, ErrorCLIClientOptions, @@ -76,7 +92,6 @@ export { ErrorCLIFileRead, ErrorCLIPolykeyAgentStatus, ErrorCLIPolykeyAgentProcess, - ErrorCLIPolykeyAsynchronousDeadlock, - ErrorNodeFindFailed, - ErrorNodePingFailed, + ErrorCLINodeFindFailed, + ErrorCLINodePingFailed, }; diff --git a/src/bin/nodes/CommandFind.ts b/src/bin/nodes/CommandFind.ts index 32169a968..92b2900c1 100644 --- a/src/bin/nodes/CommandFind.ts +++ b/src/bin/nodes/CommandFind.ts @@ -93,7 +93,7 @@ class CommandFind extends CommandPolykey { ); // Like ping it should error when failing to find node for automation reasons. if (!result.success) { - throw new binErrors.ErrorNodeFindFailed(result.message); + throw new binErrors.ErrorCLINodeFindFailed(result.message); } } finally { if (pkClient! != null) await pkClient.stop(); diff --git a/src/bin/nodes/CommandPing.ts b/src/bin/nodes/CommandPing.ts index a15779c55..c9816ad18 100644 --- a/src/bin/nodes/CommandPing.ts +++ b/src/bin/nodes/CommandPing.ts @@ -56,7 +56,7 @@ class CommandPing extends CommandPolykey { ); } catch (err) { if (err.cause instanceof nodesErrors.ErrorNodeGraphNodeIdNotFound) { - error = new binErrors.ErrorNodePingFailed( + error = new binErrors.ErrorCLINodePingFailed( `Failed to resolve node ID ${nodesUtils.encodeNodeId( nodeId, )} to an address.`, @@ -69,7 +69,7 @@ class CommandPing extends CommandPolykey { const status = { success: false, message: '' }; status.success = statusMessage ? statusMessage.getSuccess() : false; if (!status.success && !error) { - error = new binErrors.ErrorNodePingFailed('No response received'); + error = new binErrors.ErrorCLINodePingFailed('No response received'); } if (status.success) status.message = 'Node is Active.'; else status.message = error.message; diff --git a/src/bin/utils/ExitHandlers.ts b/src/bin/utils/ExitHandlers.ts index 24fa27871..a9abcfaff 100644 --- a/src/bin/utils/ExitHandlers.ts +++ b/src/bin/utils/ExitHandlers.ts @@ -1,7 +1,7 @@ import process from 'process'; import * as binUtils from './utils'; import ErrorPolykey from '../../ErrorPolykey'; -import * as CLIErrors from '../errors'; +import * as binErrors from '../errors'; class ExitHandlers { /** @@ -11,38 +11,6 @@ class ExitHandlers { public handlers: Array<(signal?: NodeJS.Signals) => Promise>; protected _exiting: boolean = false; protected _errFormat: 'json' | 'error'; - /** - * Handles synchronous and asynchronous exceptions - * This prints out appropriate error message on STDERR - * It sets the exit code according to the error - * 255 is set for unknown errors - */ - protected errorHandler = async (e: Error) => { - if (this._exiting) { - return; - } - this._exiting = true; - if (e instanceof ErrorPolykey) { - process.stderr.write( - binUtils.outputFormatter({ - type: this._errFormat, - data: e, - }), - ); - process.exitCode = e.exitCode; - } else { - // Unknown error, this should not happen - process.stderr.write( - binUtils.outputFormatter({ - type: this._errFormat, - data: e, - }), - ); - process.exitCode = 255; - } - // Fail fast pattern - process.exit(); - }; /** * Handles termination signals * This is idempotent @@ -84,10 +52,55 @@ class ExitHandlers { process.kill(process.pid, signal); } }; - + /** + * Handles asynchronous exceptions + * This prints out appropriate error message on STDERR + * It sets the exit code to SOFTWARE + */ + protected unhandledRejectionHandler = async (e: Error) => { + if (this._exiting) { + return; + } + this._exiting = true; + const error = new binErrors.ErrorBinUnhandledRejection(undefined, { + cause: e, + }); + process.stderr.write( + binUtils.outputFormatter({ + type: this._errFormat, + data: e, + }), + ); + process.exitCode = error.exitCode; + // Fail fast pattern + process.exit(); + }; + /** + * Handles synchronous exceptions + * This prints out appropriate error message on STDERR + * It sets the exit code to SOFTWARE + */ + protected uncaughtExceptionHandler = async (e: Error) => { + if (this._exiting) { + return; + } + this._exiting = true; + const error = new binErrors.ErrorBinUncaughtException(undefined, { + cause: e, + }); + process.stderr.write( + binUtils.outputFormatter({ + type: this._errFormat, + data: e, + }), + ); + process.exitCode = error.exitCode; + // Fail fast pattern + process.exit(); + }; protected deadlockHandler = async () => { if (process.exitCode == null) { - const e = new CLIErrors.ErrorCLIPolykeyAsynchronousDeadlock(); + const e = new binErrors.ErrorBinAsynchronousDeadlock(); process.stderr.write( binUtils.outputFormatter({ type: this._errFormat, @@ -122,8 +135,8 @@ class ExitHandlers { process.on('SIGQUIT', this.signalHandler); process.on('SIGHUP', this.signalHandler); // Both synchronous and asynchronous errors are handled - process.once('unhandledRejection', this.errorHandler); - process.once('uncaughtException', this.errorHandler); + process.once('unhandledRejection', this.unhandledRejectionHandler); + process.once('uncaughtException', this.uncaughtExceptionHandler); process.once('beforeExit', this.deadlockHandler); } @@ -132,8 +145,11 @@ class ExitHandlers { process.removeListener('SIGTERM', this.signalHandler); process.removeListener('SIGQUIT', this.signalHandler); process.removeListener('SIGHUP', this.signalHandler); - process.removeListener('unhandledRejection', this.errorHandler); - process.removeListener('uncaughtException', this.errorHandler); + process.removeListener( + 'unhandledRejection', + this.unhandledRejectionHandler, + ); + process.removeListener('uncaughtException', this.uncaughtExceptionHandler); process.removeListener('beforeExit', this.deadlockHandler); } diff --git a/src/errors.ts b/src/errors.ts index 3f6aba171..e2114cf55 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -41,10 +41,6 @@ class ErrorPolykeyClientDestroyed extends ErrorPolykey { exitCode = sysexits.USAGE; } -class ErrorInvalidId extends ErrorPolykey {} - -class ErrorInvalidConfigEnvironment extends ErrorPolykey {} - export { sysexits, ErrorPolykey, @@ -56,8 +52,6 @@ export { ErrorPolykeyClientRunning, ErrorPolykeyClientNotRunning, ErrorPolykeyClientDestroyed, - ErrorInvalidId, - ErrorInvalidConfigEnvironment, }; /** From 97ce1d870c8d9e8a64e7a55f3f05792962a9d49b Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 20 Sep 2022 15:46:22 +1000 Subject: [PATCH 26/40] fix: moved 'syncNodeGraph` from `NodeConnectionManager` to `NodeManager` --- src/PolykeyAgent.ts | 2 +- src/nodes/NodeConnectionManager.ts | 117 ++---------------- src/nodes/NodeManager.ts | 94 ++++++++++++++ .../NodeConnectionManager.seednodes.test.ts | 10 +- 4 files changed, 111 insertions(+), 112 deletions(-) diff --git a/src/PolykeyAgent.ts b/src/PolykeyAgent.ts index 7259ab384..83fe9072e 100644 --- a/src/PolykeyAgent.ts +++ b/src/PolykeyAgent.ts @@ -671,7 +671,7 @@ class PolykeyAgent { await this.nodeManager.start(); await this.nodeConnectionManager.start({ nodeManager: this.nodeManager }); await this.nodeGraph.start({ fresh }); - await this.nodeConnectionManager.syncNodeGraph(false); + await this.nodeManager.syncNodeGraph(false); await this.discovery.start({ fresh }); await this.vaultManager.start({ fresh }); await this.notificationsManager.start({ fresh }); diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 7901ff319..6d0b09cbe 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -13,7 +13,6 @@ import type { SeedNodes, } from './types'; import type NodeManager from './NodeManager'; -import type { TaskHandler, TaskHandlerId } from 'tasks/types'; import type { ContextTimed } from 'contexts/types'; import type { PromiseCancellable } from '@matrixai/async-cancellable'; import { withF } from '@matrixai/resources'; @@ -83,23 +82,6 @@ class NodeConnectionManager { protected backoffDefault: number = 300; // 5 min protected backoffMultiplier: number = 2; // Doubles every failure - public readonly basePath = this.constructor.name; - protected pingAndSetNodeHandler: TaskHandler = async ( - ctx, - _taskInfo, - nodeIdEncoded: string, - host: Host, - port: Port, - ) => { - const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; - const host_ = await networkUtils.resolveHost(host); - if (await this.pingNode(nodeId, host_, port, ctx)) { - await this.nodeManager!.setNode(nodeId, { host: host_, port }); - } - }; - protected pingAndSetNodeHandlerId: TaskHandlerId = - `${this.basePath}.${this.pingAndSetNodeHandler.name}` as TaskHandlerId; - public constructor({ keyManager, nodeGraph, @@ -135,11 +117,6 @@ class NodeConnectionManager { public async start({ nodeManager }: { nodeManager: NodeManager }) { this.logger.info(`Starting ${this.constructor.name}`); this.nodeManager = nodeManager; - // Setting handlers - this.taskManager.registerHandler( - this.pingAndSetNodeHandlerId, - this.pingAndSetNodeHandler, - ); // Adding seed nodes for (const nodeIdEncoded in this.seedNodes) { const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; @@ -161,8 +138,6 @@ class NodeConnectionManager { // It exists so we want to destroy it await this.destroyConnection(IdInternal.fromString(nodeId)); } - // Removing handlers - this.taskManager.deregisterHandler(this.pingAndSetNodeHandlerId); this.logger.info(`Stopped ${this.constructor.name}`); } @@ -515,9 +490,7 @@ class NodeConnectionManager { foundClosest = await this.getRemoteNodeClosestNodes( nextNodeId, targetNodeId, - ctx!.timer.getTimeout() === Infinity - ? undefined - : timerStart(ctx!.timer.getTimeout()), + ctx, ); } catch (e) { if (e instanceof nodesErrors.ErrorNodeConnectionTimeout) return; @@ -590,27 +563,33 @@ class NodeConnectionManager { * target node ID. * @param nodeId the node ID to search on * @param targetNodeId the node ID to find other nodes closest to it - * @param timer Connection timeout timer - * @returns list of nodes and their IP/port that are closest to the target + * @param ctx */ + public getRemoteNodeClosestNodes( + nodeId: NodeId, + targetNodeId: NodeId, + ctx?: Partial, + ): PromiseCancellable>; @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) + @timedCancellable(true, 20000) public async getRemoteNodeClosestNodes( nodeId: NodeId, targetNodeId: NodeId, - timer?: Timer, + @context ctx?: ContextTimed, ): Promise> { // Construct the message const nodeIdMessage = new nodesPB.Node(); nodeIdMessage.setNodeId(nodesUtils.encodeNodeId(targetNodeId)); try { // Send through client + const timeout = ctx!.timer.getTimeout(); const response = await this.withConnF( nodeId, async (connection) => { const client = connection.getClient(); return await client.nodesClosestLocalNodesGet(nodeIdMessage); }, - timer, + timeout === Infinity ? undefined : timerStart(timeout), ); const nodes: Array<[NodeId, NodeData]> = []; // Loop over each map element (from the returned response) and populate nodes @@ -641,80 +620,6 @@ class NodeConnectionManager { } } - /** - * Perform an initial database synchronisation: get k of the closest nodes - * from each seed node and add them to this database - * Establish a proxy connection to each node before adding it - * By default this operation is blocking, set `block` to false to make it - * non-blocking - */ - @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) - public async syncNodeGraph( - block: boolean = true, - timer?: Timer, - ): Promise { - this.logger.info('Syncing nodeGraph'); - for (const seedNodeId of this.getSeedNodes()) { - // Check if the connection is viable - try { - await this.getConnection(seedNodeId, timer); - } catch (e) { - if (e instanceof nodesErrors.ErrorNodeConnectionTimeout) continue; - throw e; - } - const closestNodes = await this.getRemoteNodeClosestNodes( - seedNodeId, - this.keyManager.getNodeId(), - timer, - ); - const localNodeId = this.keyManager.getNodeId(); - for (const [nodeId, nodeData] of closestNodes) { - if (!localNodeId.equals(nodeId)) { - const pingAndSetTask = await this.taskManager.scheduleTask({ - delay: 0, - handlerId: this.pingAndSetNodeHandlerId, - lazy: !block, - parameters: [ - nodesUtils.encodeNodeId(nodeId), - nodeData.address.host, - nodeData.address.port, - ], - path: [this.basePath, this.pingAndSetNodeHandlerId], - // Need to be somewhat active so high priority - priority: 100, - }); - if (block) { - try { - await pingAndSetTask.promise(); - } catch (e) { - if (!(e instanceof nodesErrors.ErrorNodeGraphSameNodeId)) throw e; - } - } - } - } - // Refreshing every bucket above the closest node - let closestNodeInfo = closestNodes.pop()!; - if (this.keyManager.getNodeId().equals(closestNodeInfo[0])) { - // Skip our nodeId if it exists - closestNodeInfo = closestNodes.pop()!; - } - let index = this.nodeGraph.nodeIdBits; - if (closestNodeInfo != null) { - const [closestNode] = closestNodeInfo; - const [bucketIndex] = this.nodeGraph.bucketIndex(closestNode); - index = bucketIndex; - } - for (let i = index; i < this.nodeGraph.nodeIdBits; i++) { - const task = await this.nodeManager!.updateRefreshBucketDelay( - i, - 0, - !block, - ); - if (block) await task.promise(); - } - } - } - /** * Performs a GRPC request to send a hole-punch message to the target. Used to * initially establish the NodeConnection from source to target. diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 98652ca97..214baf8e5 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -11,6 +11,7 @@ import type TaskManager from '../tasks/TaskManager'; import type { TaskHandler, TaskHandlerId, Task } from '../tasks/types'; import type { ContextTimed } from 'contexts/types'; import type { PromiseCancellable } from '@matrixai/async-cancellable'; +import type { Host, Port } from '../network/types'; import Logger from '@matrixai/logger'; import { StartStop, ready } from '@matrixai/async-init/dist/StartStop'; import { Semaphore, Lock } from '@matrixai/async-locks'; @@ -81,6 +82,21 @@ class NodeManager { }; public readonly gcBucketHandlerId = `${this.basePath}.${this.gcBucketHandler.name}` as TaskHandlerId; + protected pingAndSetNodeHandler: TaskHandler = async ( + ctx, + _taskInfo, + nodeIdEncoded: string, + host: Host, + port: Port, + ) => { + const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; + const host_ = await networkUtils.resolveHost(host); + if (await this.pingNode(nodeId, { host: host_, port }, ctx)) { + await this.setNode(nodeId, { host: host_, port }, false, false, ctx); + } + }; + protected pingAndSetNodeHandlerId: TaskHandlerId = + `${this.basePath}.${this.pingAndSetNodeHandler.name}` as TaskHandlerId; constructor({ db, @@ -129,6 +145,10 @@ class NodeManager { this.gcBucketHandlerId, this.gcBucketHandler, ); + this.taskManager.registerHandler( + this.pingAndSetNodeHandlerId, + this.pingAndSetNodeHandler, + ); await this.setupRefreshBucketTasks(); this.logger.info(`Started ${this.constructor.name}`); } @@ -138,6 +158,7 @@ class NodeManager { this.logger.info(`Unregistering handler for setNode`); this.taskManager.deregisterHandler(this.refreshBucketHandlerId); this.taskManager.deregisterHandler(this.gcBucketHandlerId); + this.taskManager.deregisterHandler(this.pingAndSetNodeHandlerId); this.logger.info(`Stopped ${this.constructor.name}`); } @@ -890,6 +911,79 @@ class NodeManager { if (foundTask == null) never(); return foundTask; } + + /** + * Perform an initial database synchronisation: get k of the closest nodes + * from each seed node and add them to this database + * Establish a proxy connection to each node before adding it + * By default this operation is blocking, set `block` to false to make it + * non-blocking + */ + public syncNodeGraph( + block?: boolean, + ctx?: Partial, + ): PromiseCancellable; + @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) + @timedCancellable(true, 20000) + public async syncNodeGraph( + block: boolean = true, + @context ctx?: ContextTimed, + ): Promise { + this.logger.info('Syncing nodeGraph'); + for (const seedNodeId of this.nodeConnectionManager.getSeedNodes()) { + // Check if the connection is viable + if ((await this.pingNode(seedNodeId, undefined, ctx)) === false) { + continue; + } + const closestNodes = + await this.nodeConnectionManager.getRemoteNodeClosestNodes( + seedNodeId, + this.keyManager.getNodeId(), + ctx, + ); + const localNodeId = this.keyManager.getNodeId(); + for (const [nodeId, nodeData] of closestNodes) { + if (!localNodeId.equals(nodeId)) { + const pingAndSetTask = await this.taskManager.scheduleTask({ + delay: 0, + handlerId: this.pingAndSetNodeHandlerId, + lazy: !block, + parameters: [ + nodesUtils.encodeNodeId(nodeId), + nodeData.address.host, + nodeData.address.port, + ], + path: [this.basePath, this.pingAndSetNodeHandlerId], + // Need to be somewhat active so high priority + priority: 100, + }); + if (block) { + try { + await pingAndSetTask.promise(); + } catch (e) { + if (!(e instanceof nodesErrors.ErrorNodeGraphSameNodeId)) throw e; + } + } + } + } + // Refreshing every bucket above the closest node + let closestNodeInfo = closestNodes.pop()!; + if (this.keyManager.getNodeId().equals(closestNodeInfo[0])) { + // Skip our nodeId if it exists + closestNodeInfo = closestNodes.pop()!; + } + let index = this.nodeGraph.nodeIdBits; + if (closestNodeInfo != null) { + const [closestNode] = closestNodeInfo; + const [bucketIndex] = this.nodeGraph.bucketIndex(closestNode); + index = bucketIndex; + } + for (let i = index; i < this.nodeGraph.nodeIdBits; i++) { + const task = await this.updateRefreshBucketDelay(i, 0, !block); + if (block) await task.promise(); + } + } + } } export default NodeManager; diff --git a/tests/nodes/NodeConnectionManager.seednodes.test.ts b/tests/nodes/NodeConnectionManager.seednodes.test.ts index 30ff0044e..8c8ab7638 100644 --- a/tests/nodes/NodeConnectionManager.seednodes.test.ts +++ b/tests/nodes/NodeConnectionManager.seednodes.test.ts @@ -316,7 +316,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { }); await nodeConnectionManager.start({ nodeManager }); await taskManager.startProcessing(); - await nodeConnectionManager.syncNodeGraph(); + await nodeManager.syncNodeGraph(); expect(await nodeGraph.getNode(nodeId1)).toBeDefined(); expect(await nodeGraph.getNode(nodeId2)).toBeDefined(); expect(await nodeGraph.getNode(dummyNodeId)).toBeUndefined(); @@ -378,7 +378,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { }); await nodeConnectionManager.start({ nodeManager }); await taskManager.startProcessing(); - await nodeConnectionManager.syncNodeGraph(); + await nodeManager.syncNodeGraph(); await sleep(1000); expect(mockedRefreshBucket).toHaveBeenCalled(); } finally { @@ -446,7 +446,7 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { await nodeConnectionManager.start({ nodeManager }); await taskManager.startProcessing(); // This should complete without error - await nodeConnectionManager.syncNodeGraph(true); + await nodeManager.syncNodeGraph(true); // Information on remotes are found expect(await nodeGraph.getNode(nodeId1)).toBeDefined(); expect(await nodeGraph.getNode(nodeId2)).toBeDefined(); @@ -510,8 +510,8 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { logger, }); - await node1.nodeConnectionManager.syncNodeGraph(true); - await node2.nodeConnectionManager.syncNodeGraph(true); + await node1.nodeManager.syncNodeGraph(true); + await node2.nodeManager.syncNodeGraph(true); const getAllNodes = async (node: PolykeyAgent) => { const nodes: Array = []; From 12296952b3ca4747a603c54fbc793e67ba0a6eff Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Tue, 20 Sep 2022 17:25:53 +1000 Subject: [PATCH 27/40] fix: `pingNode`s inside of `garbageCollectBucket` now have timeouts separate from the overall timer other fixes have been applied. --- src/client/service/nodesAdd.ts | 1 + src/nodes/NodeConnectionManager.ts | 10 +-- src/nodes/NodeManager.ts | 61 ++++++++++++++----- .../NodeConnectionManager.seednodes.test.ts | 5 +- 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/client/service/nodesAdd.ts b/src/client/service/nodesAdd.ts index 87b356b7f..90ecebb10 100644 --- a/src/client/service/nodesAdd.ts +++ b/src/client/service/nodesAdd.ts @@ -81,6 +81,7 @@ function nodesAdd({ } as NodeAddress, true, request.getForce(), + 1500, undefined, tran, ), diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 6d0b09cbe..bdf9cb5b3 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -433,7 +433,7 @@ class NodeConnectionManager { public async getClosestGlobalNodes( targetNodeId: NodeId, ignoreRecentOffline: boolean = false, - @context ctx?: ContextTimed, + @context ctx: ContextTimed, ): Promise { const localNodeId = this.keyManager.getNodeId(); // Let foundTarget: boolean = false; @@ -456,7 +456,7 @@ class NodeConnectionManager { const contacted: Set = new Set(); // Iterate until we've found and contacted k nodes while (contacted.size <= this.nodeGraph.nodeBucketLimit) { - if (ctx!.signal?.aborted) return; + if (ctx.signal?.aborted) return; // Remove the node from the front of the array const nextNode = shortlist.shift(); // If we have no nodes left in the shortlist, then stop @@ -500,7 +500,7 @@ class NodeConnectionManager { // Check to see if any of these are the target node. At the same time, add // them to the shortlist for (const [nodeId, nodeData] of foundClosest) { - if (ctx!.signal?.aborted) return; + if (ctx.signal?.aborted) return; // Ignore any nodes that have been contacted or our own node if (contacted[nodeId] || localNodeId.equals(nodeId)) { continue; @@ -575,14 +575,14 @@ class NodeConnectionManager { public async getRemoteNodeClosestNodes( nodeId: NodeId, targetNodeId: NodeId, - @context ctx?: ContextTimed, + @context ctx: ContextTimed, ): Promise> { // Construct the message const nodeIdMessage = new nodesPB.Node(); nodeIdMessage.setNodeId(nodesUtils.encodeNodeId(targetNodeId)); try { // Send through client - const timeout = ctx!.timer.getTimeout(); + const timeout = ctx.timer.getTimeout(); const response = await this.withConnF( nodeId, async (connection) => { diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 214baf8e5..0c467f45f 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -16,6 +16,7 @@ import Logger from '@matrixai/logger'; import { StartStop, ready } from '@matrixai/async-init/dist/StartStop'; import { Semaphore, Lock } from '@matrixai/async-locks'; import { IdInternal } from '@matrixai/id'; +import { Timer } from '@matrixai/timer'; import * as nodesErrors from './errors'; import * as nodesUtils from './utils'; import * as tasksErrors from '../tasks/errors'; @@ -71,7 +72,7 @@ class NodeManager { _taskInfo, bucketIndex: number, ) => { - await this.garbageCollectBucket(bucketIndex, ctx); + await this.garbageCollectBucket(bucketIndex, 1500, ctx); // Checking for any new pending tasks const pendingNodesRemaining = this.pendingNodes.get(bucketIndex); if (pendingNodesRemaining == null || pendingNodesRemaining.size === 0) { @@ -91,8 +92,17 @@ class NodeManager { ) => { const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; const host_ = await networkUtils.resolveHost(host); - if (await this.pingNode(nodeId, { host: host_, port }, ctx)) { - await this.setNode(nodeId, { host: host_, port }, false, false, ctx); + if ( + await this.pingNode(nodeId, { host: host_, port }, { signal: ctx.signal }) + ) { + await this.setNode( + nodeId, + { host: host_, port }, + false, + false, + 1500, + ctx, + ); } }; protected pingAndSetNodeHandlerId: TaskHandlerId = @@ -174,11 +184,11 @@ class NodeManager { address?: NodeAddress, ctx?: Partial, ): PromiseCancellable; - @timedCancellable(true, 20000) + @timedCancellable(true, 2000) public async pingNode( nodeId: NodeId, - address?: NodeAddress, - @context ctx?: ContextTimed, + address: NodeAddress | undefined, + @context ctx: ContextTimed, ): Promise { // We need to attempt a connection using the proxies // For now we will just do a forward connect + relay message @@ -475,6 +485,7 @@ class NodeManager { * @param block - When true it will wait for any garbage collection to finish before returning. * @param force - Flag for if we want to add the node without authenticating or if the bucket is full. * This will drop the oldest node in favor of the new. + * @param pingTimeout - Timeout for each ping opearation during garbage collection. * @param ctx * @param tran */ @@ -483,6 +494,7 @@ class NodeManager { nodeAddress: NodeAddress, block?: boolean, force?: boolean, + pingTimeout?: number, ctx?: Partial, tran?: DBTransaction, ): PromiseCancellable; @@ -493,7 +505,8 @@ class NodeManager { nodeAddress: NodeAddress, block: boolean = false, force: boolean = false, - @context ctx?: ContextTimed, + pingTimeout: number = 1500, + @context ctx: ContextTimed, tran?: DBTransaction, ): Promise { // We don't want to add our own node @@ -504,7 +517,7 @@ class NodeManager { if (tran == null) { return this.db.withTransactionF((tran) => - this.setNode(nodeId, nodeAddress, block, force, ctx, tran), + this.setNode(nodeId, nodeAddress, block, force, pingTimeout, ctx, tran), ); } @@ -571,6 +584,7 @@ class NodeManager { nodeId, nodeAddress, block, + pingTimeout, ctx, tran, ); @@ -579,18 +593,20 @@ class NodeManager { protected garbageCollectBucket( bucketIndex: number, + pingTimeout?: number, ctx?: Partial, tran?: DBTransaction, ): PromiseCancellable; @timedCancellable(true, 20000) protected async garbageCollectBucket( bucketIndex: number, + pingTimeout: number = 1500, @context ctx: ContextTimed, tran?: DBTransaction, ): Promise { if (tran == null) { return this.db.withTransactionF((tran) => - this.garbageCollectBucket(bucketIndex, ctx, tran), + this.garbageCollectBucket(bucketIndex, pingTimeout, ctx, tran), ); } @@ -623,7 +639,11 @@ class NodeManager { pendingPromises.push( (async () => { // Ping and remove or update node in bucket - if (await this.pingNode(nodeId, nodeData.address, ctx)) { + const pingCtx = { + signal: ctx.signal, + timer: new Timer({ delay: pingTimeout }), + }; + if (await this.pingNode(nodeId, nodeData.address, pingCtx)) { // Succeeded so update await this.setNode( nodeId, @@ -631,6 +651,7 @@ class NodeManager { false, false, undefined, + undefined, tran, ); } else { @@ -652,7 +673,15 @@ class NodeManager { for (const [nodeIdString, address] of pendingNodes) { if (removedNodes <= 0) break; const nodeId = IdInternal.fromString(nodeIdString); - await this.setNode(nodeId, address, false, false, undefined, tran); + await this.setNode( + nodeId, + address, + false, + false, + undefined, + undefined, + tran, + ); removedNodes -= 1; } } @@ -662,6 +691,7 @@ class NodeManager { nodeId: NodeId, nodeAddress: NodeAddress, block: boolean = false, + pingTimeout: number = 1500, ctx?: ContextTimed, tran?: DBTransaction, ): Promise { @@ -675,7 +705,7 @@ class NodeManager { // If set to blocking we just run the GC operation here // without setting up a new task if (block) { - await this.garbageCollectBucket(bucketIndex, ctx, tran); + await this.garbageCollectBucket(bucketIndex, pingTimeout, ctx, tran); return; } await this.setupGCTask(bucketIndex); @@ -927,12 +957,15 @@ class NodeManager { @timedCancellable(true, 20000) public async syncNodeGraph( block: boolean = true, - @context ctx?: ContextTimed, + @context ctx: ContextTimed, ): Promise { this.logger.info('Syncing nodeGraph'); for (const seedNodeId of this.nodeConnectionManager.getSeedNodes()) { // Check if the connection is viable - if ((await this.pingNode(seedNodeId, undefined, ctx)) === false) { + if ( + (await this.pingNode(seedNodeId, undefined, { signal: ctx.signal })) === + false + ) { continue; } const closestNodes = diff --git a/tests/nodes/NodeConnectionManager.seednodes.test.ts b/tests/nodes/NodeConnectionManager.seednodes.test.ts index 8c8ab7638..033a2f87d 100644 --- a/tests/nodes/NodeConnectionManager.seednodes.test.ts +++ b/tests/nodes/NodeConnectionManager.seednodes.test.ts @@ -400,7 +400,10 @@ describe(`${NodeConnectionManager.name} seed nodes test`, () => { NodeConnectionManager.prototype, 'pingNode', ); - mockedPingNode.mockImplementation(createPromiseCancellable(true)); + mockedPingNode.mockImplementation((nodeId: NodeId) => { + if (dummyNodeId.equals(nodeId)) return createPromiseCancellable(false)(); + return createPromiseCancellable(true)(); + }); try { const seedNodes: SeedNodes = {}; seedNodes[nodesUtils.encodeNodeId(remoteNodeId1)] = { From 48298f8c3f690515d0a6a459e35260f58ba3072a Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 12:40:31 +1000 Subject: [PATCH 28/40] fix: cleaning up ephemeral tasks when stopping `NodeManager` --- src/nodes/NodeManager.ts | 13 ++++++++++- tests/nodes/NodeManager.test.ts | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 0c467f45f..e1d5bd0f4 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -105,7 +105,7 @@ class NodeManager { ); } }; - protected pingAndSetNodeHandlerId: TaskHandlerId = + public readonly pingAndSetNodeHandlerId: TaskHandlerId = `${this.basePath}.${this.pingAndSetNodeHandler.name}` as TaskHandlerId; constructor({ @@ -165,6 +165,17 @@ class NodeManager { public async stop() { this.logger.info(`Stopping ${this.constructor.name}`); + this.logger.info('Cancelling ephemeral tasks'); + const tasks: Array> = []; + for await (const task of this.taskManager.getTasks('asc', false, [ + this.basePath, + ])) { + tasks.push(task.promise()); + task.cancel('cleaning up ephemeral tasks'); + } + // We don't care about the result, only that they've ended + await Promise.allSettled(tasks); + this.logger.info('Cancelled ephemeral tasks'); this.logger.info(`Unregistering handler for setNode`); this.taskManager.deregisterHandler(this.refreshBucketHandlerId); this.taskManager.deregisterHandler(this.gcBucketHandlerId); diff --git a/tests/nodes/NodeManager.test.ts b/tests/nodes/NodeManager.test.ts index e5e1166d3..9738f902d 100644 --- a/tests/nodes/NodeManager.test.ts +++ b/tests/nodes/NodeManager.test.ts @@ -1013,4 +1013,45 @@ describe(`${NodeManager.name} test`, () => { await nodeManager.stop(); } }); + test('Stopping nodeManager should cancel all ephemeral tasks', async () => { + const nodeManager = new NodeManager({ + db, + sigchain: {} as Sigchain, + keyManager, + nodeGraph, + nodeConnectionManager: dummyNodeConnectionManager, + taskManager, + logger, + }); + try { + await nodeManager.start(); + await nodeConnectionManager.start({ nodeManager }); + + // Creating dummy tasks + const task1 = await taskManager.scheduleTask({ + handlerId: nodeManager.pingAndSetNodeHandlerId, + lazy: false, + path: [nodeManager.basePath], + }); + const task2 = await taskManager.scheduleTask({ + handlerId: nodeManager.pingAndSetNodeHandlerId, + lazy: false, + path: [nodeManager.basePath], + }); + + // Stopping nodeManager should cancel any nodeManager tasks + await nodeManager.stop(); + const tasks: Array = []; + for await (const task of taskManager.getTasks('asc', true, [ + nodeManager.basePath, + ])) { + tasks.push(task); + } + expect(tasks.length).toEqual(0); + await expect(task1.promise()).toReject(); + await expect(task2.promise()).toReject(); + } finally { + await nodeManager.stop(); + } + }); }); From 9f3d2c01fcd927ec8c177a791d30c26509ec01c7 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 12:46:40 +1000 Subject: [PATCH 29/40] fix: cleaning up errors --- src/nodes/NodeManager.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index e1d5bd0f4..70d720fb2 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -62,7 +62,6 @@ class NodeManager { parameters: [bucketIndex], path: [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], priority: 0, - deadline: ctx.timer.delay, }); }; public readonly refreshBucketHandlerId = @@ -929,8 +928,7 @@ class NodeManager { ); } else { // These are extra, so we cancel them - // TODO: make error - task.cancel(Error('TMP, cancel extra tasks')); + task.cancel('removing duplicate tasks'); this.logger.warn( `Duplicate refreshBucket task was found for bucket ${bucketIndex}, cancelling`, ); @@ -964,7 +962,7 @@ class NodeManager { block?: boolean, ctx?: Partial, ): PromiseCancellable; - @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) + @ready(new nodesErrors.ErrorNodeManagerNotRunning()) @timedCancellable(true, 20000) public async syncNodeGraph( block: boolean = true, From 45360d434bcc49ae079e1ffa5931fba2874e6bb6 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 13:03:26 +1000 Subject: [PATCH 30/40] fix: small fix to `updateRefreshBucketDelay` --- src/nodes/NodeManager.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 70d720fb2..b818db977 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -895,15 +895,14 @@ class NodeManager { this.refreshBucketDelayJitter, ); let foundTask: Task | undefined; - let count = 0; + let existingTask = false; for await (const task of this.taskManager.getTasks( 'asc', true, [this.basePath, this.refreshBucketHandlerId, `${bucketIndex}`], tran, )) { - count += 1; - if (count <= 1) { + if (!existingTask) { foundTask = task; // Update the first one // total delay is refreshBucketDelay + time since task creation @@ -916,10 +915,12 @@ class NodeManager { jitter; try { await this.taskManager.updateTask(task.id, { delay: delayNew }); + existingTask = true; } catch (e) { - if (e instanceof tasksErrors.ErrorTaskMissing) { - count -= 1; - } else if (!(e instanceof tasksErrors.ErrorTaskRunning)) { + if (e instanceof tasksErrors.ErrorTaskRunning) { + // Ignore running + existingTask = true; + } else if (!(e instanceof tasksErrors.ErrorTaskMissing)) { throw e; } } @@ -934,7 +935,7 @@ class NodeManager { ); } } - if (count === 0) { + if (!existingTask) { this.logger.debug( `No refreshBucket task for bucket ${bucketIndex}, new one was created`, ); From 81e55323c48aace4a75997ca02d0b3eb5d18695a Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 13:27:05 +1000 Subject: [PATCH 31/40] fix: rollback of proxy changes, was out of scope for this PR --- src/network/Proxy.ts | 22 +++++++--------------- src/nodes/NodeConnectionManager.ts | 6 +++++- tests/network/Proxy.test.ts | 7 +++---- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/network/Proxy.ts b/src/network/Proxy.ts index ab15f9dd1..973c7f525 100644 --- a/src/network/Proxy.ts +++ b/src/network/Proxy.ts @@ -12,8 +12,6 @@ import type { NodeId } from '../nodes/types'; import type { Timer } from '../types'; import type UTPConnection from 'utp-native/lib/connection'; import type { ConnectionsReverse } from './ConnectionReverse'; -import type { PromiseCancellable } from '@matrixai/async-cancellable'; -import type { ContextTimed } from 'contexts/types'; import http from 'http'; import UTP from 'utp-native'; import Logger from '@matrixai/logger'; @@ -24,7 +22,6 @@ import ConnectionReverse from './ConnectionReverse'; import ConnectionForward from './ConnectionForward'; import * as networkUtils from './utils'; import * as networkErrors from './errors'; -import { context, timedCancellable } from '../contexts'; import * as nodesUtils from '../nodes/utils'; import { promisify, timerStart, timerStop } from '../utils'; @@ -317,22 +314,17 @@ class Proxy { * It will only stop the timer if using the default timer * Set timer to `null` explicitly to wait forever */ - public openConnectionForward( - nodeId: NodeId, - proxyHost: Host, - proxyPort: Port, - ctx?: Partial, - ): PromiseCancellable; - @timedCancellable(true, 20000) @ready(new networkErrors.ErrorProxyNotRunning(), true) public async openConnectionForward( nodeId: NodeId, proxyHost: Host, proxyPort: Port, - @context ctx?: ContextTimed, + timer?: Timer, ): Promise { - const timerDelay = ctx?.timer.getTimeout() ?? this.connConnectTime; - const timer_: Timer = timerStart(timerDelay); + let timer_ = timer; + if (timer === undefined) { + timer_ = timerStart(this.connConnectTime); + } const proxyAddress = networkUtils.buildAddress(proxyHost, proxyPort); let lock = this.connectionLocksForward.get(proxyAddress); if (lock == null) { @@ -348,8 +340,8 @@ class Proxy { timer_, ); } finally { - if (timer_ != null) { - timerStop(timer_); + if (timer === undefined) { + timerStop(timer_!); } this.connectionLocksForward.delete(proxyAddress); } diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index bdf9cb5b3..596dbc5e9 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -371,7 +371,11 @@ class NodeConnectionManager { proxyPort: Port, ctx?: ContextTimed, ): Promise { - await this.proxy.openConnectionForward(nodeId, proxyHost, proxyPort, ctx); + const timer = + ctx?.timer.getTimeout() != null + ? timerStart(ctx.timer.getTimeout()) + : undefined; + await this.proxy.openConnectionForward(nodeId, proxyHost, proxyPort, timer); } /** diff --git a/tests/network/Proxy.test.ts b/tests/network/Proxy.test.ts index d80881810..5bab753c4 100644 --- a/tests/network/Proxy.test.ts +++ b/tests/network/Proxy.test.ts @@ -6,7 +6,6 @@ import http from 'http'; import tls from 'tls'; import UTP from 'utp-native'; import Logger, { LogLevel, StreamHandler } from '@matrixai/logger'; -import { Timer } from '@matrixai/timer'; import Proxy from '@/network/Proxy'; import * as networkUtils from '@/network/utils'; import * as networkErrors from '@/network/errors'; @@ -312,16 +311,16 @@ describe(Proxy.name, () => { ).rejects.toThrow(networkErrors.ErrorConnectionStartTimeout); expect(receivedCount).toBe(1); // Can override the timer - const timer = new Timer({ delay: 1000 }); + const timer = timerStart(2000); await expect(() => proxy.openConnectionForward( nodeIdABC, localHost, utpSocketHangPort as Port, - { timer }, + timer, ), ).rejects.toThrow(networkErrors.ErrorConnectionStartTimeout); - timer.cancel('clean up'); + timerStop(timer); expect(receivedCount).toBe(2); await expect(() => httpConnect( From a33ea26a6503ead1a6a3dc2b2b4213171fe8a1e4 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 13:28:23 +1000 Subject: [PATCH 32/40] fix: small fix to `garbageCollectBucket` concurrent pinging --- src/nodes/NodeManager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index b818db977..2d4c028c2 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -672,9 +672,9 @@ class NodeManager { removedNodes += 1; }); } - // Releasing semaphore - await semaphoreReleaser(); - })(), + })() + // Clean ensure semaphore is released + .finally(async () => await semaphoreReleaser()), ); } // Wait for pending pings to complete From f11197c24ac8e5799284f7dcb7c47eaa8f2d6deb Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 15:16:18 +1000 Subject: [PATCH 33/40] fix: updated default timeout for `NodeConnectionManager.pingNode` --- src/nodes/NodeConnectionManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index 596dbc5e9..f5a67eeee 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -722,7 +722,7 @@ class NodeConnectionManager { ctx?: Partial, ): PromiseCancellable; @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) - @timedCancellable(true, 20000) + @timedCancellable(true, 2000) public async pingNode( nodeId: NodeId, host: Host | Hostname, From 0f729b1220e02b55427658f0459e9ced27347e05 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 15:51:33 +1000 Subject: [PATCH 34/40] fix: using Symbols for cancelling tasks --- src/nodes/NodeManager.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 2d4c028c2..82d7eebff 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -29,6 +29,9 @@ import * as sigchainUtils from '../sigchain/utils'; import * as claimsUtils from '../claims/utils'; import { never } from '../utils/utils'; +const abortEphemeralTaskReason = Symbol('abort ephemeral task reason'); +const abortSingletonTaskReason = Symbol('abort singleton task reason'); + interface NodeManager extends StartStop {} @StartStop() class NodeManager { @@ -170,7 +173,7 @@ class NodeManager { this.basePath, ])) { tasks.push(task.promise()); - task.cancel('cleaning up ephemeral tasks'); + task.cancel(abortEphemeralTaskReason); } // We don't care about the result, only that they've ended await Promise.allSettled(tasks); @@ -738,14 +741,14 @@ class NodeManager { { if (scheduled) { // Duplicate scheduled are removed - task.cancel('Removing extra scheduled task'); + task.cancel(abortSingletonTaskReason); break; } scheduled = true; } break; default: - task.cancel('Removing extra task'); + task.cancel(abortSingletonTaskReason); break; } } @@ -929,7 +932,7 @@ class NodeManager { ); } else { // These are extra, so we cancel them - task.cancel('removing duplicate tasks'); + task.cancel(abortSingletonTaskReason); this.logger.warn( `Duplicate refreshBucket task was found for bucket ${bucketIndex}, cancelling`, ); From 607095bb05a9840f199607633b978fed8239e8f0 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 15:53:14 +1000 Subject: [PATCH 35/40] fix: `TaskManager` should extend the `CreateDestroyStartStop` interface --- src/tasks/TaskManager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tasks/TaskManager.ts b/src/tasks/TaskManager.ts index 6dc221def..bd4ae29dd 100644 --- a/src/tasks/TaskManager.ts +++ b/src/tasks/TaskManager.ts @@ -31,6 +31,7 @@ import * as utils from '../utils'; const abortSchedulingLoopReason = Symbol('abort scheduling loop reason'); const abortQueuingLoopReason = Symbol('abort queuing loop reason'); +interface TaskManager extends CreateDestroyStartStop {} @CreateDestroyStartStop( new tasksErrors.ErrorTaskManagerRunning(), new tasksErrors.ErrorTaskManagerDestroyed(), From 43027e7d937d635ae9dabe7cbc419b04d3779feb Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 16:11:15 +1000 Subject: [PATCH 36/40] fix: test wasn't overriding key-pair generation --- tests/agent/service/nodesCrossSignClaim.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/agent/service/nodesCrossSignClaim.test.ts b/tests/agent/service/nodesCrossSignClaim.test.ts index 994ccd391..d405c0618 100644 --- a/tests/agent/service/nodesCrossSignClaim.test.ts +++ b/tests/agent/service/nodesCrossSignClaim.test.ts @@ -53,7 +53,7 @@ describe('nodesCrossSignClaim', () => { password, nodePath: path.join(dataDir, 'remoteNode'), keysConfig: { - rootKeyPairBits: 2048, + privateKeyPemOverride: globalRootKeyPems[1], }, seedNodes: {}, // Explicitly no seed nodes on startup networkConfig: { From 0267d3b5f35867ea41410fdaa3f7443c1d1900dc Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 16:14:47 +1000 Subject: [PATCH 37/40] fix: `TaskManager`'s `stopProcessing` and `stopTasks` are now properly idempotent the @ready decorator caused them to throw if ran while `taskManager` was not running. They needed to be called during incomplete startup, so I removed the decorator. --- src/tasks/TaskManager.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tasks/TaskManager.ts b/src/tasks/TaskManager.ts index bd4ae29dd..d4c00b032 100644 --- a/src/tasks/TaskManager.ts +++ b/src/tasks/TaskManager.ts @@ -236,7 +236,6 @@ class TaskManager { * Stop the scheduling and queuing loop * This call is idempotent */ - @ready(new tasksErrors.ErrorTaskManagerNotRunning(), false, ['stopping']) public async stopProcessing(): Promise { await Promise.all([this.stopQueueing(), this.stopScheduling()]); } @@ -245,7 +244,6 @@ class TaskManager { * Stop the active tasks * This call is idempotent */ - @ready(new tasksErrors.ErrorTaskManagerNotRunning(), false, ['stopping']) public async stopTasks(): Promise { for (const [, activePromise] of this.activePromises) { activePromise.cancel(new tasksErrors.ErrorTaskStop()); From ccc61a87eef7a95ca4bec1fa4c7f771d009c223b Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 16:39:51 +1000 Subject: [PATCH 38/40] tests: slightly increasing timeouts for two tests --- tests/bin/notifications/sendReadClear.test.ts | 2 +- tests/nodes/NodeConnection.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bin/notifications/sendReadClear.test.ts b/tests/bin/notifications/sendReadClear.test.ts index f681e68bd..b70024554 100644 --- a/tests/bin/notifications/sendReadClear.test.ts +++ b/tests/bin/notifications/sendReadClear.test.ts @@ -315,6 +315,6 @@ describe('send/read/claim', () => { .map(JSON.parse); expect(readNotifications).toHaveLength(0); }, - globalThis.defaultTimeout * 2, + globalThis.defaultTimeout * 3, ); }); diff --git a/tests/nodes/NodeConnection.test.ts b/tests/nodes/NodeConnection.test.ts index 228fd5b1a..efa71300f 100644 --- a/tests/nodes/NodeConnection.test.ts +++ b/tests/nodes/NodeConnection.test.ts @@ -506,7 +506,7 @@ describe(`${NodeConnection.name} test`, () => { // Have a nodeConnection try to connect to it const killSelf = jest.fn(); nodeConnection = await NodeConnection.createNodeConnection({ - timer: timerStart(500), + timer: timerStart(2000), proxy: clientProxy, keyManager: clientKeyManager, logger: logger, From b29ba9e0ea66fc02d6b35e5a4da354bcb6507dd4 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 19:16:56 +1000 Subject: [PATCH 39/40] tests: general fixes for tests failing in CI --- src/PolykeyAgent.ts | 2 +- src/nodes/NodeConnectionManager.ts | 16 ++++++++++------ src/nodes/NodeManager.ts | 20 +++++++++++++++----- tests/discovery/Discovery.test.ts | 3 +++ 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/PolykeyAgent.ts b/src/PolykeyAgent.ts index 83fe9072e..997010d21 100644 --- a/src/PolykeyAgent.ts +++ b/src/PolykeyAgent.ts @@ -700,10 +700,10 @@ class PolykeyAgent { await this.notificationsManager?.stop(); await this.vaultManager?.stop(); await this.discovery?.stop(); - await this.taskManager?.stop(); await this.nodeGraph?.stop(); await this.nodeConnectionManager?.stop(); await this.nodeManager?.stop(); + await this.taskManager?.stop(); await this.proxy?.stop(); await this.grpcServerAgent?.stop(); await this.grpcServerClient?.stop(); diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts index f5a67eeee..9861e2445 100644 --- a/src/nodes/NodeConnectionManager.ts +++ b/src/nodes/NodeConnectionManager.ts @@ -29,7 +29,7 @@ import GRPCClientAgent from '../agent/GRPCClientAgent'; import * as validationUtils from '../validation/utils'; import * as networkUtils from '../network/utils'; import * as nodesPB from '../proto/js/polykey/v1/nodes/nodes_pb'; -import { timerStart } from '../utils'; +import { timerStart, never } from '../utils'; type ConnectionAndTimer = { connection: NodeConnection; @@ -119,7 +119,8 @@ class NodeConnectionManager { this.nodeManager = nodeManager; // Adding seed nodes for (const nodeIdEncoded in this.seedNodes) { - const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; + const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded); + if (nodeId == null) never(); await this.nodeManager.setNode( nodeId, this.seedNodes[nodeIdEncoded], @@ -224,7 +225,8 @@ class NodeConnectionManager { const [release, conn] = await acquire(); let caughtError; try { - return yield* g(conn!); + if (conn == null) never(); + return yield* g(conn); } catch (e) { caughtError = e; throw e; @@ -701,9 +703,11 @@ class NodeConnectionManager { */ @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning()) public getSeedNodes(): Array { - return Object.keys(this.seedNodes).map( - (nodeIdEncoded) => nodesUtils.decodeNodeId(nodeIdEncoded)!, - ); + return Object.keys(this.seedNodes).map((nodeIdEncoded) => { + const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded); + if (nodeId == null) never(); + return nodeId; + }); } /** diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts index 82d7eebff..7d313f62c 100644 --- a/src/nodes/NodeManager.ts +++ b/src/nodes/NodeManager.ts @@ -92,7 +92,13 @@ class NodeManager { host: Host, port: Port, ) => { - const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded)!; + const nodeId = nodesUtils.decodeNodeId(nodeIdEncoded); + if (nodeId == null) { + this.logger.error( + `pingAndSetNodeHandler received invalid NodeId: ${nodeIdEncoded}`, + ); + never(); + } const host_ = await networkUtils.resolveHost(host); if ( await this.pingNode(nodeId, { host: host_, port }, { signal: ctx.signal }) @@ -569,7 +575,8 @@ class NodeManager { // We just add the new node anyway without checking the old one const oldNodeId = ( await this.nodeGraph.getOldestNode(bucketIndex, 1, tran) - ).pop()!; + ).pop(); + if (oldNodeId == null) never(); this.logger.debug( `Force was set, removing ${nodesUtils.encodeNodeId( oldNodeId, @@ -1013,10 +1020,13 @@ class NodeManager { } } // Refreshing every bucket above the closest node - let closestNodeInfo = closestNodes.pop()!; - if (this.keyManager.getNodeId().equals(closestNodeInfo[0])) { + let closestNodeInfo = closestNodes.pop(); + if ( + closestNodeInfo != null && + this.keyManager.getNodeId().equals(closestNodeInfo[0]) + ) { // Skip our nodeId if it exists - closestNodeInfo = closestNodes.pop()!; + closestNodeInfo = closestNodes.pop(); } let index = this.nodeGraph.nodeIdBits; if (closestNodeInfo != null) { diff --git a/tests/discovery/Discovery.test.ts b/tests/discovery/Discovery.test.ts index 3a5ebf34e..f99c45ee9 100644 --- a/tests/discovery/Discovery.test.ts +++ b/tests/discovery/Discovery.test.ts @@ -22,6 +22,7 @@ import * as nodesUtils from '@/nodes/utils'; import * as claimsUtils from '@/claims/utils'; import * as discoveryErrors from '@/discovery/errors'; import * as keysUtils from '@/keys/utils'; +import * as grpcUtils from '@/grpc/utils/index'; import * as testNodesUtils from '../nodes/utils'; import TestProvider from '../identities/TestProvider'; import { globalRootKeyPems } from '../fixtures/globalRootKeyPems'; @@ -59,6 +60,8 @@ describe('Discovery', () => { let nodeB: PolykeyAgent; let identityId: IdentityId; beforeEach(async () => { + // Sets the global GRPC logger to the logger + grpcUtils.setLogger(logger); dataDir = await fs.promises.mkdtemp( path.join(os.tmpdir(), 'polykey-test-'), ); From 23f470baeb76ef80b885e3692a7c8302de4ca726 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 21 Sep 2022 19:45:19 +1000 Subject: [PATCH 40/40] syntax: formatting change for `ExitHandlers.ts` --- src/bin/utils/ExitHandlers.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/bin/utils/ExitHandlers.ts b/src/bin/utils/ExitHandlers.ts index a9abcfaff..fbb1ee854 100644 --- a/src/bin/utils/ExitHandlers.ts +++ b/src/bin/utils/ExitHandlers.ts @@ -11,6 +11,7 @@ class ExitHandlers { public handlers: Array<(signal?: NodeJS.Signals) => Promise>; protected _exiting: boolean = false; protected _errFormat: 'json' | 'error'; + /** * Handles termination signals * This is idempotent @@ -52,6 +53,7 @@ class ExitHandlers { process.kill(process.pid, signal); } }; + /** * Handles asynchronous exceptions * This prints out appropriate error message on STDERR @@ -75,6 +77,7 @@ class ExitHandlers { // Fail fast pattern process.exit(); }; + /** * Handles synchronous exceptions * This prints out appropriate error message on STDERR @@ -98,6 +101,7 @@ class ExitHandlers { // Fail fast pattern process.exit(); }; + protected deadlockHandler = async () => { if (process.exitCode == null) { const e = new binErrors.ErrorBinAsynchronousDeadlock();