Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: reprovider #2184

Closed
wants to merge 4 commits into from
Closed

feat: reprovider #2184

wants to merge 4 commits into from

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Jun 17, 2019

In the context of js-ipfs reannouncing blocks to the network ipfs/js-ipfs#2160, this PR aims to add a Provider layer controlled by js-ipfs. This provider will be used by bitswap.

Needs:

@vasco-santos
Copy link
Member Author

@alanshaw @dirkmc could I get your feedback on this PR?

It is important to also look at the bitswapPR in this context.

FYI, for getting this to the finish line I only need to add the proper logic for the reprovide strategies. However, there is some code in Dirk's PR for the garbage collection ipfs/js-ipfs#2022 that would be great to use for this. So, it would be great to have that merged first.

src/core/provider/index.js Outdated Show resolved Hide resolved
src/core/provider/index.js Outdated Show resolved Hide resolved
src/core/provider/index.js Outdated Show resolved Hide resolved
src/core/provider/index.js Show resolved Hide resolved
src/core/provider/queue.js Show resolved Hide resolved
src/core/provider/reprovider.js Outdated Show resolved Hide resolved
src/core/provider/reprovider.js Show resolved Hide resolved
src/core/provider/reprovider.js Show resolved Hide resolved
src/core/provider/reprovider.js Show resolved Hide resolved
src/core/provider/reprovider.js Outdated Show resolved Hide resolved
@dirkmc
Copy link
Contributor

dirkmc commented Jun 27, 2019

Looks like it's on the right track @vasco-santos 👍
I left a few comments for minor things in the code.

I wonder if it would make sense to persist the time at which the last provide occurred, so that if a node restarts it doesn't wait the full 12 hours to reprovide?

@vasco-santos vasco-santos force-pushed the feat/reprovider branch 5 times, most recently from 998deac to 89fbff3 Compare June 28, 2019 10:32
@vasco-santos
Copy link
Member Author

Thanks @dirkmc ! I think that I addressed all your comments!

Regarding persisting the time of the last provide, the first reprovide will happen once the node starts, after a short delay. So, I think it should not be necessary

src/core/provider/index.js Outdated Show resolved Hide resolved
src/core/components/start.js Outdated Show resolved Hide resolved
src/core/provider/index.js Outdated Show resolved Hide resolved
src/core/provider/reprovider.js Outdated Show resolved Hide resolved
src/core/provider/reprovider.js Outdated Show resolved Hide resolved
src/core/provider/reprovider.js Show resolved Hide resolved
@vasco-santos vasco-santos force-pushed the feat/reprovider branch 2 times, most recently from 02d65c7 to 98723a7 Compare June 28, 2019 15:52
@alanshaw
Copy link
Member

@vasco-santos what's the status of this PR - still draft?

@alanshaw alanshaw mentioned this pull request Aug 27, 2019
10 tasks
@vasco-santos
Copy link
Member Author

I was waiting on #2022 getting merged, which had a lot of useful code to integrate here without duplicating.

@alanshaw alanshaw mentioned this pull request Aug 27, 2019
66 tasks
@achingbrain achingbrain mentioned this pull request Sep 25, 2019
52 tasks
@daviddias
Copy link
Member

Hi! js-ipfs master just got a whole new set of automated tests with #2528, #2440 and also running some of the test suites from our early testers (hi5 to @achingbrain for setting it all up!). Would you mind rebasing the master branch on this PR to ensure it runs all the latest tests? Thank you!

@achingbrain
Copy link
Member

I'm going to close this because I think since the .provide method lives on libp2p's ContentRouter interface, re-provides should be handled by the various implementations of content routers since they all may have different rules about provider record validity, etc.

Some may call out to a remote service that does the re-provide for us, for example, or some may have shorter lived records than others, who knows!

@achingbrain achingbrain closed this Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants