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.

Sign up for GitHub

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

Jump to bottom

eds/store: Fix ShardAccessor caching for use in GetCAR and CARBlockstore #1514

Closed
Tracked by #1099
distractedm1nd opened this issue Dec 20, 2022 · 0 comments · Fixed by #2000
Closed
Tracked by #1099

eds/store: Fix ShardAccessor caching for use in GetCAR and CARBlockstore #1514

distractedm1nd opened this issue Dec 20, 2022 · 0 comments · Fixed by #2000
Labels
area:shares Shares and samples bug Something isn't working

Comments

@distractedm1nd
Copy link
Collaborator

In #1511 we stopped using the cached ShardAccessor on every GetCAR and CARBlockstore request, as we realized it shares an underlying reader with other callers.

This caching of the reader was made possible with celestiaorg/dagstore#1, but new modifications need to be made in order to protect the reader from both exhaustion and concurrent reads.

One proposed solution from @walldiss is to use io.TeeReader with concurrency protection, making sure a copy is saved into a buffer for subsequent reads.
@Wondertan Wondertan added the bug Something isn't working label Dec 20, 2022
@Wondertan Wondertan mentioned this issue Dec 20, 2022
Closed
45 tasks
This was referenced Apr 1, 2023
Merged
Merged
distractedm1nd added a commit that referenced this issue Apr 4, 2023
@distractedm1nd @renaynay @Wondertan
fix(share/eds): use cached accessors in GetCAR and GetDAH (#2000)
d97e634 
Closes #1514 . This PR enables accessor cache usage for GetCAR and
GetDAH. This will allow shrexeds and shrexnd servers to only require
opening the underlying file once (until removed from cache). If many
peers request the EDS at once, the server previously needed to open the
file each time.

Will still require a dependency bump of our dagstore fork for the new
test to not fail - so draft until then. Related:
celestiaorg/dagstore#2

---------

Co-authored-by: rene <41963722+renaynay@users.noreply.github.com>
Co-authored-by: Hlib Kanunnikov <hlibwondertan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:shares Shares and samples bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(share/eds): use cached accessors in GetCAR and GetDAH distractedm1nd/celestia-node
2 participants
@distractedm1nd @Wondertan