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

Remove the Caliper Flow Control system #1586

Open
davidkel opened this issue Jun 1, 2024 · 1 comment
Open

Remove the Caliper Flow Control system #1586

davidkel opened this issue Jun 1, 2024 · 1 comment
Labels
component/core Related to the core code-base component/ethereum Related to the Ethereum adapter documentation Related to the documentation enhancement New feature or request test

Comments

@davidkel
Copy link
Contributor

davidkel commented Jun 1, 2024

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
  • start and end are the phases controlled by the start and end properties in the network configuration file section
{
    "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"
          }
  • test is the actual running of the benchmark phase
  • install is the installing of a contract phase
  • init - well this is the most confusing of the phases

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
Copy link
Contributor Author

davidkel commented Jun 1, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/core Related to the core code-base component/ethereum Related to the Ethereum adapter documentation Related to the documentation enhancement New feature or request test
Projects
None yet
Development

No branches or pull requests

1 participant