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

Give more time for e2e_basic_start_stop's verify_none_running #3385

Closed
wants to merge 3 commits into from

Conversation

cce
Copy link
Contributor

@cce cce commented Jan 6, 2022

Summary

This test has been failing more often in recent nightly jobs. Giving it more time to wait for algod to shut down should help.

######################################################################
  e2e_basic_start_stop
######################################################################
Verifying a generic node will start using goal
Algorand node successfully started!
Verifying we can stop it using goal
The node was successfully stopped.
Verifying a generic node will start directly
Verifying that the datadir algod lock works correctly
Logging to:  /opt/cibuild/project/tmp/out/e2e/13191-1641172161879/data/node.log
Deadlock detection is set to: enabled (Default state is 'enable')
Initializing the Algorand node... Success!
⇨ http server started on 127.0.0.1:8080
Node running and accepting RPC requests over HTTP on port 127.0.0.1:8080. Press Ctrl-C to exit
algod not expected to be running but it is
subcommand returncode 1: ['./e2e_basic_start_stop.sh']
Cleaning up temp dir.

Test Plan

Modifies existing test.

@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2022

Codecov Report

Merging #3385 (e8afef3) into master (2346615) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3385      +/-   ##
==========================================
- Coverage   47.65%   47.63%   -0.02%     
==========================================
  Files         370      370              
  Lines       59804    59804              
==========================================
- Hits        28498    28490       -8     
- Misses      28001    28007       +6     
- Partials     3305     3307       +2     
Impacted Files Coverage Δ
crypto/merkletrie/trie.go 66.42% <0.00%> (-2.19%) ⬇️
catchup/service.go 68.65% <0.00%> (-2.00%) ⬇️
crypto/merkletrie/node.go 91.62% <0.00%> (-1.87%) ⬇️
ledger/tracker.go 75.55% <0.00%> (-1.34%) ⬇️
catchup/peerSelector.go 98.95% <0.00%> (-1.05%) ⬇️
data/abi/abi_type.go 87.67% <0.00%> (-0.95%) ⬇️
data/transactions/verify/txn.go 44.29% <0.00%> (ø)
network/wsPeer.go 69.16% <0.00%> (+0.55%) ⬆️
ledger/acctupdates.go 66.15% <0.00%> (+1.33%) ⬆️
ledger/blockqueue.go 85.05% <0.00%> (+2.87%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2346615...e8afef3. Read the comment docs.

@algobarb
Copy link
Contributor

algobarb commented Jan 10, 2022

The CircleCi tests still seem to be failing? Do you think it would still help rel/nightly?

@cce
Copy link
Contributor Author

cce commented Jan 10, 2022

I'm assuming there's some timeout that is high enough to handle these occasional cases where algod needs a little more time to quit. So hopefully the last commit (which raises it to 20s) should be enough.

Copy link
Contributor

@algobarb algobarb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if it was the last commit that made it pass, but I don't think this will hurt to try it on our flaky tests

@tsachiherman
Copy link
Contributor

tsachiherman commented Jan 13, 2022

I think that we have a bug in algod which prevents it from shutting down when requested. While this PR is not wrong on it's own, I think that figuring out the slowness during the shutdown would be a good thing to accomplish. ( which might also resolve this issue ).

i.e. the following is one step toward achieving this :
#3416

@cce cce closed this Feb 9, 2022
@cce cce deleted the cce/flaky-e2e_basic_start_stop branch February 19, 2022 01:41
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.

5 participants