e2e: add defer to ensure waitgroup is finished.#9822
Merged
williambanfield merged 3 commits intomainfrom Dec 2, 2022
Merged
Conversation
thanethomson
approved these changes
Dec 2, 2022
Contributor
thanethomson
left a comment
There was a problem hiding this comment.
This should be backported too right?
Contributor
Author
|
@thanethomson Yup. I've been in the habit of adding the tags once everything looks right, but I'll just add those beforehand to make it clearer. |
mergify bot
pushed a commit
that referenced
this pull request
Dec 2, 2022
* defer wait group completion * generate correct number of tx (cherry picked from commit 654e565)
mergify bot
pushed a commit
that referenced
this pull request
Dec 2, 2022
* defer wait group completion * generate correct number of tx (cherry picked from commit 654e565)
williambanfield
added a commit
that referenced
this pull request
Dec 2, 2022
williambanfield
added a commit
that referenced
this pull request
Dec 2, 2022
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aimed at fixing: https://github.com/tendermint/tendermint/actions/runs/3598326181/jobs/6060978982
The issue is that if the context in the
createBatchfunction is canceled before all of the transactions are generated, we will hit the return in thectx.Done()case of the select and thewg.Done()will never be called, causing thecreateBatchfunction to never return and therefore never allow more transactions to be generated.To test this, I passed a context with very short timeout into the
createBatchfunction and observed that, after the initial set of transactions fail because the connection is not yet established to the node, thectx.Done()case is hit and no more transactions are ever created, leading to the same failure we see in the e2e nightly.A sample of output from this experiment is below:
This pull request also contains a fix to the number of transactions generated per batch. The purpose of the
LoadTxBatchSizeparameter was to generate a total ofLoadTxBatchSizetx each second. However, the code as written generatesworkerPoolSize*LoadTxBatchSizetransactions since each worker createsLoadTxBatchSize. This pull request places another loop outside the worker pool that instructs the worker pools to generate new transactions for a total ofLoadTxBatchSize.A full nightly run of the e2e test has been run with this code in place: https://github.com/tendermint/tendermint/actions/runs/3602704988
PR checklist
CHANGELOG_PENDING.mdupdated, or no changelog entry neededdocs/) and code comments, or nodocumentation updates needed