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

go/consensus/tendermint: Make sure DBs are only closed during cleanup #4823

Merged
merged 1 commit into from
Jun 27, 2022

Conversation

kostko
Copy link
Member

@kostko kostko commented Jun 27, 2022

Tendermint Core will close the block/state store DBs during Stop which
can make certain in-flight queries trigger a panic due to the database
being already closed.

This commit introduces a DB wrapper that enables us to defer closing
the DB until Cleanup is called (as at that point all of the services
have already been stopped).

Tendermint Core will close the block/state store DBs during Stop which
can make certain in-flight queries trigger a panic due to the database
being already closed.

This commit introduces a DB wrapper that enables us to defer closing
the DB until Cleanup is called (as at that point all of the services
have already been stopped).
@kostko kostko added c:consensus/cometbft Category: CometBFT c:bug Category: bug labels Jun 27, 2022
@kostko kostko force-pushed the kostko/fix/tm-db-cleanup branch from 80e4eee to 27f763c Compare June 27, 2022 11:09
@kostko kostko marked this pull request as ready for review June 27, 2022 11:44
@codecov
Copy link

codecov bot commented Jun 27, 2022

Codecov Report

Merging #4823 (27f763c) into master (5fd5b80) will increase coverage by 0.03%.
The diff coverage is 76.47%.

❗ Current head 27f763c differs from pull request most recent head c5375f6. Consider uploading reports for the commit c5375f6 to get more accurate results

@@            Coverage Diff             @@
##           master    #4823      +/-   ##
==========================================
+ Coverage   66.64%   66.68%   +0.03%     
==========================================
  Files         451      452       +1     
  Lines       50310    50320      +10     
==========================================
+ Hits        33529    33554      +25     
- Misses      12620    12623       +3     
+ Partials     4161     4143      -18     
Impacted Files Coverage Δ
go/consensus/tendermint/full/light.go 59.25% <0.00%> (-6.13%) ⬇️
go/consensus/tendermint/full/common.go 66.42% <57.14%> (-0.73%) ⬇️
go/consensus/tendermint/db/closer.go 100.00% <100.00%> (ø)
go/consensus/tendermint/full/archive.go 77.39% <100.00%> (+2.60%) ⬆️
go/consensus/tendermint/full/full.go 65.04% <100.00%> (+0.07%) ⬆️
go/runtime/host/sandbox/sandbox.go 60.82% <0.00%> (-6.53%) ⬇️
go/common/grpc/auth/auth.go 94.73% <0.00%> (-5.27%) ⬇️
.../worker/compute/executor/committee/transactions.go 86.36% <0.00%> (-4.55%) ⬇️
go/runtime/host/multi/multi.go 70.58% <0.00%> (-4.42%) ⬇️
go/common/sgx/common.go 66.01% <0.00%> (-3.89%) ⬇️
... and 29 more

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 40eed54...c5375f6. Read the comment docs.

@kostko kostko enabled auto-merge June 27, 2022 12:11
@kostko kostko force-pushed the kostko/fix/tm-db-cleanup branch from 27f763c to c5375f6 Compare June 27, 2022 12:15
@kostko kostko merged commit d0affc9 into master Jun 27, 2022
@kostko kostko deleted the kostko/fix/tm-db-cleanup branch June 27, 2022 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:bug Category: bug c:consensus/cometbft Category: CometBFT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants