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

ethereum: prevent unnecessary additional requests #780

Merged

Conversation

benjamincburns
Copy link
Contributor

Web3 has recently added features that lazily fetches missing information
when signing and sending transactions. In our case, this was the
chainId, and the current gasPrice. Because transactions are submitted
asynchronously, these additional requests would sometimes cause
transactions to be submitted out of order. Even if this probelm were
mitigated, these additional requests added spurious latency spikes to
some transactions, negatively impacting the test results.

This partially addresses #776 by eliminating request reordering issues
for stable websocket connections. This change does not however address
transaction reordering for HTTP connections, or for websocket
connections that require reconnects. It also does not solve the
"christmas tree light" issue w.r.t. nonce errors.

Web3 has recently added features that lazily fetches missing information
when signing and sending transactions. In our case, this was the
chainId, and the current gasPrice. Because transactions are submitted
asynchronously, these additional requests would sometimes cause
transactions to be submitted out of order. Even if this probelm were
mitigated, these additional requests added spurious latency spikes to
some transactions, negatively impacting the test results.

This partially addresses hyperledger-caliper#776 by eliminating request reordering issues
for stable websocket connections. This change does not however address
transaction reordering for HTTP connections, or for websocket
connections that require reconnects. It also does not solve the
"christmas tree light" issue w.r.t. nonce errors.

Signed-off-by: Ben Burns <[email protected]>
@benjamincburns benjamincburns force-pushed the 776_eth_no_extra_reqs branch from af8395f to e663ce3 Compare March 31, 2020 21:34
@shemnon
Copy link
Contributor

shemnon commented Apr 1, 2020

LGTM

@benjamincburns
Copy link
Contributor Author

benjamincburns commented Apr 1, 2020

I spent most of yesterday testing out this patch and it seems to fix most of the problems we've observed (but only when using a websocket connection). Prior to this change, if we set the transaction send rate too high, we'd see a bunch of errors coming back. Now it works as you'd expect - there aren't any errors when the send rate is set higher than the chain can accept, and the tool reports the actual TPS. Personally I wouldn't trust benchmarking results from Caliper versions prior to this change, as they're very likely under reporting on performance.

@nklincoln nklincoln merged commit ec6f582 into hyperledger-caliper:master Apr 2, 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.

3 participants