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

Feature request to check if a connection to a peer already exists #6091

Open
remmerw opened this issue Mar 16, 2019 · 11 comments
Open

Feature request to check if a connection to a peer already exists #6091

remmerw opened this issue Mar 16, 2019 · 11 comments
Labels
kind/feature A new feature topic/icebox Topic icebox

Comments

@remmerw
Copy link

remmerw commented Mar 16, 2019

Version information:

All Versions

Type:

"feature": "ipfs swarm connect" with an option to check if a connection to a peer exists (but not open a new connection in case it does not exists) -> maybe a similar option exists already, but I was not able to find it)

Description:

Currently I use the function "ipfs swarm peers" to check if a connection to a peer exists.
But parsing all connected peers might be to not appropriate.

@Stebalien Stebalien added the kind/feature A new feature label Mar 18, 2019
@Stebalien
Copy link
Member

Thanks for the suggestion. At the moment, the only way to do this is to parse ipfs swarm peers. However, it may make sense to add an option to filter ipfs swarm peers by peer (the command is actually misnamed, it should be called ipfs swarm connections).

@momack2 momack2 added this to Inbox in ipfs/go-ipfs May 9, 2019
@michaelavila
Copy link
Contributor

michaelavila commented May 15, 2019

@remmerw What is the proposed interface for this change? I think this is a great introductory task if we can provide enough actionable definition.

@michaelavila michaelavila added good first issue Good issue for new contributors help wanted Seeking public contribution on this issue labels May 15, 2019
@remmerw
Copy link
Author

remmerw commented May 16, 2019

@michaelavila My assumption is that the command
ipfs swarm connect QmHash
already keeps the connection. (Since it is added by the user and has therefore a
higher priority then other connections)
The case here is, what if the connection is not existing anymore (e.g. one node has no network) .
How can the user of a node can check fast, if the connection is still valid
and existing to the other node.

@michaelavila michaelavila removed good first issue Good issue for new contributors help wanted Seeking public contribution on this issue labels May 16, 2019
@michaelavila
Copy link
Contributor

@remmerw apologies, I misunderstood. Do you have an interface proposal? You want to do more than just check that the peer is in your list, right? You want to do something to determine if the connection to that peer is still valid? Maybe by acknowledging some kind of message?

@remmerw
Copy link
Author

remmerw commented May 17, 2019

@michaelavila Yes, that is true, just checking if the peer is in the list, might be not be the "best" solution.
When I have a wish, I would like to have a kind of event mechanism.
When deep down, a connection failure is detected, it will be broadcasted via this event mechanism.
The user can listen on such events and can react.
Maybe the "ipfs swarm connect" returns also a kind of event ID, the user can then listen on such
event ID. So in case a connection is "broken" an event will be triggered to the user and also other
parts of the software can be notified.
[But I do not know the code, to say how expensive such an approach is. Just guessing that
it might be to expensive]
My general approach is only, the faster the user can be the informed, the better the solution might be.
Right now, I am polling if a connection to a peer still exist (via the function "pfs swarm peers")

@michaelavila
Copy link
Contributor

@remmerw understood. Polling the output of ipfs swarm peers is probably your best bet right now. Adding a flag to the command to filter down the list is an option, but if you're already filtering that list down then you're not going to get much from it.

@michaelavila michaelavila added the topic/icebox Topic icebox label May 21, 2019
@remmerw
Copy link
Author

remmerw commented May 26, 2019

Ok, after a quick evaluation of the source code.
One very simple approach would be, to add an additonal function to SwarmAPI interface

Interface :
https://github.com/ipfs/interface-go-ipfs-core/blob/master/swarm.go
type SwarmAPI interface {
// Connect to a given peer
Connect(context.Context, pstore.PeerInfo) error
IsConnected(context.Context, pstore.PeerInfo) (bool, error) *** NEW ***

....
}

Implementation :
https://github.com/ipfs/go-ipfs/blob/master/core/coreapi/swarm.go

*** NEW ***

func (api *SwarmAPI) IsConnected(ctx context.Context, pi pstore.PeerInfo) (bool, error) {
if api.peerHost == nil {
return false, coreiface.ErrOffline
}
id := peer.ID
net := api.peerHost.Network()
connected := net.Connectedness(id) == inet.Connected
return connected, nill
}

About the implementation I am not 100% sure if all is correct (but I hope you get the idea)

For me right now, such solution only on Core API level is fine (no need to add to HTTP API )
Still it is not the best solution, at least 1000 times better then checking all peers
with ipfs swarm peers

I would be very grateful to have such a functionality

@michaelavila
Copy link
Contributor

@remmerw what are the next steps for this approach? Can you write the test that this code you've posted will pass? Have you attempted to use the change you're proposing yet?

@remmerw
Copy link
Author

remmerw commented Jun 3, 2019

@michaelavila I have implemented it and it works (and also it is a little bit faster) but I have to admit
that this is still not the solution what I need. It still takes to much time till the network recoginizes
that a connection is not valid anymore (whatever reason that is). Still I am looking for a faster
solution, that when a connection is broken a notification will be send. (I am not sure whether this
is possible at all)

@michaelavila
Copy link
Contributor

@remmerw do you want to submit the changes you're using now as a PR? The notification-based change can be discussed further in this issue in the meantime.

@remmerw
Copy link
Author

remmerw commented Jun 5, 2019

@michaelavila Nope no intension to submit it, The solution which I thought was a solution, should be called correctly "IsConnectedOrNot" instead of "IsConnected" [wich make no sense]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A new feature topic/icebox Topic icebox
Projects
No open projects
Development

No branches or pull requests

3 participants