[tmpnet] Minimize duration of tx acceptance for e2e testing #3685
Conversation
maru-ava
force-pushed
the
tmpnet-minimize-tx-acceptance-duration
branch
from
d55ad5a to
021ba8f
Compare
maru-ava
force-pushed
the
tmpnet-minimize-tx-acceptance-duration
branch
from
021ba8f to
6d51c62
Compare
There was a problem hiding this comment.
- What's the overall time saved?
Reduce the wallet poll interval from 100ms to 10ms- just as a word of warning, getting close to millisecond in tests can be risky on CI running on shared/slow runners, and produce flaky tests, just because the runner is slow and things take milliseconds to execute... think Pentium 4 running CI 😄
tests/fixture/tmpnet/defaults.go
Outdated
| @@ -76,3 +78,11 @@ func DefaultChainConfigs() map[string]FlagsMap { | |||
| }, | |||
| } | |||
| } | |||
|
|
|||
| // A set of subnet configuration appropriate for testing. | |||
| func DefaultSubnetConfig() FlagsMap { | |||
There was a problem hiding this comment.
since this is only used in this package, can we unexport it?
There was a problem hiding this comment.
tmpnet is used by other repos - including subnet-evm and hypersdk - and the intention is for reuse.
I wasn't too concerned with total time saved. The goal was speeding up perceived execution speed when For a given transaction, a min block delay of 1s ensures that a set of sequentially executed transactions (as is commonly done in a test that uses a single wallet) take a minimum of 1s per transaction. In Stephen's example PR with a polling interval of 1ms, tx acceptance could approach that lower bound. I considered 1ms potentially problematic in the context of CI, though, so bumped the interval to 10ms. Going from a minimum of 1s to a minimum of 10ms is a pretty nice speedup!
Sure, that's why I didn't go all the way down to 1ms. If its flakey, we'll adjust it. |
maru-ava
force-pushed
the
tmpnet-minimize-tx-acceptance-duration
branch
from
6d51c62 to
a392bdb
Compare
config/config.go
Outdated
| @@ -1132,7 +1132,7 @@ func getDefaultSubnetConfig(v *viper.Viper) subnets.Config { | |||
| return subnets.Config{ | |||
| ConsensusParameters: getConsensusConfig(v), | |||
| ValidatorOnly: false, | |||
| ProposerMinBlockDelay: proposervm.DefaultMinBlockDelay, | |||
| ProposerMinBlockDelay: v.GetDuration(ProposerMinBlockDelayKey), | |||
There was a problem hiding this comment.
This causes some weirdness in the defaults.
- The default value of
ProposerMinBlockDelayfor all subnets is currentlyproposervm.DefaultMinBlockDelay. - With this change, the default value for subnets that do not specify a custom subnet config is still
proposervm.DefaultMinBlockDelay. - With this change, the default value for subnets that do specify a custom subnet config is equal to
--proposer-min-block-delay.
I think (2) and (3) should be unified.
There was a problem hiding this comment.
I've updated loading of subnet config to set the default subnet config if one is not provided. PTAL
config/keys.go
Outdated
| @@ -208,4 +208,5 @@ const ( | |||
| TracingExporterTypeKey = "tracing-exporter-type" | |||
| TracingHeadersKey = "tracing-headers" | |||
| ProcessContextFileKey = "process-context-file" | |||
| ProposerMinBlockDelayKey = "proposer-min-block-delay" | |||
There was a problem hiding this comment.
Should we include this in RELEASES.md (and maybe in config.md as well?)
There was a problem hiding this comment.
Added to config.md. How to add it to RELEASES?
config/keys.go
Outdated
| @@ -208,4 +208,5 @@ const ( | |||
| TracingExporterTypeKey = "tracing-exporter-type" | |||
| TracingHeadersKey = "tracing-headers" | |||
| ProcessContextFileKey = "process-context-file" | |||
| ProposerMinBlockDelayKey = "proposer-min-block-delay" | |||
There was a problem hiding this comment.
Also wondering, if this is only supposed to impact the primary network, if we should name it something like --primary-network-min-block-delay
There was a problem hiding this comment.
Ok to rename it to --promposevm-min-block-delay or is --proposer-min-block-delay prefereable?
- Enable configuration of min block delay for primary subnet with the addition of node flag `--proposer-min-block-delay` - Configure tmpnet to default the block delay to zero for all subnets - Reduce the wallet poll interval from 100ms to 10ms - Remove unnecessary sleep in virtuous test
maru-ava
force-pushed
the
tmpnet-minimize-tx-acceptance-duration
branch
from
5234800 to
579d425
Compare
Why this should be merged
Faster tests mean faster iteration
How this works
--proposer-min-block-delayHow this was tested
CI against regression, manually verified decreased runtime
Need to be documented in RELEASES.md?
New node flag should be documented.