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.

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

Implement a spike feature for generateload #2553

Merged
merged 1 commit into from
Jun 12, 2020

Conversation

hidenori-shinohara
Copy link
Contributor

@hidenori-shinohara hidenori-shinohara commented May 29, 2020

Description

Implementation of a spike feature. By setting spikeSize = S & spikeInterval = I, a spike that injects S transactions will occur every I seconds on top of the base rate. Whenever there is no spike, txRate transactions will be injected per second.
For instance, with the command generateload?mode=pay&txs=5000&txrate=3&spikeinterval=4&spikesize=200,

  • For most parts, we inject 3 txs/s.
  • In the first 3.9 seconds, we inject 3.9 x 3 txs.
  • In the first 4 seconds, we inject 4 x 3 + 200 txs.
    ...

until 5000 transactions get injected in total.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v5.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

docs/software/commands.md Outdated Show resolved Hide resolved
src/simulation/LoadGenerator.cpp Outdated Show resolved Hide resolved
src/simulation/LoadGenerator.cpp Outdated Show resolved Hide resolved
src/simulation/LoadGenerator.h Outdated Show resolved Hide resolved
src/main/CommandHandler.cpp Show resolved Hide resolved
src/simulation/LoadGenerator.cpp Outdated Show resolved Hide resolved
src/simulation/LoadGenerator.cpp Outdated Show resolved Hide resolved
@hidenori-shinohara hidenori-shinohara force-pushed the simulate-command-line branch 2 times, most recently from afa8b43 to 923c9c8 Compare June 2, 2020 16:33
Copy link
Contributor

@marta-lokhova marta-lokhova left a comment

Choose a reason for hiding this comment

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

two minor suggestions, otherwise looks good to me.

src/simulation/LoadGenerator.cpp Outdated Show resolved Hide resolved
src/main/CommandHandler.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@MonsieurNicolas MonsieurNicolas left a comment

Choose a reason for hiding this comment

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

Questions and/or suggestions to simplify

fmt::format(" Generating load: {:d} {:s}, {:d} tx/s = {:f} hours",
numItems, itemType, txRate, hours);

if (spikeSize < txRate)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure I understand the relationship between txRate and spikeSize as written.
txRate is expressed in transactions per second, while spikeSize is a number of transactions per spikeInterval seconds so on can only compare txRate * spikeInterval to spikeSize.

I thought what you wanted to implement was that you'd configure the load generator to generate load with the same average of txRate, but causing a spike of spikeSize every spikeInterval?

So wouldn't the relationship be spikeSize/spikeInterval <= txRate so the test here would really be (spikeSize > txRate * spikeInterval) and when this happens, it's just a warning that the average rate will be higher than expected (and there will only be spikes for the most part)?

That or you can add the spikes on top of the steady rate, and this makes the math even simpler (and in that case, there is no restriction on txRate vs spikeSize).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestions!

I changed my code to the "additive" approach (add spikes on top of the steady rate) as this makes the math simpler.

  • txRate is the "base" rate of the number of transactions per second.
  • A spike will occur every spikeInterval seconds.
  • A spike adds spikeSize transactions in one second on top of the base rate. Thus when there's a spike, spikeSize + txRate transactions will be injected in a second.

{
auto elapsedSec =
std::chrono::duration_cast<std::chrono::seconds>(elapsed);
txs += bigDivide(elapsedSec.count(), 1, spikeInterval.count(),
Copy link
Contributor

Choose a reason for hiding this comment

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

If you make spikes additive to the base rate, this gets simplified down to txs += bigDivide(elapsedSec.count(), spikeSize, spikeInterval.count(),Rounding::ROUND_DOWN);

which is quite simpler to understand (I'm actually quite not sure the math written here is correct as it's mixing spikeSize and txRate)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to me that the division may need to happen before the multiplication. For instance, if elapsedSec.count() = 1, spikeInterval.count() = 3 and spikeSize = 300, we want to add 0 on this line as we have not had our first spike, but bigDivide(1, 300, 3, Rounding::ROUND_DOWN) would give us 100.

@hidenori-shinohara hidenori-shinohara force-pushed the simulate-command-line branch 2 times, most recently from 234b0e8 to 99604b7 Compare June 9, 2020 18:15
@hidenori-shinohara
Copy link
Contributor Author

hidenori-shinohara commented Jun 9, 2020

@MonsieurNicolas I updated the code based on our discussion. Specifically, the code used to generate spikeSize txs in one second, but now the code just generates spikeSize txs every spikeInterval seconds, and a spike just happens without being evened out over a second. This made the math simpler. Let me know what you think!

@MonsieurNicolas
Copy link
Contributor

r+ f1247de

@latobarita latobarita merged commit c5c8c74 into stellar:master Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants