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.

Sign up for GitHub

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

drive.download() does not work as expected #350

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

drive.download() does not work as expected #350

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 22 additions & 0 deletions test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,28 @@ test('drive.download(folder, [options])', async (t) => {
t.is(count, _count + 1)
})

test.solo('drive.download()', async (t) => {
const store1 = new Corestore(RAM)
const store2 = new Corestore(RAM)
const drive1 = new Hyperdrive(store1)
await drive1.ready()
const drive2 = new Hyperdrive(store2, drive1.key)
await drive2.ready()

const nil = b4a.from('nil')

await drive1.put('/foo', nil)

const s = store1.replicate(true)
s.pipe(store2.replicate(false)).pipe(s)

// Must do this in order for this test to pass
// await drive2.db.core.update({ wait: true })
await drive2.download()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's expected behaviour because you didn't receive the first version/length update yet

If I understand correctly, you can "update" only once for waiting to "bootstrap" into the network, so you receive the latest length from peers, but then subsequent updates won't work so you should not rely on force updating to receive a new length

I might be wrong, I was also confused with this for a long time haha

In other news, we're working to improve this so replicating becomes way easier!

Copy link
Contributor

Choose a reason for hiding this comment

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

Funny enough, check this really old version of drives:
https://github.com/holepunchto/drives/blob/43a2f23fc39a0cce713b4d7a62779490d1ccc11f/download.js#L40
I experienced something similar

Copy link
Contributor

Choose a reason for hiding this comment

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

Top summary: I would say it's ok to wait for the first update

We just need to fix Hyperbee so you can just do drive.update({ wait: true }) instead of accessing the db core, etc but we have a PR for that already

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @LuKks - the behaviour seemed to change with Hypercore 10.6.0 which made updates local only - at least I had a test that did something similar to this (without calling update({ wait: true }) first) that worked with prior to 10.6.0, but maybe I was using it wrong!

I do find the update() stuff to be confusing - I'm never sure when I need to call it to make sure things work as expected, and since we don't exclusively use hyperswarm for discovery (we also use mdns) I'm never quite sure how the findingPeers stuff is supposed to work.

Copy link
Author

Choose a reason for hiding this comment

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

FYI I have been using this for implementing a "live" download functionality: https://github.com/digidem/mapeo-core-next/blob/main/lib/blob-store/live-download.js#L87

Is this something you would consider adding to hyperdrive?

Copy link
Contributor

Choose a reason for hiding this comment

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

Once Hyperbee update method is fixed then just do like so:

const swarm = new Hyperswarm()
const done = drive.findingPeers()
swarm.on('connection', (socket) => drive.replicate(socket))
swarm.join(drive.discoveryKey)
swarm.flush().then(done, done)
await drive.update({ wait: true })

Copy link
Contributor

Choose a reason for hiding this comment

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

Keep in mind something like this:

Let's say you already have the code above working and everything is ok, let's say you later re-execute again for a second time this:
await drive.update({ wait: true })

You expect the drive to PULL from other peers their latest length, right? That's the main issue, peers must PUSH to you new data in this case the latest version/length

So you only get background updates for new versions! I think you can always use the Hypercore 'append' event to know when you receive an update

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, sorry, I didn't answer your doubt about how findingPeers works

First check this: https://github.com/holepunchto/hyperdrive#const-done--drivefindingpeers
Very important is that you don't depend on Hyperswarm

When you do this:
const done = core.findingPeers()
That does literally almost nothing, it's just an internal counter that gets increased

So all requests made after that are put on hold, until you call done(), so call it when your mdns thing has connected to all peers that it found, that's it

Copy link
Author

Choose a reason for hiding this comment

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

Ah right, thanks for that @LuKks, helpful. I think in our case the main challenge is deciding when "done" happens - new devices can appear at any time, so it's never actually "done".

Copy link
Contributor

Choose a reason for hiding this comment

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

When you start the mdns search, if there is a peer available, it's normally extremely fast to appear, right?

So you could just give 3-5 seconds of wait until a peer appears, otherwise continue without

When you enter a swarm topic i.e. drive discovery key and there is no peers, the update takes a few seconds as well, you should try it out


t.alike(await drive2.get('/foo'), nil)
})

test.skip('drive.downloadRange(dbRanges, blobRanges)', async (t) => {
const { drive, swarm, mirror, corestore } = await testenv(t.teardown)
swarm.on('connection', (conn) => corestore.replicate(conn))
Expand Down