hyperledger-archives / caliper

A blockchain benchmark framework to measure performance of multiple blockchain solutions
Apache License 2.0
74 stars 24 forks source link

modularise the rate control mechanism #57

Closed nklincoln closed 6 years ago

nklincoln commented 6 years ago

At present the rate control mechanism is a simple 'TPS' rate control. A basic TPS control is good for driving simple tests at a set rate, however it has limitations in that:

I have been using a custom rate controller that loads a set number of transactions and maintains a set level of these transactions by adjusting the TPS, which has the benefit of never timing out and the results yield a 'maximum TPS' based on the set loading of transactions that are being processed.

To enable the above I modularised the rate component and would like to submit this modularisation back for consideration to be included in the main Caliper repository.

Users will then be able to submit their own rate controllers for experimentation.

The changes include:

haojun commented 6 years ago

@nklincoln It looks like a great idea, just one comment: Why not remove tps from 'txNumbAndTps' or 'durationAndTps' thoroughly, and define it as an argument of 'basic-interval'? That would make the configuration clearer.
BTW: maybe the name basic-interval should also be changed to a more understandable name like fixed-rate?

nklincoln commented 6 years ago

@haojun Sounds like a plan - I'll make those edits and resubmit 👍

nklincoln commented 6 years ago

Hi @haojun,

I've made the suggested edits, though it resulted in needing to go a bit deeper with changes through the code:

haojun commented 6 years ago

Comments after a quick review:

  1. Some old terms like 'fixed-interval' and 'basic-interval' are still used
  2. (client-util.js) The txDuration should not be divided by number of clients
  3. (local-client.js) The original 'txNum++' is missed in the new 'runFixedNumber' and 'runDuration'. The value is used to update the number of submitted transactions.
  4. (config.json) I prefer to keep 'clients.number' larger than 1 to demo the capability of using multiple clients

BTW: the fixedRate may have some problem when running with zookeeper model, but I can fix it later

nklincoln commented 6 years ago

Hi, Thank you for your prompt review - I have made the above edits (good spot!) and re-pushed the changes.

I will also look into the zookeeper mode, as I have not experimented with yet and I'm keen to investigate it