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

Two connection events for single peer? #44

Closed
KrishnaPG opened this issue Nov 23, 2019 · 5 comments
Closed

Two connection events for single peer? #44

KrishnaPG opened this issue Nov 23, 2019 · 5 comments

Comments

@KrishnaPG
Copy link

KrishnaPG commented Nov 23, 2019

Trying to understand the behavior of connection by running the sample code from ReadMe. Not sure if this is the intended behavior, but when running the below code (the same code twice from two different terminals), seeing two connection events on each of them (total 4).

const hyperswarm = require('hyperswarm')
const crypto = require('crypto')
const swarm = hyperswarm()
const topic = crypto.createHash('sha256').update('my-hyperswarm-topic').digest()
swarm.join(topic, {lookup: true, announce: true})
swarm.on('connection', (socket, details) => {
  console.log('new connection!', details.peer)
})

The ouptut looks like below on both terminals:

new connection! {
  port: 1002,
  host: '192.168.0.101',
  local: true,
  referrer: null,
  topic: <Buffer fc f8 54 23 fe 2d 35 c2 a2 5f f5 90 62 7d ed b0 17 aa 2f b7 5a 76 eb 1d 69 ca 74 e6 40 43 db 41>
}
new connection! null

This happens with every connected peer. Two connection events are raised on both sides of the connection (total 4 for each p2p connection).

Is this valid behavior?

Do we need server instance on both sides of a connection? Sockets are by default duplex right?

A single client -> server connection between two nodes can be fully duplex, I believe. Do we need the additional reverse server <- client connection also for every p2p?

Or is it expected form the app that it will kill / close the duplicate connection? Would like to learn more about the right way to deal with these duplicate connections, since these connections count are rapidly escalating as we connect across multiple topics creating unnecessary resource pressure on the process (which can be better utilized for other connections, such as database connections or other app-level things). Especially on resource contraint devices.

@KrishnaPG
Copy link
Author

Sorry for this additional post, but I am kind of completely stuck and unable to make progress. It is not clear how to use these "connections".

For each peer, we get a 'connection' event with a socket. What is the recommended usage pattern afterwards?

  • Is it expected that all these sockets are put in a list/array and send a message to all of them? Or is it allowed to just keep one socket and discard the others? (But closing a socket is leading to indefinite reconnects, so I am not sure if that works. Issue explained here)
  • If we get multiple connection events, it essentially means we have multiple peers that offer the same "topic". But what if I am not interested in all of them? Is there a way to specify "how many" I am looking for, or "whitelist" / "blacklist" etc?

Also, regarding the topics:

  • Can the swarm.join be called repeatedly multiple times to join different topics, or should different swarm object be created once for each topic? Will duplicate connections to same peer be automatically removed in both cases?

@LuKks
Copy link
Contributor

LuKks commented Nov 24, 2019

I also want to know why there is a double connection but the real problem is that the extra connection doesn't actually disconnect and the total connections still increasing.
discovery-swarm doesn't have this problem but Maf said that hyperswarm is the successor.

app.js:

const hyperswarm = require('hyperswarm')
const crypto = require('crypto')

const swarm = hyperswarm()

const topic = crypto.createHash('sha256')
  .update('my-hyperswarm-topic-a59fa874')
  .digest()

const isLookup = process.argv[2] === 'lookup' ? true : false

swarm.join(topic, {
  lookup: isLookup,
  announce: !isLookup
})

swarm.on('connection', (socket, details) => {
  console.log('connection', details.stream.remoteAddress, 'size', swarm.connections.size)

  socket.on('close', () => {
    console.log('close', details.stream.remoteAddress, 'size', swarm.connections.size)
  })
})

// same as hyperssh
process.once('SIGINT', function () {
  swarm.once('close', function () {
    process.exit()
  })
  swarm.destroy()
  setTimeout(() => process.exit(), 2000)
})

Run in two consoles: node app announce and node app lookup.
The announce output:

connection ::ffff:192.168.0.2 size 1
connection 167.249.197.179 size 2

There you have the double connection, the remote address is actually my address.
Do ctrl+c on the lookup app and run again node app lookup.
The new announce output:

close ::ffff:192.168.0.2 size 1
connection ::ffff:192.168.0.2 size 2
connection 167.249.197.179 size 3

And you can repeat it and the connections size still increasing.

@mafintosh
Copy link
Contributor

A quick note to please open separate issues for separate problems.

It autoreconnects and you have to use details.backoff() to make it not connect immediately again (needs docs) https://github.com/hyperswarm/hyperswarm/blob/master/lib/peer-info.js#L41

In the near future we'll add connection dedup support, but in the meanwhile you can work around it using that api if you need too. For hypercores atm we just use both of the duplicate connections

@LuKks LuKks mentioned this issue Nov 25, 2019
@LuKks
Copy link
Contributor

LuKks commented Nov 25, 2019

Is this valid behavior?
But closing a socket is leading to indefinite reconnects, so I am not sure if that works.

I was checking the double connection and one is tcp and the other is utp.
So if I understand correctly you can backoff and destroy the utp. I choose destroy the utp because is the one that is causing me problems.

swarm.on('connection', (socket, details) => {
  if (details.type === 'utp') {
    details.backoff()
    socket.destroy()
    return
  }
  // ...
})

Is it expected that all these sockets are put in a list/array and send a message to all of them?

I think yes, you already have swarm.connections, maybe we can add swarm.broadcast('my data').

for (let peer of swarm.connections) {
  peer.write('my data');
}

Is there a way to specify "how many" I am looking for?

options.maxPeers? Check the readme.

"whitelist" / "blacklist" etc?

I also need whitelist #46

Can the swarm.join be called repeatedly multiple times to join different topics?

As far as I can test yes. You have 'connection' event with details.topics and details.peer.topic.

@mafintosh
Copy link
Contributor

there is a dedup api for this now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants