Skip to content

[ipfs/go-bitswap] Failure to fetch content from connected nodes #83

Open
Open
[ipfs/go-bitswap] Failure to fetch content from connected nodes#83
Labels
P2Medium: Good to have, but can wait until someone steps uphelp wantedSeeking public contribution on this issue

Description

@vdamle
vdamle
opened on Aug 30, 2021

Environment

IPFS Version:

go-ipfs version: 0.9.1-dc2715a
Repo version: 11
System version: amd64/linux
Golang version: go1.15.2

We are running a private IPFS cluster with 2 or more nodes (docker/k8s)

Description

As an example, we've got a network with 2 nodes - node1 and node2. After startup, we observe that node2 cannot fetch content from node1 (a bootstrap peer) because it is not registered as a Partner. We see on node1 that node2 is registered as a Peer/Partner. I tried adding some additonal debug logs and best I could tell, the steps to register a BitSwapNetwork Receiver to receive events from the network was not complete before node2 received a "connected" notification from node1. It seems to me like this is a race condition and we see this occur 90% of the time, when we spin up nodes (ref: https://github.com/ipfs/go-bitswap/blob/0e81f7c1fef795a8d34cdd494fa2d8fdc045d199/bitswap.go#L215 ). As a throw of dice, I tried to move up the call before any options/decision engine is setup, this did not make a whole lot of difference to the outcome.

Workaround

  • Execute ipfs swarm peer disconnect <address>
  • Execute ipfs swarm peer connect <address>

We can see that the PeerConnected handler is invoked from Connected, which adds the Peer to the peer list. After this, WANT_HAVE is sent as expected to the peer and content is retrieved.

PS: Originally, I created the following issue in go-ipfs, but after further investigation, it feels like go-bitswap is the right place to report this.

I'd love to hear any suggestions/thoughts for how this can be addressed since I have a limited view of the architecture/intent for how connections are supposed to work. I have attached logs and output of commands from both nodes.

ipfs-config-node1.txt
ipfs-config-node2.txt
ipfs-id-node1.txt
ipfs-id-node2.txt
ipfs-node1.log
ipfs-node2.log

