Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ class Hypercore extends EventEmitter {
const onseq = opts.onseq === undefined ? this.onseq : opts.onseq
const timeout = opts.timeout === undefined ? this.timeout : opts.timeout
const weak = opts.weak === undefined ? this.weak : opts.weak
const active = opts.active === undefined ? this._active : opts.active
const Clz = opts.class || Hypercore
const s = new Clz(null, this.key, {
...opts,
Expand All @@ -232,6 +233,7 @@ class Hypercore extends EventEmitter {
timeout,
writable,
weak,
active,
parent: this
})

Expand Down Expand Up @@ -415,6 +417,8 @@ class Hypercore extends EventEmitter {
await this.core.setManifest(createManifest(opts.manifest))
}

if (this.state !== this.core.state) this._active = false

this.core.replicator.updateActivity(this._active ? 1 : 0)

this.opened = true
Expand Down
4 changes: 3 additions & 1 deletion lib/replicator.js
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,7 @@ module.exports = class Replicator {
clearTimeout(this._downloadingTimer)

if (this.destroyed) return
if (downloading || this._notDownloadingLinger === 0) {
if (downloading || this._notDownloadingLinger === 0 || !this.peers.length) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sanity check: isn't the point of having this._notDownloadingLinger that we postpone switching off the downloading a bit? We seem to override that check when this.peers.length === 0

this.setDownloadingNow(downloading)
return
}
Expand Down Expand Up @@ -2461,6 +2461,8 @@ module.exports = class Replicator {
}

_makePeer (protomux) {
if (!this.downloading) return false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs some thought, because I think it could be dangerous. Doesn't this mean we will never open replication sessions to peers if we're not downloading the hypercore ourselves?

So the underlying assumption is: we always have this.downloading set to true when we open a connection and actually want to replicate.

For example, this assumes there's no path where we open a connection to a peer while this.downloading === false but set it to true later.

(not saying this isn't correct, it just rings some alarm bells of 'potentially dangerous'. The fact that all tests pass does seem like a good sign)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ya true, we should add a test that checks if an inactive peer can replicate with an active peer if we dont


const replicator = this
if (protomux.opened({ protocol: 'hypercore/alpha', id: this.core.discoveryKey })) return onnochannel()

Expand Down
106 changes: 106 additions & 0 deletions test/replicate.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,112 @@ test('basic downloading is set immediately after ready', async function (t) {
})
})

test('basic session on inactive core is inactive', async function (t) {
t.plan(5)

const createA = await createStored(t)
const a = await createA()

a.on('ready', function () {
t.ok(a.core.replicator.downloading)
})

const createB = await createStored(t)
const b = await createB({ active: false })

b.on('ready', function () {
t.absent(b.core.replicator.downloading)

const c = b.session()
t.teardown(() => c.close())

c.on('ready', function () {
t.absent(b.core.replicator.downloading)
t.absent(c.core.replicator.downloading)

const d = c.session({ active: true })
t.teardown(() => d.close())

d.on('ready', function () {
t.ok(d.core.replicator.downloading)
})
})
})

t.teardown(async () => {
await a.close()
await b.close()
})
})

test('basic named session is always inactive', async function (t) {
t.plan(4)

const createA = await createStored(t)
const a = await createA()

a.on('ready', function () {
t.ok(a.core.replicator.downloading)
})

const createB = await createStored(t)
const b = await createB({
notDownloadingLinger: 0 // replicator activity updates immediately
})

b.on('ready', function () {
t.ok(b.core.replicator.downloading)

const c = b.session({ name: 'named' })
t.teardown(() => c.close())

c.on('ready', async function () {
b.setActive(false)

t.absent(b.core.replicator.downloading)
t.absent(c.core.replicator.downloading)
})
})

t.teardown(async () => {
await a.close()
await b.close()
})
})

test('basic toggle active with existing connection', async function (t) {
t.plan(4)

const a = await create(t)
await a.ready()

const b = await create(t, a.key, { active: false })

await b.ready()

t.absent(b.core.replicator.downloading)

replicate(a, b, t)

await a.append('a1')

setTimeout(() => {
t.is(b.length, 0)

b.setActive(true)
t.ok(b.core.replicator.downloading)

b.on('append', () => {
t.is(b.length, 1)
})
}, 1000)

t.teardown(async () => {
await a.close()
await b.close()
})
})

test('basic replication from fork', async function (t) {
const a = await create(t)

Expand Down
Loading