Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

add more test for fixedFeedbackRate controller implementation #1628

Merged
merged 1 commit into from
Nov 12, 2024
Merged

add more test for fixedFeedbackRate controller implementation #1628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/caliper-core/lib/worker/rate-control/fixedFeedbackRate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ class FixedFeedbackRateController extends RateInterface {
if (this.generalSleepTime === 0 || currentSubmitted < this.unfinishedPerWorker) {
return;
}

if (this.stats.getTotalFinishedTx() === 0) {
return;
}
Expand All @@ -74,7 +73,12 @@ class FixedFeedbackRateController extends RateInterface {
return;
}

let diff = (this.generalSleepTime * currentSubmitted - ((Date.now() - this.totalSleepTime) - this.stats.getRoundStartTime()));
const expectedTime = this.generalSleepTime * currentSubmitted;
const actualElapsedTime = (Date.now() - this.totalSleepTime) - this.stats.getRoundStartTime();
const diff = expectedTime - actualElapsedTime;

// If we're ahead by more than 5ms, adjust by sleeping for the difference
// 5ms is used to avoid negligible adjustments that could cause unnecessary delays
if (diff > 5) {
await util.sleep(diff);
return;
Expand Down
159 changes: 147 additions & 12 deletions packages/caliper-core/test/worker/rate-control/fixedFeedbackRate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('fixedFeedbackRate controller implementation', () => {
});

describe('#applyRateController', () => {
let controller, sleepStub, txnStats, clock;
let controller, sleepStub, txnStats, clock, testMessage;

beforeEach( () => {
const msgContent = {
Expand All @@ -99,7 +99,7 @@ describe('fixedFeedbackRate controller implementation', () => {
sleepStub = sinon.stub();
FixedFeedbackRate.__set__('util.sleep', sleepStub);

const testMessage = new TestMessage('test', [], msgContent);
testMessage = new TestMessage('test', [], msgContent);
txnStats = new TransactionStatisticsCollector();
controller = new FixedFeedbackRate.createRateController(testMessage, txnStats, 0);
});
Expand All @@ -108,41 +108,176 @@ describe('fixedFeedbackRate controller implementation', () => {
clock.restore();
});

it('should not sleep if the generalSleepTime is 0', () => {
it('should not sleep if the generalSleepTime is 0', async () => {
controller.generalSleepTime = 0;
txnStats.stats.txCounters.totalSubmitted = 2000;
controller.applyRateControl();
await controller.applyRateControl();

// should have not called the sleep method
sinon.assert.notCalled(sleepStub);
});

it('should not sleep if there are no unfinished transactions', () => {
txnStats.stats.txCounters.totalSubmitted = 0;
controller.applyRateControl();
it('should not sleep if there are no unfinished transactions', async () => {
// Stub methods if necessary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why stub only in this test when all others, including the original test just set the stats directly ?

sinon.stub(txnStats, 'getTotalSubmittedTx').returns(10);
sinon.stub(txnStats, 'getTotalFinishedTx').returns(0);

// should have not called the sleep method
await controller.applyRateControl();

// Ensure sleep was not called
sinon.assert.notCalled(sleepStub);
});

it('should not sleep if backlog transaction is below half the target', () => {
it('should not sleep if backlog transaction is below half the target', async () => {
txnStats.stats.txCounters.totalSubmitted = 1000;
txnStats.stats.txCounters.totalFinished = 999;
controller.generalSleepTime = 1;
controller.applyRateControl();
await controller.applyRateControl();

// should have not called the sleep method
sinon.assert.notCalled(sleepStub);
});

it ('should sleep if the elapsed time difference is greater than 5ms', () => {
it ('should sleep if the elapsed time difference is greater than 5ms', async () => {
txnStats.stats.txCounters.totalSubmitted = 100;
txnStats.stats.txCounters.totalFinished = 2;
controller.applyRateControl();
await controller.applyRateControl();

// should have called the sleep method with a value equal to diff
sinon.assert.calledOnce(sleepStub);
sinon.assert.calledWith(sleepStub, 20000);
});

it('should sleep for backOffTime when no successful transactions have occurred once', async () => {
txnStats.stats.txCounters.totalSubmitted = 5;
txnStats.stats.txCounters.totalFinished = 2;
txnStats.stats.txCounters.totalSuccessful = 0;
controller.generalSleepTime = 1;

await controller.applyRateControl();

sinon.assert.calledOnce(sleepStub);
sinon.assert.calledWith(sleepStub, 1 * controller.backOffTime);
});

it('should increase sleep time when no successful transactions have occurred multiple times', async () => {
// Configure the rate controller with a high TPS value
testMessage.content.rateControl.opts = {
tps: 10000,
sleepTime: 100,
transactionLoad: 10
};

controller = new FixedFeedbackRate.createRateController(testMessage, txnStats, 0);
txnStats.stats.txCounters.totalSubmitted = 5;
txnStats.stats.txCounters.totalFinished = 2;
txnStats.stats.txCounters.totalSuccessful = 0;

controller.zeroSuccessfulCounter = 3;
clock.tick(0); // Ensure clock starts at 0

await controller.applyRateControl();

sinon.assert.calledOnce(sleepStub);
// Expected sleep time should be 4 * backOffTime = 400ms
sinon.assert.calledWith(sleepStub, 4 * controller.backOffTime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is 1 more than the zeroSuccessfulCounter, would be good to show this in this test

});

it('should cap the sleep time increase when zeroSuccessfulCounter exceeds threshold', async () => {
// Configure the rate controller with a high TPS value
testMessage.content.rateControl.opts = {
tps: 10000,
sleepTime: 100,
transactionLoad: 10
};

// Re-instantiate the controller with the new configuration
controller = new FixedFeedbackRate.createRateController(testMessage, txnStats, 0);

// Simulate conditions where there are no successful transactions
txnStats.stats.txCounters.totalSubmitted = 5;
txnStats.stats.txCounters.totalFinished = 2;
txnStats.stats.txCounters.totalSuccessful = 0;

// Set zeroSuccessfulCounter to 31
controller.zeroSuccessfulCounter = 31;

// Use the fake timers to control Date.now()
clock.tick(0); // Ensure clock starts at 0

await controller.applyRateControl();

sinon.assert.calledOnce(sleepStub);
// Expected sleep time should be capped at 30 * backOffTime = 3000ms
sinon.assert.calledWith(sleepStub, 30 * controller.backOffTime);
});

it('should sleep to adjust rate when expected time exceeds actual elapsed time', async () => {
txnStats.stats.txCounters.totalSubmitted = 44;
txnStats.stats.txCounters.totalFinished = 40;

// Simulate time progression
const elapsedTime = 1000; // e.g., 1000ms since start
clock.tick(elapsedTime);

await controller.applyRateControl();

sinon.assert.calledOnce(sleepStub);
// Calculate expected adjustment time
const expectedTime = controller.generalSleepTime * txnStats.getTotalSubmittedTx();
const actualElapsedTime = (Date.now() - controller.totalSleepTime) - txnStats.getRoundStartTime();
const adjustmentTime = expectedTime - actualElapsedTime;

sinon.assert.calledWith(sleepStub, adjustmentTime);
});

it('should sleep correctly based on the number of unfinished transactions', async () => {
// Configure the rate controller
testMessage.content.rateControl.opts = {
tps: 10000,
sleepTime: 100,
transactionLoad: 10
};

for (let i = 10; i > 0; --i) {
// Re-instantiate the controller for each iteration
controller = new FixedFeedbackRate.createRateController(testMessage, txnStats, 0);
const unfinishedPerWorker = controller.unfinishedPerWorker; // Should be 5

// Reset sleep stub
sleepStub.resetHistory();

// Ensure there are successful transactions
sinon.stub(txnStats, 'getTotalSuccessfulTx').returns(5);

// Stub getRoundStartTime
sinon.stub(txnStats, 'getRoundStartTime').returns(0);

// Stub getTotalSubmittedTx and getTotalFinishedTx
const unfinished = i * unfinishedPerWorker;
const totalFinished = 10;
const totalSubmitted = totalFinished + unfinished;

sinon.stub(txnStats, 'getTotalSubmittedTx').returns(totalSubmitted);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again wonder why you stub here, but all others you set the txnStats values directly

sinon.stub(txnStats, 'getTotalFinishedTx').returns(totalFinished);

// Control time
clock.reset();
clock.tick(12);

await controller.applyRateControl();

const expectedSleepTime = i * controller.backOffTime;

sinon.assert.calledOnce(sleepStub);
sinon.assert.calledWith(sleepStub, expectedSleepTime);

// Restore stubs
txnStats.getTotalSubmittedTx.restore();
txnStats.getTotalFinishedTx.restore();
txnStats.getTotalSuccessfulTx.restore();
txnStats.getRoundStartTime.restore();
}
});
});
});
Loading