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

fix(gc): put gc into a separate actor #3090

Closed
wants to merge 1 commit into from
Closed

Conversation

bowenwang1996
Copy link
Collaborator

Currently gc is slow because when we delete state changes, we access old data from database that are not necessarily cached, which is slow. We cannot afford to run garbage collection in the same thread as client actor as it potentially blocks client when every step takes several hundred millisecond or even more than one second. This PR separates gc into its own actor and runs in a separate thread.

Test plan

  • test with a node on testnet to make sure that the node is still functional and that it does not slow down because of garbage collection.

@gitpod-io
Copy link

gitpod-io bot commented Aug 5, 2020

@codecov
Copy link

codecov bot commented Aug 6, 2020

Codecov Report

Merging #3090 into master will decrease coverage by 0.32%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3090      +/-   ##
==========================================
- Coverage   87.71%   87.39%   -0.33%     
==========================================
  Files         213      213              
  Lines       42067    41236     -831     
==========================================
- Hits        36901    36040     -861     
- Misses       5166     5196      +30     
Impacted Files Coverage Δ
chain/chain/src/chain.rs 88.73% <ø> (+0.11%) ⬆️
chain/client/src/client.rs 92.20% <ø> (+0.17%) ⬆️
chain/client/src/lib.rs 100.00% <ø> (ø)
chain/client/src/view_client.rs 68.29% <ø> (-0.19%) ⬇️
neard/src/lib.rs 68.46% <80.00%> (+0.79%) ⬆️
chain/chain/src/store.rs 86.83% <87.69%> (-0.05%) ⬇️
chain/chain/tests/gc.rs 98.38% <100.00%> (+<0.01%) ⬆️
chain/client/src/gc_actor.rs 100.00% <100.00%> (ø)
chain/client/tests/process_blocks.rs 90.01% <100.00%> (+0.22%) ⬆️
core/chain-configs/src/client_config.rs 98.43% <100.00%> (+0.05%) ⬆️
... 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 c162dc3...ec4ecbf. Read the comment docs.

@mikhailOK
Copy link
Contributor

We can write the following test:

  • make a contract takes a key as input, and writes key->"bowen" in its storage or deletes key
  • call it in every block writing to some key or deleting
  • run it for a while, let gc delete some blocks
  • delete all keys, then stop doing writes and wait for gc get to this height
  • in theory we should crash with a 50% chance because of storage corruption

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.

4 participants