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

Move providing responsabilities from bitswap to blockservice #534

Open
wants to merge 5 commits into
base: revert-bad-sessions-changes
Choose a base branch
from

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Jan 3, 2024

No description provided.

@Jorropo Jorropo requested a review from a team as a code owner January 3, 2024 17:55
@Jorropo Jorropo changed the title Move providing responsabilities Move providing responsabilities from bitswap to blockservice Jan 3, 2024
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (d8d7675) 65.64% compared to head (4060154) 65.45%.

Impacted file tree graph

@@                       Coverage Diff                       @@
##           revert-bad-sessions-changes     #534      +/-   ##
===============================================================
- Coverage                        65.64%   65.45%   -0.20%     
===============================================================
  Files                              207      208       +1     
  Lines                            25564    25511      -53     
===============================================================
- Hits                             16781    16697      -84     
- Misses                            7310     7332      +22     
- Partials                          1473     1482       +9     
Files Coverage Δ
bitswap/bitswap.go 69.51% <ø> (+2.07%) ⬆️
bitswap/options.go 41.17% <ø> (-3.27%) ⬇️
bitswap/server/server.go 58.50% <100.00%> (-7.97%) ⬇️
blockservice/providing_blockstore.go 14.28% <14.28%> (ø)
blockservice/blockservice.go 74.41% <55.00%> (-2.77%) ⬇️

... and 12 files with indirect coverage changes

@Jorropo
Copy link
Contributor Author

Jorropo commented Jan 11, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Jorropo and the rest of your teammates on Graphite Graphite

@Jorropo Jorropo changed the base branch from main to 01-15-blockservice_remove_inner_fields_access_methods January 16, 2024 09:05
@Jorropo Jorropo force-pushed the move-providing-responsabilities branch from 94be73e to ac0f6e0 Compare January 16, 2024 09:05
@Jorropo Jorropo requested a review from lidel as a code owner January 16, 2024 09:05
@Jorropo Jorropo force-pushed the move-providing-responsabilities branch from ac0f6e0 to e0d5341 Compare February 14, 2024 18:40
@Jorropo Jorropo changed the base branch from 01-15-blockservice_remove_inner_fields_access_methods to revert-bad-sessions-changes February 14, 2024 18:40
@Jorropo Jorropo force-pushed the move-providing-responsabilities branch from 2b1c552 to f060dd4 Compare February 16, 2024 14:00
…mContext inside newSession"

Supperseeded by #570

This reverts commit ea04c77.
This brings us to a state before #549 back then I also did cleanup in this session code, that I have kept, I only removed the sessions in context feature.
This allows to recreate the behavior of advertising added blocks the bitswap server used to do.
We always had a very weird relationship between bitswap and providing.
Bitswap took care of doing the initial provide and then reprovider did it later.
The Bitswap server had a complicated providing workflow where it slurped thing into memory.

Reprovide accepts provides and is able to queue them in a database, such as on disk, this is much better.

I'll add options to hook initial provide logic from the blockservice to the reprovider queue so consumers don't have to do this themselves.
@Jorropo Jorropo force-pushed the move-providing-responsabilities branch from f060dd4 to 4060154 Compare February 16, 2024 14:09
Jorropo added a commit to ipfs/kubo that referenced this pull request Feb 16, 2024
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.

None yet

2 participants