From 138d29a820e8a61d10ba03fe42c5d77596ef788c Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Sat, 6 May 2023 15:21:23 +0200 Subject: [PATCH] fix: fix faire share worker choice stategy internals update MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/pools/abstract-pool.ts | 1 + .../abstract-worker-choice-strategy.ts | 3 ++ .../fair-share-worker-choice-strategy.ts | 11 +++++-- .../less-busy-worker-choice-strategy.ts | 5 +++ .../less-used-worker-choice-strategy.ts | 5 +++ .../round-robin-worker-choice-strategy.ts | 5 +++ .../selection-strategies-types.ts | 9 ++++- ...hted-round-robin-worker-choice-strategy.ts | 13 +++++--- .../worker-choice-strategy-context.ts | 13 ++++++++ .../selection-strategies.test.js | 33 +++++++++++++++---- 10 files changed, 85 insertions(+), 13 deletions(-) diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 820656bb..b778fe17 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -432,6 +432,7 @@ export abstract class AbstractPool< workerTasksUsage.medRunTime = median(workerTasksUsage.runTimeHistory) } } + this.workerChoiceStrategyContext.update() } /** diff --git a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts index 2af13d63..9aabdfd2 100644 --- a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts @@ -65,6 +65,9 @@ export abstract class AbstractWorkerChoiceStrategy< /** @inheritDoc */ public abstract reset (): boolean + /** @inheritDoc */ + public abstract update (): boolean + /** @inheritDoc */ public abstract choose (): number diff --git a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts index f0bc8787..17133408 100644 --- a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts @@ -58,12 +58,19 @@ export class FairShareWorkerChoiceStrategy< return true } + /** @inheritDoc */ + public update (): boolean { + for (const [workerNodeKey] of this.pool.workerNodes.entries()) { + this.computeWorkerVirtualTaskTimestamp(workerNodeKey) + } + return true + } + /** @inheritDoc */ public choose (): number { let minWorkerVirtualTaskEndTimestamp = Infinity let chosenWorkerNodeKey!: number for (const [workerNodeKey] of this.pool.workerNodes.entries()) { - this.computeWorkerVirtualTaskTimestamp(workerNodeKey) const workerVirtualTaskEndTimestamp = this.workersVirtualTaskTimestamp[workerNodeKey]?.end ?? 0 if (workerVirtualTaskEndTimestamp < minWorkerVirtualTaskEndTimestamp) { @@ -95,7 +102,7 @@ export class FairShareWorkerChoiceStrategy< : this.pool.workerNodes[workerNodeKey].tasksUsage.avgRunTime this.workersVirtualTaskTimestamp[workerNodeKey] = { start: workerVirtualTaskStartTimestamp, - end: workerVirtualTaskStartTimestamp + (workerVirtualTaskTRunTime ?? 0) + end: workerVirtualTaskStartTimestamp + workerVirtualTaskTRunTime } } } diff --git a/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts b/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts index 79e65c59..8feac71c 100644 --- a/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts @@ -43,6 +43,11 @@ export class LessBusyWorkerChoiceStrategy< return true } + /** @inheritDoc */ + public update (): boolean { + return true + } + /** @inheritDoc */ public choose (): number { const freeWorkerNodeKey = this.findFreeWorkerNodeKey() diff --git a/src/pools/selection-strategies/less-used-worker-choice-strategy.ts b/src/pools/selection-strategies/less-used-worker-choice-strategy.ts index 1503e057..078dd8fd 100644 --- a/src/pools/selection-strategies/less-used-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-used-worker-choice-strategy.ts @@ -35,6 +35,11 @@ export class LessUsedWorkerChoiceStrategy< return true } + /** @inheritDoc */ + public update (): boolean { + return true + } + /** @inheritDoc */ public choose (): number { const freeWorkerNodeKey = this.findFreeWorkerNodeKey() diff --git a/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts b/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts index d2d6f6d6..5e35636a 100644 --- a/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts @@ -41,6 +41,11 @@ export class RoundRobinWorkerChoiceStrategy< return true } + /** @inheritDoc */ + public update (): boolean { + return true + } + /** @inheritDoc */ public choose (): number { const chosenWorkerNodeKey = this.nextWorkerNodeId diff --git a/src/pools/selection-strategies/selection-strategies-types.ts b/src/pools/selection-strategies/selection-strategies-types.ts index 0e549cec..968d2996 100644 --- a/src/pools/selection-strategies/selection-strategies-types.ts +++ b/src/pools/selection-strategies/selection-strategies-types.ts @@ -77,9 +77,15 @@ export interface IWorkerChoiceStrategy { */ readonly requiredStatistics: RequiredStatistics /** - * Resets strategy internals (counters, statistics, etc.). + * Resets strategy internals. */ reset: () => boolean + /** + * Updates strategy internals. + * + * @returns `true` if the update is successful, `false` otherwise. + */ + update: () => boolean /** * Chooses a worker node in the pool and returns its key. */ @@ -88,6 +94,7 @@ export interface IWorkerChoiceStrategy { * Removes a worker node key from strategy internals. * * @param workerNodeKey - The worker node key. + * @returns `true` if the worker node key is removed, `false` otherwise. */ remove: (workerNodeKey: number) => boolean /** diff --git a/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts b/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts index ad20db42..18173949 100644 --- a/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts @@ -61,16 +61,21 @@ export class WeightedRoundRobinWorkerChoiceStrategy< return true } + /** @inheritDoc */ + public update (): boolean { + return true + } + /** @inheritDoc */ public choose (): number { const chosenWorkerNodeKey = this.currentWorkerNodeId - const workerVirtualTaskRunTime = this.workerVirtualTaskRunTime ?? 0 - const workerTaskWeight = + const workerVirtualTaskRunTime = this.workerVirtualTaskRunTime + const workerWeight = this.opts.weights?.[chosenWorkerNodeKey] ?? this.defaultWorkerWeight - if (workerVirtualTaskRunTime < workerTaskWeight) { + if (workerVirtualTaskRunTime < workerWeight) { this.workerVirtualTaskRunTime = workerVirtualTaskRunTime + - (this.getWorkerVirtualTaskRunTime(chosenWorkerNodeKey) ?? 0) + this.getWorkerVirtualTaskRunTime(chosenWorkerNodeKey) } else { this.currentWorkerNodeId = this.currentWorkerNodeId === this.pool.workerNodes.length - 1 diff --git a/src/pools/selection-strategies/worker-choice-strategy-context.ts b/src/pools/selection-strategies/worker-choice-strategy-context.ts index 2a487acb..0a488549 100644 --- a/src/pools/selection-strategies/worker-choice-strategy-context.ts +++ b/src/pools/selection-strategies/worker-choice-strategy-context.ts @@ -114,6 +114,19 @@ export class WorkerChoiceStrategyContext< this.workerChoiceStrategies.get(this.workerChoiceStrategy)?.reset() } + /** + * Updates the worker choice strategy internals in the context. + * + * @returns `true` if the update is successful, `false` otherwise. + */ + public update (): boolean { + return ( + this.workerChoiceStrategies.get( + this.workerChoiceStrategy + ) as IWorkerChoiceStrategy + ).update() + } + /** * Executes the worker choice strategy algorithm in the context. * diff --git a/tests/pools/selection-strategies/selection-strategies.test.js b/tests/pools/selection-strategies/selection-strategies.test.js index 1edc1234..e8d9d66b 100644 --- a/tests/pools/selection-strategies/selection-strategies.test.js +++ b/tests/pools/selection-strategies/selection-strategies.test.js @@ -438,6 +438,12 @@ describe('Selection strategies test suite', () => { promises.push(pool.execute()) } await Promise.all(promises) + for (const workerNode of pool.workerNodes) { + expect(workerNode.tasksUsage.avgRunTime).toBeDefined() + expect(workerNode.tasksUsage.avgRunTime).toBeGreaterThanOrEqual(0) + expect(workerNode.tasksUsage.medRunTime).toBeDefined() + expect(workerNode.tasksUsage.medRunTime).toBe(0) + } expect( pool.workerChoiceStrategyContext.workerChoiceStrategies.get( pool.workerChoiceStrategyContext.workerChoiceStrategy @@ -461,6 +467,12 @@ describe('Selection strategies test suite', () => { promises.push(pool.execute()) } await Promise.all(promises) + for (const workerNode of pool.workerNodes) { + expect(workerNode.tasksUsage.avgRunTime).toBeDefined() + expect(workerNode.tasksUsage.avgRunTime).toBeGreaterThanOrEqual(0) + expect(workerNode.tasksUsage.medRunTime).toBeDefined() + expect(workerNode.tasksUsage.medRunTime).toBe(0) + } expect( pool.workerChoiceStrategyContext.workerChoiceStrategies.get( pool.workerChoiceStrategyContext.workerChoiceStrategy @@ -493,7 +505,7 @@ describe('Selection strategies test suite', () => { expect(workerNode.tasksUsage.avgRunTime).toBeDefined() expect(workerNode.tasksUsage.avgRunTime).toBe(0) expect(workerNode.tasksUsage.medRunTime).toBeDefined() - expect(workerNode.tasksUsage.medRunTime).toBeGreaterThan(0) + expect(workerNode.tasksUsage.medRunTime).toBeGreaterThanOrEqual(0) } expect( pool.workerChoiceStrategyContext.workerChoiceStrategies.get( @@ -627,6 +639,12 @@ describe('Selection strategies test suite', () => { promises.push(pool.execute()) } await Promise.all(promises) + for (const workerNode of pool.workerNodes) { + expect(workerNode.tasksUsage.avgRunTime).toBeDefined() + expect(workerNode.tasksUsage.avgRunTime).toBeGreaterThanOrEqual(0) + expect(workerNode.tasksUsage.medRunTime).toBeDefined() + expect(workerNode.tasksUsage.medRunTime).toBe(0) + } expect( pool.workerChoiceStrategyContext.workerChoiceStrategies.get( pool.workerChoiceStrategyContext.workerChoiceStrategy @@ -650,14 +668,17 @@ describe('Selection strategies test suite', () => { ) // TODO: Create a better test to cover `WeightedRoundRobinWorkerChoiceStrategy#choose` const promises = [] - const maxMultiplier = - pool.workerChoiceStrategyContext.workerChoiceStrategies.get( - pool.workerChoiceStrategyContext.workerChoiceStrategy - ).defaultWorkerWeight * 50 + const maxMultiplier = 2 for (let i = 0; i < max * maxMultiplier; i++) { promises.push(pool.execute()) } await Promise.all(promises) + for (const workerNode of pool.workerNodes) { + expect(workerNode.tasksUsage.avgRunTime).toBeDefined() + expect(workerNode.tasksUsage.avgRunTime).toBeGreaterThanOrEqual(0) + expect(workerNode.tasksUsage.medRunTime).toBeDefined() + expect(workerNode.tasksUsage.medRunTime).toBe(0) + } expect( pool.workerChoiceStrategyContext.workerChoiceStrategies.get( pool.workerChoiceStrategyContext.workerChoiceStrategy @@ -695,7 +716,7 @@ describe('Selection strategies test suite', () => { expect(workerNode.tasksUsage.avgRunTime).toBeDefined() expect(workerNode.tasksUsage.avgRunTime).toBe(0) expect(workerNode.tasksUsage.medRunTime).toBeDefined() - expect(workerNode.tasksUsage.medRunTime).toBeGreaterThan(0) + expect(workerNode.tasksUsage.medRunTime).toBeGreaterThanOrEqual(0) } expect( pool.workerChoiceStrategyContext.workerChoiceStrategies.get( -- 2.34.1