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

No need to poll #71

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

No need to poll #71

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
33124ba
feat: no need to poll
richardschneider Feb 2, 2018
18b7918
docs: broadcast defaults to true
richardschneider Feb 2, 2018
8f5d4e4
test: faster tests with smaller RSA key
richardschneider Feb 2, 2018
4cae8fa
test: linting
richardschneider Feb 2, 2018
8d057c9
fix: racey stop
richardschneider Feb 5, 2018
9dec015
fix: lint
richardschneider Feb 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ const mdns = new MDNS(peerInfo, options)
mdns.on('peer', (peerInfo) => {
console.log('Found a peer in the local network', peerInfo.id.toB58String())
})
process.on('exit', mdns.stop)

// Broadcast for 20 seconds
mdns.start(() => setTimeout(() => mdns.stop(() => {}), 20 * 1000))
// Listen for peers
mdns.start()
```

- options
- `broadcast` - (true/false) announce our presence through mDNS, default false
- `interval` - query interval, default 10 * 1000 (10 seconds)
- `broadcast` - (true/false) announce our presence through mDNS, default true
- `serviceTag` - name of the service announcedm, default 'ipfs.local`
9 changes: 2 additions & 7 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ class MulticastDNS extends EventEmitter {
options = options || {}

this.broadcast = options.broadcast !== false
this.interval = options.interval || (1e3 * 10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@richardschneider instead of removing the feature all together, it should support for a single beacon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure why you want a beacon?

interval controlled how often an MDNS query is sent. Every time it is sent, all other IPFS nodes must respond with their DNS records.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diasdavid any comments?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see a need to drop the interval beacon feature. The module should support both, single beacon and interval.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off, a beacon is not what MDNS is about. Secondly it can be very expensive.

Consider a data centre with 100 servers all running IPFS with an interval of 10 minutes, or 600 seconds. On average, a node will beacon every 6 seconds. Then every 6 seconds all other nodes must wake-up and reply with their DNS records. All nodes will then see all the nodes responses and have to then .emit discovered nodes. It's a real waste of network bandwidth and CPU.

this.serviceTag = options.serviceTag || 'ipfs.local'
this.port = options.port || 5353
this.peerInfo = peerInfo
this._queryInterval = null
}

start (callback) {
Expand All @@ -25,8 +23,6 @@ class MulticastDNS extends EventEmitter {

this.mdns = mdns

this._queryInterval = query.queryLAN(this.mdns, this.serviceTag, this.interval)

mdns.on('response', (event) => {
query.gotResponse(event, this.peerInfo, this.serviceTag, (err, foundPeer) => {
if (err) {
Expand All @@ -41,15 +37,14 @@ class MulticastDNS extends EventEmitter {
query.gotQuery(event, this.mdns, this.peerInfo, this.serviceTag, this.broadcast)
})

setImmediate(() => callback())
query.queryLAN(this.mdns, this.serviceTag)
setImmediate(callback)
}

stop (callback) {
if (!this.mdns) {
callback(new Error('MulticastDNS service had not started yet'))
} else {
clearInterval(this._queryInterval)
this._queryInterval = null
this.mdns.destroy(callback)
this.mdns = undefined
}
Expand Down
20 changes: 7 additions & 13 deletions src/query.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,13 @@ const tcp = new TCP()
module.exports = {

queryLAN: function (mdns, serviceTag, interval) {
const query = () => {
log('query', serviceTag)
mdns.query({
questions: [{
name: serviceTag,
type: 'PTR'
}]
})
}

// Immediately start a query, then do it every interval.
query()
return setInterval(query, interval)
log('query', serviceTag)
mdns.query({
questions: [{
name: serviceTag,
type: 'PTR'
}]
})
},

gotResponse: function (rsp, peerInfo, serviceTag, callback) {
Expand Down
139 changes: 82 additions & 57 deletions test/multicast-dns.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,29 @@ const expect = chai.expect
chai.use(dirtyChai)
const multiaddr = require('multiaddr')
const PeerInfo = require('peer-info')
const PeerId = require('peer-id')
const parallel = require('async/parallel')
const series = require('async/series')

const MulticastDNS = require('./../src')

function createPeer (callback) {
PeerId.create({ bits: 512 }, (err, id) => {
if (err) throw err
PeerInfo.create(id, callback)
})
}

describe('MulticastDNS', () => {
let pA
let pB
let pC
let pD

before(function (done) {
this.timeout(80 * 1000)
parallel([
(cb) => {
PeerInfo.create((err, peer) => {
createPeer((err, peer) => {
expect(err).to.not.exist()

pA = peer
Expand All @@ -31,7 +38,7 @@ describe('MulticastDNS', () => {
})
},
(cb) => {
PeerInfo.create((err, peer) => {
createPeer((err, peer) => {
expect(err).to.not.exist()

pB = peer
Expand All @@ -41,7 +48,7 @@ describe('MulticastDNS', () => {
})
},
(cb) => {
PeerInfo.create((err, peer) => {
createPeer((err, peer) => {
expect(err).to.not.exist()
pC = peer
pC.multiaddrs.add(multiaddr('/ip4/127.0.0.1/tcp/20003'))
Expand All @@ -50,7 +57,7 @@ describe('MulticastDNS', () => {
})
},
(cb) => {
PeerInfo.create((err, peer) => {
createPeer((err, peer) => {
if (err) { cb(err) }
pD = peer
pD.multiaddrs.add(multiaddr('/ip4/127.0.0.1/tcp/30003/ws'))
Expand All @@ -61,8 +68,6 @@ describe('MulticastDNS', () => {
})

it('find another peer', function (done) {
this.timeout(40 * 1000)

const options = {
port: 50001 // port must be the same
}
Expand All @@ -72,25 +77,22 @@ describe('MulticastDNS', () => {
})
const mdnsB = new MulticastDNS(pB, options)

mdnsA.once('peer', (peerInfo) => {
expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String())
parallel([
(cb) => mdnsA.stop(cb),
(cb) => mdnsB.stop(cb)
], done)
})

mdnsB.once('peer', (peerInfo) => {})
parallel([
(cb) => mdnsA.start(cb),
(cb) => mdnsB.start(cb)
], () => {
mdnsA.once('peer', (peerInfo) => {
expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String())
parallel([
(cb) => mdnsA.stop(cb),
(cb) => mdnsB.stop(cb)
], done)
})

mdnsB.once('peer', (peerInfo) => {})
})
], () => {})
})

it('only announce TCP multiaddrs', function (done) {
this.timeout(40 * 1000)

const options = {
port: 50003 // port must be the same
}
Expand All @@ -102,74 +104,97 @@ describe('MulticastDNS', () => {
const mdnsC = new MulticastDNS(pC, options)
const mdnsD = new MulticastDNS(pD, options)

mdnsA.once('peer', (peerInfo) => {
expect(pC.id.toB58String()).to.eql(peerInfo.id.toB58String())
expect(peerInfo.multiaddrs.size).to.equal(1)
parallel([
(cb) => mdnsA.stop(cb),
(cb) => mdnsC.stop(cb),
(cb) => mdnsD.stop(cb)
], done)
})
mdnsC.once('peer', (peerInfo) => {})
parallel([
(cb) => mdnsA.start(cb),
(cb) => mdnsC.start(cb),
(cb) => mdnsD.start(cb)

], () => {
mdnsA.once('peer', (peerInfo) => {
expect(pC.id.toB58String()).to.eql(peerInfo.id.toB58String())
expect(peerInfo.multiaddrs.size).to.equal(1)
parallel([
(cb) => mdnsA.stop(cb),
(cb) => mdnsC.stop(cb),
(cb) => mdnsD.stop(cb)
], done)
})

mdnsC.once('peer', (peerInfo) => {})
})
], () => {})
})

it('announces IP6 addresses', function (done) {
this.timeout(40 * 1000)

const options = {
port: 50001 // port must be the same
}
const mdnsA = new MulticastDNS(pA, {
broadcast: false, // do not talk to ourself
port: 50001
})
const mdnsA = new MulticastDNS(pA, options)
const mdnsB = new MulticastDNS(pB, options)

mdnsA.once('peer', (peerInfo) => {
expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String())
expect(peerInfo.multiaddrs.size).to.equal(2)
parallel([
(cb) => mdnsA.stop(cb),
(cb) => mdnsB.stop(cb)
], done)
})
mdnsB.once('peer', (peerInfo) => {})
series([
(cb) => mdnsB.start(cb),
(cb) => mdnsA.start(cb)
], () => {
mdnsA.once('peer', (peerInfo) => {
expect(pB.id.toB58String()).to.eql(peerInfo.id.toB58String())
expect(peerInfo.multiaddrs.size).to.equal(2)
parallel([
(cb) => mdnsA.stop(cb),
(cb) => mdnsB.stop(cb)
], done)
})

mdnsB.once('peer', (peerInfo) => {})
})
], () => {})
})

it('doesn\'t emit peers after stop', function (done) {
this.timeout(40 * 1000)

const options = {
port: 50004 // port must be the same
}
const mdnsA = new MulticastDNS(pA, options)
const mdnsC = new MulticastDNS(pC, options)
mdnsC.once('peer', (peerInfo) => {
done(new Error('Should not receive new peer.'))
})

series([
(cb) => mdnsA.start(cb),
(cb) => setTimeout(cb, 1000),
(cb) => mdnsA.stop(cb),
(cb) => mdnsC.start(cb)
], () => {
setTimeout(() => mdnsC.stop(done), 5000)
mdnsC.once('peer', (peerInfo) => {
done(new Error('Should not receive new peer.'))
})
setTimeout(() => mdnsC.stop(done), 1000)
})
})

it('find all peers', function (done) {
const options = {
port: 50001 // port must be the same
}
const mdnsA = new MulticastDNS(pA, options)
const mdnsB = new MulticastDNS(pB, options)
const mdnsC = new MulticastDNS(pC, options)
const peersA = {}
const peersB = {}
const peersC = {}

// After all the peers have started, each peer should see two other peers.
function check (receiver, peerInfo) {
receiver[peerInfo.id.toB58String()] = true
if (Object.keys(peersA).length === 2 && Object.keys(peersB).length === 2 && Object.keys(peersC).length === 2) {
parallel([
(cb) => mdnsA.stop(cb),
(cb) => mdnsB.stop(cb),
(cb) => mdnsC.stop(cb)
], done)
}
}
mdnsA.on('peer', (peerInfo) => check(peersA, peerInfo))
mdnsB.on('peer', (peerInfo) => check(peersB, peerInfo))
mdnsC.on('peer', (peerInfo) => check(peersC, peerInfo))
series([
(cb) => mdnsA.start(cb),
(cb) => setTimeout(cb, 500),
(cb) => mdnsB.start(cb),
(cb) => setTimeout(cb, 500),
(cb) => mdnsC.start(cb)
], () => {})
})
})