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

feat(share): Periodic GC over EDSStore #1359

Merged
merged 6 commits into from
Dec 6, 2022

Conversation

distractedm1nd
Copy link
Collaborator

@distractedm1nd distractedm1nd commented Nov 14, 2022

PULL REQUEST

Overview

This PR implements a goroutine that runs during the lifetime of the EDSStore, which calls dgstr.GC periodically. This garbage collection cleans up shards from transient storage that are available but inactive or errored.

The PR is based on #1232 and closes #1116.

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@distractedm1nd distractedm1nd self-assigned this Nov 14, 2022
@distractedm1nd distractedm1nd changed the title feat(share): EDSStore definition feat(share): Periodic GC over EDSStore Nov 14, 2022
@distractedm1nd distractedm1nd added area:shares Shares and samples kind:feat Attached to feature PRs labels Nov 14, 2022
@distractedm1nd distractedm1nd marked this pull request as ready for review November 15, 2022 09:36
Wondertan
Wondertan previously approved these changes Nov 28, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2022

Codecov Report

Merging #1359 (c153997) into main (b4a8145) will increase coverage by 0.78%.
The diff coverage is 83.15%.

@@            Coverage Diff             @@
##             main    #1359      +/-   ##
==========================================
+ Coverage   55.28%   56.07%   +0.78%     
==========================================
  Files         180      188       +8     
  Lines       10904    11573     +669     
==========================================
+ Hits         6028     6489     +461     
- Misses       4276     4447     +171     
- Partials      600      637      +37     
Impacted Files Coverage Δ
share/eds/store.go 51.23% <73.33%> (+3.03%) ⬆️
nodebuilder/p2p/p2p.go 85.18% <85.18%> (ø)
api/rpc/client/client.go 78.57% <100.00%> (+0.79%) ⬆️
nodebuilder/p2p/flags.go 67.46% <100.00%> (ø)
nodebuilder/p2p/host.go 90.90% <100.00%> (+9.65%) ⬆️
nodebuilder/p2p/module.go 93.75% <100.00%> (+0.20%) ⬆️
header/core/exchange.go 40.90% <0.00%> (-9.10%) ⬇️
header/local/exchange.go 61.90% <0.00%> (-6.52%) ⬇️
header/p2p/server.go 66.11% <0.00%> (-5.57%) ⬇️
header/store/store.go 62.26% <0.00%> (-3.86%) ⬇️
... and 36 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@distractedm1nd
Copy link
Collaborator Author

The GC test is racy! will try to fix today

Wondertan
Wondertan previously approved these changes Dec 6, 2022
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

LGTM

renaynay
renaynay previously approved these changes Dec 6, 2022
@distractedm1nd distractedm1nd dismissed stale reviews from renaynay and Wondertan via c153997 December 6, 2022 12:57
renaynay
renaynay previously approved these changes Dec 6, 2022
Wondertan
Wondertan previously approved these changes Dec 6, 2022
@distractedm1nd distractedm1nd merged commit 0af528a into celestiaorg:main Dec 6, 2022
renaynay pushed a commit to renaynay/celestia-node that referenced this pull request Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:shares Shares and samples kind:feat Attached to feature PRs
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

share: Periodic GC over DAGStore inside EDSStore
7 participants