Persist DHT nodes #1431

bookmoons · 2018-07-14T15:07:10Z

Persists DHT nodes to disk every 15 minutes.
Loads persisted nodes on construction.
Enabled by default.

Default save file is dht.json, under crossplatform app data folder provided by app-data-folder.

Adds option dhtState to configure:

false disables
true enables with default path
String specifies path to custom save file

Closes #72.


        Persist DHT nodes

Persists DHT nodes to disk every 15 minutes. Loads persisted nodes on construction. Enabled by default. Default save file is dht.json, under crossplatform app data folder provided by app-data-folder. Adds option dhtState to configure: * false disables * true enables with default path * String specifies path to custom save file

welcome · 2018-07-14T15:07:13Z

🙌 Thanks for opening this pull request! You're awesome.

bookmoons · 2018-07-14T15:13:27Z

Hi guys. Aiming to implement #72 here, persisting DHT nodes. All local tests succeed.

It uses app-data-folder to get a good crossplatform default location for variable app data.

Loading is unfortunately synchronous. Making the DHT initialization async breaks the client for some reason unclear to me. Usage expects to call methods immediately, but they start failing if DHT loading is not finished. Even with that ready event delayed until everything is ready. If there's an easy way to fix that I can update.

bookmoons · 2018-07-14T15:14:26Z

I'm proposing a new option dhtState here. Default is true, which enables persistence with the default path. A string enables with a custom path. False disables.

DiegoRBaquero · 2018-07-24T23:45:07Z

I like this, but we need some tests. Would you add them?


        Move DHT persistence logic into lib/ module

Increases cleanliness of index.js.


        Add save DHT state test


        Distinguish addresses in DHT persistence test

Uses loopback for the test DHT server, a local address provided by the network-address module for the client. This is the pattern used in other tests.


        Correct DHT state loading

Saved node data and the bootstrap list use different formats. Converts to the bootstrap format.


        Allow DHT state loading when bootstrap disabled

Prior logic disabled DHT state load with any bootstrap value, including a false to disable. Changes to allowing DHT state load if bootstrap is explicitly disabled with a falsy value.


        Add load DHT state test

bookmoons · 2018-07-30T17:46:52Z

Thanks for looking it over. I've added 2 tests, 1 for saving, 1 for loading. They're in persist-dht-nodes.js. They test with temp files. All tests are passing, including the new ones.

Started splitting things out into lib/dhtpersist.js and ended up not needing it. I left it in; think it makes index.js cleaner.

I added an API method saveDhtstate, to make it possible to get at it in the test. Not sure if that's a good idea. It starts here:
https://github.com/webtorrent/webtorrent/pull/1431/files#diff-168726dbe96b3ce427e7fedce31bb0bcR439

KayleePop · 2018-08-03T04:11:11Z

index.js

@@ -19,6 +20,7 @@ var randombytes = require('randombytes')
 var speedometer = require('speedometer')
 var zeroFill = require('zero-fill')

+var dhtPersist = require('./lib/dhtpersist') // browser exclude


It says browser exclude, but it's not actually excluded in the browser field of package.json?

like this

"browser": { "./lib/dhtpersist": false }

Misunderstood what that was about. I was thinking it was some control message to the bundler. I've updated.

KayleePop · 2018-08-03T04:15:57Z

docs/api.md

+  peerId: String|Buffer,    // Wire protocol peer ID (default=randomly generated)
+  tracker: Boolean|Object,  // Enable trackers (default=true), or options object for Tracker
+  dht: Boolean|Object,      // Enable DHT (default=true), or options object for DHT
+  dhtState: Boolean|String, // Persist DHT nodes (default=true), or save file path


I think this would be clearer as two separate variables. A boolean for whether to persist the state, and a file path for persisting the state

