hyperledger / caliper

A blockchain benchmark framework to measure performance of multiple blockchain solutions https://wiki.hyperledger.org/display/caliper
https://hyperledger.github.io/caliper/
Apache License 2.0
642 stars 403 forks source link

Remove the Caliper Flow Control system #1586

Open davidkel opened 1 month ago

davidkel commented 1 month ago

Caliper has a flow control system referenced only briefly in the documentation as "Caliper test phase control" when talking about the launch command so it's not well documented at all. The 5 phases are

  1. start
  2. init
  3. install
  4. test
  5. end
{
    "caliper": {
        "blockchain": "ethereum",
        "command" : {
            "start": "docker-compose -f network/ethereum/1node-clique/docker-compose.yml up -d && sleep 3",
            "end" : "docker-compose -f network/ethereum/1node-clique/docker-compose.yml down"
          }

Each of these phases currently MUST run and are run by the caliper manager although the following is noted

  1. if the start/end commands don't exist then nothing is done for those phases
  2. caliper will warn that you cannot install chaincode for fabric, but the ethereum connector will fail if the install fails for any reason (eg it's already deployed)

You can skip the running of a specific phase, so for example for already deployed ethereum contracts, you should run caliper with either the required flow-only options or you can skip with the --caliper-flow-skip-install. But none of this is well documented and it certainly isn't obvious.

The init phase is strange as because it appears to exist to initialise a connector in the caliper manager process, yet it's not something a worker would call so why would the manager need to initialise the connector ? Well maybe it needs to collect some information that it needs to pass to a worker for example ? Fabric ignores the init request effectively and ethereum does this

    /**
     * Initialize the {Ethereum} object.
     * @param {boolean} workerInit Indicates whether the initialization happens in the worker process.
     * @return {object} Promise<boolean> True if the account got unlocked successful otherwise false.
     */
    init(workerInit) {
        if (this.ethereumConfig.contractDeployerAddressPrivateKey) {
            this.web3.eth.accounts.wallet.add(this.ethereumConfig.contractDeployerAddressPrivateKey);
        } else if (this.ethereumConfig.contractDeployerAddressPassword) {
            return this.web3.eth.personal.unlockAccount(this.ethereumConfig.contractDeployerAddress, this.ethereumConfig.contractDeployerAddressPassword, 1000);
        }
    }

I don't know much about the connector but I have to assume it's related only to performing an install as this information is not passed onto workers.

2 Questions therefore

  1. Why have an init phase if this could be done in the install phase with a flag to track the initial setup if required
  2. Why make the init phase controllable at all ? It certainly isn't documented as being useful to not perform the init in the ethereum connector

Also note that the code around this flow control is confusing. For example in the ethereum connector and in the connector interface we see the signature

    /**
     * Initialize the connector and potentially the SUT.
     * @param {boolean} workerInit Indicates whether the initialization happens in the worker process.
     * @async
     */
    async init(workerInit) {

However in the code base, the only place init is called is in the manager

            if (!flowOpts.performInit) {
                logger.info('Skipping initialization phase due to benchmark flow conditioning');
            } else {
                connector = connector ? connector : await this.adapterFactory(-1);
                let initStartTime = Date.now();
                try {
                    await connector.init();

which doesn't occur within a worker process and also doesn't pass a parameter. I see no reason for an controllable init phase, nor a reason for the manager to call init on a connector if it's only to prime the install phase.

This leads to the final consideration, should caliper install smart contracts at all ? The fabric connector doesn't do it now as it's way to complicated and impossible to cover all the required scenarios. I can't speak for Ethereum but I would have to assume that the install process has/will evolve and caliper hasn't kept up with it and will not continue to be kept upto date especially if it is dependent on external versions of SUTs or clients. In order to reduce maintenance of caliper and to make it consistent. I believe we should remove the ability for caliper to install smart contracts on ethereum, and only benchmark SUTs that are already ready to be benchmarked (ie contracts deployed)

Even if we don't remove install it could still detect a deployed contract and not fail the install phase (but I much prefer the idea of removing install. Caliper shouldn't be about setting up a target SUT).

Start and End phases add no value. It's easy to script doing something before the test, running the test and doing something after the test.

Therefore I propose we remove flow control completely from Caliper and remove the ability to install smart contracts. The only issue here would be the integration tests and the caliper-benchmarks around how these currently do testing as they rely on caliper's ability to install a smart contract in order to setup the test SUT (as well as the start/end phases but that's an easy workaround).

davidkel commented 1 month ago

If we don't remove these phases then we need much clearer documentation about how the phases work, what they do and what they affect. For example, the init and install phases are irrelevant to fabric, you can keep them in and they will only perform reporting tasks.