add lastChange property

[robertkowalski]

Hi! I'm improving bittorrent-dht's handling of node-lists and how it handles unresponsive nodes.

This adds a lastChange property.

Each bucket should maintain a "last changed" property to indicate how "fresh" the 
contents  are. When a node in a bucket is pinged and it responds, or a node is added 
to a bucket, or a node in a bucket is replaced with another node, the bucket's last 
changed property should be updated. Buckets that have not been changed in 
15 minutes should be "refreshed."

from: http://www.bittorrent.org/beps/bep_0005.html#routing-table

The implementation / updates for the lastChange property should be done in "userland" as ping implementation is also handled there.

[robertkowalski]

related: webtorrent/bittorrent-dht#20

[fanatid]

Is it all changes? Where property lastChange is changed?

[robertkowalski]

yes - we change it in https://github.com/webtorrent/bittorrent-dht/pull/172/files

[robertkowalski]

as i mentioned:

The implementation / updates for the lastChange property should be done in "userland" as the ping implementation is also handled there.

[fanatid]

if you do this in "userland", why not create lastChange property in "userland"?

[robertkowalski]

thank you for your question.

to initialise it and set it explicitly to null as a property for k-bucket

[fanatid]

I checked https://github.com/webtorrent/bittorrent-dht/pull/172/files
That PR should work without this changes, right?

[robertkowalski]

yes, thats right. it just makes use of the property which is set to null with this PR.

…

On Mon, Aug 14, 2017 at 9:41 PM, Kirill Fomichev ***@***.***> wrote: I checked https://github.com/webtorrent/bittorrent-dht/pull/172/files That PR should work without this changes, right? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#43 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASMttIJ7J2p25OqKP9zHejNfzVRyZqfks5sYKMFgaJpZM4Ou03s> .

[tristanls]

Maybe this can be addressed with clarification about what the API is? The contact is well defined in that only the contact.id property is used and any satellite properties are untouched and preserved. If we had a concept/definition of satellite data for the k-bucket itself (which we don't), that could be used to define how safe it is to add properties to k-bucket and how that should be done.

So, if we defined the properties on k-bucket something like arbiter, distance, localNodeId, numberOfNodesPerKBucket, numberOfNodesToPing, and root. As well as all properties starting with underscore _ are reserved. Then, it should be safe for anyone to attach other properties. Alternatively, we could define metadata (or some name like that) that could serve as a container for satellite data, so that lastChange would end up on kBucket.metadata.lastChange or similar.

It seems to me easier to define metadata property as an object to carry satellite data. That way we don't have to enumerate properties, methods, and privates as part of the definition, but instead could say, "put your stuff in metadata property on k-bucket and it will be safe there."

[robertkowalski]

Thank you for the explanation. That makes sense and is really well thought.

I agree. Are you happy to merge when I switch over to metadata and document it?

[tristanls]

Yes. (not sure if you get notified if I only use emoji on your post 🙂 )

[robertkowalski]

@tristanls i didn't get a notice from the emoji reaction. thanks for writing a separate message.

I changed the PR accordingly.