Activity

  1. vdamle
    added
    need/triageNeeds initial labeling and prioritization
    on Aug 30, 2021

  2. welcome commented on Aug 30, 2021

    @welcome
    welcomebot
    on Aug 30, 2021

    Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
    In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment.
    Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

    • "Priority" labels will show how urgent this is for the team.
    • "Status" labels will show if this is ready to be worked on, blocked, or in progress.
    • "Need" labels will indicate if additional input or analysis is required.

    Finally, remember to use https://discuss.ipfs.io if you just need general support.

  3. Stebalien commented on Sep 9, 2021

    @Stebalien
    Stebalien
    on Sep 9, 2021
    Member

    We usually handle this by setting up the node as follows:

    1. Instantiate the libp2p node, but don't start listening on addresses.
    2. Instantiate any services that need libp2p.
    3. Finally, start listening.

    However, ideally bitswap would add all existing peers on startup. If you want to write a quick patch to do that, I'd be happy to take a look.

  4. BigLep
    added
    need/author-inputNeeds input from the original author
    and removed
    need/triageNeeds initial labeling and prioritization
    on Sep 24, 2021

  5. BigLep commented on Sep 24, 2021

    @BigLep
    BigLep
    on Sep 24, 2021
    Contributor

    @vdamle : is this a patch you're interested in contributing?

  6. vdamle commented on Sep 24, 2021

    @vdamle
    vdamle
    on Sep 24, 2021
    Author

    Sorry, I missed the update @BigLep / @Stebalien - thanks for the input!

    I'd love to contribute a patch.

    @Stebalien : Here's what I'm thinking:

    Please let me know if I'm on the right track. In the meantime, I will try to make the above changes and test in my private network deployment.

  7. Stebalien commented on Sep 26, 2021

    @Stebalien
    Stebalien
    on Sep 26, 2021
    Member

    So, I tried to implement it and realized this just isn't going to work, or at least not "well" because your libp2p host may have received connections before it's listening on the bitswap protocol.

    The correct fix is to:

    1. Wait for "identify" and "protocol changed" events on the libp2p event bus instead of connect events. That way, two peers will only "see" each other once they start listening on the bitswap protocol, not when they first connect.
    2. On start, iterate through all peers and check to see if they speak the bitswap protocol. If so, simulate an event as in step 1.

    But this needs to be done carefully to make sure it's somewhat atomic.

  8. BigLep commented on Oct 1, 2021

    @BigLep
    BigLep
    on Oct 1, 2021
    Contributor

    @vdamle: do you have a better idea of what a patch would need to look like given @Stebalien 's feedback?

  9. BigLep
    added
    help wantedSeeking public contribution on this issue
    P2Medium: Good to have, but can wait until someone steps up
    on Jan 7, 2022

  10. BigLep commented on Jan 7, 2022

    @BigLep
    BigLep
    on Jan 7, 2022
    Contributor

    @vdamle : is this something you'd be to contribute?

    2022-01-07 conversation: @aschmahmann ran into this as well in 202112 while working on Testground/integration testing. Not fixing this means tests have to work around this issue.

  11. BigLep
    removed
    need/author-inputNeeds input from the original author
    on Mar 25, 2022

  12. peterargue commented on Oct 11, 2022

    @peterargue
    peterargue
    on Oct 11, 2022

    @BigLep @Stebalien I took a stab at this in ipfs/go-bitswap#590. Am I on the right track?

  13. BigLep
    added
    need/author-inputNeeds input from the original author
    on Oct 11, 2022

  14. github-actions commented on Oct 18, 2022

    @github-actions
    github-actionsbot
    on Oct 18, 2022

    Oops, seems like we needed more information for this issue, please comment with more details or this issue will be closed in 7 days.

  15. peterargue commented on Oct 18, 2022

    @peterargue
    peterargue
    on Oct 18, 2022

    what additional information is needed?

  16. BigLep commented on Oct 18, 2022

    @BigLep
    BigLep
    on Oct 18, 2022
    Contributor

    @peterargue : apologies for the confusion. It looks like we need to do some bot adjustments for when the ball is back in the maintainer's court. We'll need maintainers to look at this (next triage is scheduled for 2022-10-20).

  17. peterargue commented on Oct 18, 2022

    @peterargue
    peterargue
    on Oct 18, 2022

    @BigLep no worries and thanks for the update

  18. peterargue commented on Nov 24, 2022

    @peterargue
    peterargue
    on Nov 24, 2022

    @BigLep any idea when the team will have some time to take a look at this PR? FWIW, we've been running with these changes on a live network for the last 4 weeks with great results. All of the correct peers are added on startup and updated as expected.

  19. BigLep commented on Nov 27, 2022

    @BigLep
    BigLep
    on Nov 27, 2022
    Contributor

    Hi @peterargue : I personally haven't been at the most recent triage meetings to give more insight.

    @guseggert : has this come up during weekly triage? Do we have any initial feedback here?

  20. Jorropo
    changed the title [-]Failure to fetch content from connected nodes[/-] [+][ipfs/go-bitswap] Failure to fetch content from connected nodes[/+] on Jan 27, 2023

  21. welcome commented on Jan 27, 2023

    @welcome
    welcomebot
    on Jan 27, 2023

    Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
    In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment.
    Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

    • "Priority" labels will show how urgent this is for the team.
    • "Status" labels will show if this is ready to be worked on, blocked, or in progress.
    • "Need" labels will indicate if additional input or analysis is required.

    Finally, remember to use https://discuss.ipfs.io if you just need general support.

  22. Jorropo
    transferred this issue fromipfs/go-bitswapon Jan 27, 2023

  23. github-actions commented on Feb 3, 2023

    @github-actions
    github-actionsbot
    on Feb 3, 2023

    Oops, seems like we needed more information for this issue, please comment with more details or this issue will be closed in 7 days.

  24. github-actions
    added
    kind/stale
    on Feb 3, 2023
  25. BigLep
    removed
    need/author-inputNeeds input from the original author
    kind/stale
    on Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Medium: Good to have, but can wait until someone steps uphelp wantedSeeking public contribution on this issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions