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

Move providing responsabilities from bitswap to blockservice #534

Closed
wants to merge 5 commits into from
Closed

Move providing responsabilities from bitswap to blockservice #534

wants to merge 5 commits into 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
@codecov Codecov
Copy link

codecov bot commented Jan 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 49.15254% with 30 lines in your changes missing coverage. Please review.

Project coverage is 65.45%. Comparing base (79cb4e2) to head (4060154).
Report is 8 commits behind head on revert-bad-sessions-changes.

Files with missing lines Patch % Lines
blockservice/blockservice.go 55.00% 13 Missing and 5 partials ⚠️
blockservice/providing_blockstore.go 14.28% 11 Missing and 1 partial ⚠️

Impacted file tree graph

@@                       Coverage Diff                       @@
##           revert-bad-sessions-changes     #534      +/-   ##
===============================================================
- Coverage                        65.56%   65.45%   -0.12%     
===============================================================
  Files                              207      208       +1     
  Lines                            25597    25511      -86     
===============================================================
- Hits                             16782    16697      -85     
+ Misses                            7336     7332       -4     
- Partials                          1479     1482       +3
Files with missing lines 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%) ⬇️
gateway/blocks_backend.go 42.23% <ø> (+0.13%) ⬆️
blockservice/providing_blockstore.go 14.28% <14.28%> (ø)
blockservice/blockservice.go 74.41% <55.00%> (-3.82%) ⬇️

... and 11 files with indirect coverage changes

This was referenced Jan 3, 2024
Open
Closed
@Jorropo Jorropo force-pushed the move-providing-responsabilities branch from 10aa04b to 29ff0cc Compare January 5, 2024 15:56
Jorropo added a commit to ipfs/kubo that referenced this pull request Jan 5, 2024
@Jorropo
chore: update boxo for bitswap providing refactor
732d56c 
Keeps in sync with:
- ipfs/boxo#534
- ipfs/boxo#535
- ipfs/boxo#536
@Jorropo Jorropo mentioned this pull request Jan 5, 2024
Draft
@Jorropo
Copy link
Contributor Author

Jorropo commented Jan 11, 2024 •
edited
Loading

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 mentioned this pull request Jan 11, 2024
Closed
@Jorropo Jorropo force-pushed the move-providing-responsabilities branch from 29ff0cc to 94be73e Compare January 11, 2024 18:10
@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
This was referenced Jan 16, 2024
Merged
Merged
Merged
Closed
Closed
Closed
Merged
Closed
Open
Open
@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 mentioned this pull request Feb 14, 2024
Open
@Jorropo Jorropo force-pushed the move-providing-responsabilities branch from 2b1c552 to f060dd4 Compare February 16, 2024 14:00
@Jorropo Jorropo mentioned this pull request Feb 16, 2024
Closed
@Jorropo
Revert "blockservice: make ContextWithSession shortcut grabSessionFro…
4b8ade1 
…mContext inside newSession"

Supperseeded by #570

This reverts commit ea04c77.
@Jorropo
blockservice: remove session embeding in context
d8d7675 
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.
@Jorropo
blockservice: add WithProvider option
ff3ac26 
This allows to recreate the behavior of advertising added blocks the bitswap server used to do.
@Jorropo
bitswap/server: remove provide
047fce0 
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
blockservice: add session workaround to work with wrapped blockservices
4060154
@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
@Jorropo
chore: update boxo for bitswap providing refactor
dae471e 
Keeps in sync with:
- ipfs/boxo#578
- ipfs/boxo#534
- ipfs/boxo#535
- ipfs/boxo#536
gammazero added a commit that referenced this pull request Sep 30, 2024
@gammazero
Move providing responsabilities from bitswap to blockservice
29faea9 
- bitswap/server: remove provide
- blockservice: add session workaround to work with wrapped blockservices
- blockservice: add WithProvider option
- blockservice: remove session embeding in context

Replaces #534 by @Jorropo which was outdated enough to make merging difficult.
@gammazero gammazero mentioned this pull request Sep 30, 2024
Closed
@gammazero
Copy link
Contributor

Replaced by #677

@gammazero gammazero closed this Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hacdias hacdias hacdias approved these changes

@lidel lidel Awaiting requested review from lidel lidel is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jorropo @gammazero @hacdias