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

providers: don't add every connected node as a provider #59

Merged
merged 1 commit into from
Feb 6, 2019

Conversation

Stebalien
Copy link
Member

We now do exactly what the comment is warning about: track peers providing keys.

NOTE: This should be tested against the live network.

@ghost ghost assigned Stebalien Jan 22, 2019
@ghost ghost added the status/in-progress In progress label Jan 22, 2019
@Stebalien
Copy link
Member Author

I believe (untested) the tests are failing because we're never sending out a want request due to a lack of peers. The failing test is TestSessionFindMorePeers.

@hannahhoward
Copy link
Contributor

@Stebalien this looks LGTM to me. But I have no idea the affect it will have in practice. I wonder if we should wait at least until the non-sessions GetBlocks & providerqueryworker is removed (see ProviderQueryManager PR) -- in the case of sessions we only call FindProvidersAsync when we don't have any peers already that are returning the given block, so I agree that it wouldn't make sense to send these, though I don't believe it will do any harm.

TestSessionFindMorePeers does not actually use any kind of a real network (the mock one is right in the test) so I don't think we are affected by this however.

@hannahhoward
Copy link
Contributor

I need to do a test reliability pass -- see #61 -- I didn't realize it was possible to run tests locally multiple times in a row.

@Stebalien
Copy link
Member Author

SGTM. We can sit on this a bit.

@hannahhoward
Copy link
Contributor

@Stebalien based on our experience with Gateway tracing (that this results in a huge number of spurious attempts to connect to peers), I would like to go ahead and merge this after a bit more live testing.

@Stebalien
Copy link
Member Author

SGTM.

Note: I'm not sure if this was causing us to actually connect to many new peers as it returns peers we're already connected to. But, IMO, we should still go with this. Otherwise, sessions don't quite work correctly.

We now do exactly what the comment is warning about: track peers providing keys.
@hannahhoward hannahhoward merged commit 4038218 into master Feb 6, 2019
@ghost ghost removed the status/in-progress In progress label Feb 6, 2019
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
providers: don't add every connected node as a provider

This commit was moved from ipfs/go-bitswap@4038218
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.

2 participants