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/store): store eds in memory for index builder #2409

Merged

Conversation

walldiss
Copy link
Member

Overview

Previously generated eds was first store to disk and then was read again while building car index for dagstore.
This PR contains 2 changes:

  • writes eds into buffer to write it as 1 chunk instead of many small writes to disk to reduce disk i/o pressure.
  • introduces in-memory cache that will be used to read eds for building car index.

Combined changes brings 3x performance improvement for eds.Put operation

@walldiss walldiss added area:shares Shares and samples area:storage kind:feat Attached to feature PRs labels Jun 28, 2023
@walldiss walldiss self-assigned this Jun 28, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2023

Codecov Report

Merging #2409 (e90458b) into storage_performance_improvements (beaf6db) will decrease coverage by 0.44%.
The diff coverage is 29.90%.

@@                         Coverage Diff                          @@
##           storage_performance_improvements    #2409      +/-   ##
====================================================================
- Coverage                             53.28%   52.84%   -0.44%     
====================================================================
  Files                                   156      156              
  Lines                                  9915     9995      +80     
====================================================================
- Hits                                   5283     5282       -1     
- Misses                                 4179     4248      +69     
- Partials                                453      465      +12     
Impacted Files Coverage Δ
cmd/celestia/rpc.go 14.63% <0.00%> (ø)
das/metrics.go 9.75% <0.00%> (-0.17%) ⬇️
header/headertest/testing.go 71.92% <0.00%> (-0.56%) ⬇️
nodebuilder/p2p/genesis.go 0.00% <ø> (ø)
nodebuilder/p2p/network.go 57.89% <ø> (ø)
share/empty.go 0.00% <0.00%> (ø)
share/getters/shrex.go 66.66% <0.00%> (-0.77%) ⬇️
share/p2p/discovery/metrics.go 11.53% <0.00%> (ø)
share/p2p/peers/metrics.go 9.93% <0.00%> (-0.32%) ⬇️
state/metrics.go 0.00% <0.00%> (ø)
... and 16 more

... and 4 files with indirect coverage changes

@walldiss walldiss force-pushed the store_in_memory_index_builder branch from 3e4d133 to 886ccfa Compare June 28, 2023 14:19
Copy link
Collaborator

@distractedm1nd distractedm1nd left a comment

Choose a reason for hiding this comment

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

Mount type must be registered on registry inside edsstore constructor

@walldiss walldiss force-pushed the store_in_memory_index_builder branch from 6cf5a10 to f784a9d Compare July 4, 2023 08:02
@walldiss walldiss changed the base branch from main to storage_performance_improvements July 5, 2023 11:54
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, fixes an old #2018

share/eds/store.go Outdated Show resolved Hide resolved
share/eds/store.go Outdated Show resolved Hide resolved
@walldiss walldiss requested a review from Wondertan July 10, 2023 15:32
Wondertan
Wondertan previously approved these changes Jul 14, 2023
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.

It would be nice to merge this one as it's not breaking, yet very valuable

@walldiss walldiss changed the base branch from storage_performance_improvements to v0.11.0-rc9 August 1, 2023 12:30
@walldiss walldiss dismissed Wondertan’s stale review August 1, 2023 12:30

The base branch was changed.

@walldiss walldiss merged commit 21cb185 into celestiaorg:v0.11.0-rc9 Aug 2, 2023
12 of 14 checks passed
walldiss added a commit to walldiss/celestia-node that referenced this pull request Aug 3, 2023
…#2409)

## Overview

Previously generated eds was first store to disk and then was read again
while building car index for dagstore.
This PR contains 2 changes:
- writes eds into buffer to write it as 1 chunk instead of many small
writes to disk to reduce disk i/o pressure.
- introduces in-memory cache that will be used to read eds for building
car index.

Combined changes brings 3x performance improvement for eds.Put operation
walldiss added a commit to walldiss/celestia-node that referenced this pull request Aug 4, 2023
…#2409)

## Overview

Previously generated eds was first store to disk and then was read again
while building car index for dagstore.
This PR contains 2 changes:
- writes eds into buffer to write it as 1 chunk instead of many small
writes to disk to reduce disk i/o pressure.
- introduces in-memory cache that will be used to read eds for building
car index.

Combined changes brings 3x performance improvement for eds.Put operation
renaynay pushed a commit to renaynay/celestia-node that referenced this pull request Aug 23, 2023
…#2409)

## Overview

Previously generated eds was first store to disk and then was read again
while building car index for dagstore.
This PR contains 2 changes:
- writes eds into buffer to write it as 1 chunk instead of many small
writes to disk to reduce disk i/o pressure.
- introduces in-memory cache that will be used to read eds for building
car index.

Combined changes brings 3x performance improvement for eds.Put operation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:shares Shares and samples area:storage kind:feat Attached to feature PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants