Extend load generator to produce high volume traffic #3492
Conversation
…traffic. This is based on the capability to generate load ignoring low fee tx errors (stellar/stellar-core#3492) I'm adding the following scenarios: - Regular load generation, but with the limiting logic - Protocol upgrade under load (and with limiting) - this could be relevant for e.g. p20 upgrade - Running the mixed image version network at 51% quorum and with load - this could have caught network forking of 19.3.0 release.
…traffic. This is based on the capability to generate load and ignore low fee tx errors (stellar/stellar-core#3492) I'm adding the following scenarios: - Regular load generation, but with limiting/surge pricing triggering - Protocol upgrade under load/limiting - this would be relevant for e.g. p20 upgrade to use generalized tx sets - Running the mixed image version network at 51% quorum and with load - this could have caught network forking of 19.3.0 release.
These are introduced by Core PR stellar/stellar-core#3492
| @@ -49,6 +50,10 @@ const uint32_t LoadGenerator::TX_SUBMIT_MAX_TRIES = 10; | |||
| // ledger. | |||
| const uint32_t LoadGenerator::TIMEOUT_NUM_LEDGERS = 20; | |||
|
|
|||
| // After successfully submitting desired load, wait for this many ledgers | |||
| // without checking for account consistency. | |||
| const uint32_t LoadGenerator::COMPLETION_TIMEOUT_WITHOUT_CHECKS = 4; | |||
There was a problem hiding this comment.
I'm less familiar with the internals of the tx queue, but was wondering if this might be too aggressive? In other loadgen modes we allow up to 20 ledgers to finish load processing.
There was a problem hiding this comment.
I'm actually not sure why do we wait for so much. We buffer just 2x the max tx set size operations, so that should be applied within a single ledger from the load generation finish. Supercluster has an additional synchronization check mechanism that doesn't rely on account seq num consistency. So I think this should be enough for at least single-node load generation and small missions. I haven't tested this mode with complex topologies though; OTOH I'm not sure that would be useful for those.
There was a problem hiding this comment.
Oh, I see. Should we just use one parameter across all loadgen modes for consistency? I think as soon loadgen is done submitting load, the transactions would either be picked up within the next 3 ledgers or dropped, so it seems like there isn't much value in waiting for 20 ledgers (I can't recall how we landed on 20, it might have been an arbitrary choice. @MonsieurNicolas might know better)
There was a problem hiding this comment.
I guess for the old constant we wouldn't normally reach the limit (unless something is broken), so there was no point in fine-tuning it. For the new constant we always have to wait this much. I'm not opposed to using a single small constant, though maybe would do that separately to be safe.
These are introduced by Core PR stellar/stellar-core#3492
docs/software/commands.md
Outdated
| `txrate` | ||
| * `maxfeerate` defines the maximum per-operation fee for generated | ||
| transactions (when not specified only minimum base fee is used) | ||
| * when `skiplowfeetxs` is set to `1` the transactions that are not accepted by |
There was a problem hiding this comment.
nit: we use explicit "true" and "false" for bools in other http commands, so it'd be nice to do the same here for consistency.
src/simulation/LoadGenerator.cpp
Outdated
| auto feeRateDistr = | ||
| uniform_int_distribution<uint32_t>(baseFee, *maxGeneratedFeeRate); | ||
| // Add a bit more fee to get non-integer fee rates, such that | ||
| // `ceil(fee / ops.size()) == feeRate`, but |
There was a problem hiding this comment.
should be feeRate + 1, right?
There was a problem hiding this comment.
Actually it should be floor instead (when added value is 0 ceil would be equal to feeRate, so + 1 won't be correct).
src/main/CommandHandler.cpp
Outdated
| bool const& defaultValue) | ||
| { | ||
| auto paramStr = parseOptionalParam<std::string>(map, key); | ||
| return paramStr && *paramStr == "true"; |
There was a problem hiding this comment.
this condition doesn't respect the default value (if option is not provided, it should return the default)
src/simulation/LoadGenerator.cpp
Outdated
| auto baseFee = mApp.getLedgerManager().getLastTxFee(); | ||
| if (baseFee > *maxGeneratedFeeRate) | ||
| { | ||
| throw std::runtime_error( |
There was a problem hiding this comment.
hmm, this isn't an internal bug, just bad parameters from the user, so no need to crash here (need to gracefully fail instead). Also, is this the right place for the check? We could fast fail in CommandHandler right after parsing maxGeneratedFeeRate (and return an appropriate error message to the user in retStr)
There was a problem hiding this comment.
True, moved the check to CommandHandler.
Rebased & squashed. |
This currently has a significant number of arguments, some of which are only modified in a single place. The following changes need even more parameters, so maintaining all the args/defaults becomes unwieldy. This will look nicer with struct designators in C++20, currently I'm using factory functions to address the common patterns.
…te diverse fees. While it's currently possible to trigger surge pricing with load generator, the request need to fit exactly into 1x and 2x the tx set size, which is tricky if we want to also randomize operation counts. This mode also allows to exercise the tx queue limiter, which is useful too.
|
r+ 31566c8 |
These are introduced by Core PR stellar/stellar-core#3492
These are introduced by Core PR stellar/stellar-core#3492
Description
Extend load generator to produce high volume traffic that easily triggers limiting and surge pricing logic. Also added fee diversification and did some refactoring to easier introduce new parameters.
Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)