Maybe named (doesn't matter)

{ persistDht: Boolean, persistDhtPath: String }

I've updated to split them that way.


        Exclude dhtpersist on browser


        Split DHT persistence options

Splits from single option dhtState into 2 options: * persistDht - Boolean. Flag to enable. * persistDhtPath - String. Optional custom path for save file.

bookmoons · 2018-08-06T03:02:00Z

Thanks for the review. Didn't even show up in my activity list. Will have to find a better way to watch for that. I've addressed both of them.

bookmoons · 2018-08-06T21:16:19Z

I guess one of the automated builds failed. It looks like it failed on npm install. Is it possible that was a temporary network error?

DiegoRBaquero · 2018-08-24T17:42:59Z

I don't like 3 new dependencies, any way these could be integrated as libs?

Also, should we enable this by default? @feross ?

bookmoons · 2018-08-27T09:53:46Z

I don't like 3 new dependencies, any way these could be integrated as libs?

I'm dug into something else for a while here, but will look at this when I have a chance.

stale · 2018-11-25T11:19:43Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

bookmoons added 6 commits Jul 30, 2018

Move DHT persistence logic into lib/ module

Verified

This commit was signed with a verified signature.

bookmoons

GPG key ID: 29D2A92F3145E7A7 Learn about signing commits

a7ed3a2

Increases cleanliness of index.js.

Add save DHT state test

Verified

This commit was signed with a verified signature.

bookmoons

GPG key ID: 29D2A92F3145E7A7 Learn about signing commits

c2f5943

Correct DHT state loading

Verified

This commit was signed with a verified signature.

bookmoons

GPG key ID: 29D2A92F3145E7A7 Learn about signing commits

ff755bc

Saved node data and the bootstrap list use different formats. Converts to the bootstrap format.

Add load DHT state test

Verified

This commit was signed with a verified signature.

bookmoons

GPG key ID: 29D2A92F3145E7A7 Learn about signing commits

Loading status checks…

2a3e7ee

KayleePop reviewed Aug 3, 2018

View changes

bookmoons added 2 commits Aug 6, 2018

Exclude dhtpersist on browser

Verified

This commit was signed with a verified signature.

bookmoons

GPG key ID: 29D2A92F3145E7A7 Learn about signing commits

02879a0

stale bot added the stale label Nov 25, 2018

stale bot closed this Dec 2, 2018

lock bot locked as resolved and limited conversation to collaborators Mar 2, 2019

webtorrent unlocked this conversation Sep 6, 2019

feross added the enhancement label Sep 6, 2019

feross reopened this Sep 6, 2019

stale bot removed the stale label Sep 6, 2019

webtorrent / webtorrent

Persist DHT nodes #1431

Persist DHT nodes #1431

bookmoons commented Jul 14, 2018 •

edited

This comment has been minimized.

welcome bot commented Jul 14, 2018

This comment has been minimized.

bookmoons commented Jul 14, 2018

This comment has been minimized.

bookmoons commented Jul 14, 2018 •

edited

This comment has been minimized.

DiegoRBaquero commented Jul 24, 2018

This comment has been minimized.

bookmoons commented Jul 30, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bookmoons commented Aug 6, 2018

This comment has been minimized.

bookmoons commented Aug 6, 2018

This comment has been minimized.

DiegoRBaquero commented Aug 24, 2018

This comment has been minimized.

bookmoons commented Aug 27, 2018

This comment has been minimized.

stale bot commented Nov 25, 2018

webtorrent / webtorrent

Sponsor webtorrent/webtorrent

Join GitHub today

Persist DHT nodes #1431

Persist DHT nodes #1431

Conversation

bookmoons commented Jul 14, 2018 • edited

This comment has been minimized.

welcome bot commented Jul 14, 2018

This comment has been minimized.

bookmoons commented Jul 14, 2018

This comment has been minimized.

bookmoons commented Jul 14, 2018 • edited

This comment has been minimized.

DiegoRBaquero commented Jul 24, 2018

This comment has been minimized.

bookmoons commented Jul 30, 2018

This comment has been minimized.

KayleePop Aug 3, 2018

This comment has been minimized.

bookmoons Aug 6, 2018

This comment has been minimized.

KayleePop Aug 3, 2018

This comment has been minimized.

bookmoons Aug 6, 2018

This comment has been minimized.

bookmoons commented Aug 6, 2018

This comment has been minimized.

bookmoons commented Aug 6, 2018

This comment has been minimized.

DiegoRBaquero commented Aug 24, 2018

This comment has been minimized.

bookmoons commented Aug 27, 2018

This comment has been minimized.

stale bot commented Nov 25, 2018

bookmoons commented Jul 14, 2018 •

edited

bookmoons commented Jul 14, 2018 •

edited