New values for DHT Provider Record Republish and Expiration (22h/48h, RFM17) #451

yiannisbot · 2022-09-14T08:50:42Z

This PR updates the description of the Provider Record settings and most importantly proposes new values for both the republish interval and the expiration interval. The new proposed values are:

republish interval to be set to 22hrs, from its current 12hrs
expiration interval to be set to 48hrs, from its current

and they are based on the comprehensive study published here: https://github.com/protocol/network-measurements/blob/master/results/rfm17-provider-record-liveness.md

kad-dht/README.md

lidel

Bumping Republish and Expiration sounds sensible ("reduce the overhead without interfering with the performance and reliability").

@mxinden any concerns from non-IPFS side of things?

kad-dht/README.md

Co-authored-by: Marcin Rataj <lidel@lidel.org>

mxinden

Wonderful to see these optimizations based on comprehensive studies!

Can you bump the revision of the specification at the top of the document?

kad-dht/README.md

mxinden · 2022-09-16T14:49:55Z

kad-dht/README.md

+to prevent storing potentially outdated address information. Implementations that choose
+to keep the network address (i.e., the `multiaddress`) of the providing peer should do it for **the
+first 10 mins** after the provider record (re-)publication. The setting of 10 mins follows
+the DHT Routing Table refresh interval. After that, peers provide 
+the provider's `peerID` only, in order to avoid pointing to stale network addresses 
+(i.e., the case where the peer has moved to a new network address).


If I recall correctly, this is the status quo in the Golang implementation, correct? Is there any data backing up this decision? What I am surprised by is that addresses go stale so quickly. Is that really the case on IPFS today?

this is the status quo in the Golang implementation, correct?

Yup.

Is there any data backing up this decision?

Nope :) But plan to start some investigation asap. See: ipfs/kubo#9264, protocol/prodeng#22 and probe-lab/thunderdome#91 as an optimisation.

What I am surprised by is that addresses go stale so quickly. Is that really the case on IPFS today?

Given that IPFS DHT servers have public addresses, I doubt that they go stale so quickly. This might change when hole punching is widespread, but the optimisation proposed in the above issues won't hurt, I believe :)

Given that we are not sure this optimization is a good idea, how about only documenting it in this specification once we know it is a good idea?

Otherwise new implementations like Iroh (//CC @dignifiedquire) would have to implement this despite not being an optimization in the first place.

In fact, we've figured out that this has changed to 30mins: libp2p/go-libp2p@c282179 - I've rephrased the text accordingly to make it more generic and mention this value for the kubo implementation.

There's also this: ipfs/kubo#9264 - any views more than welcome! :)

kad-dht/README.md

mxinden · 2022-09-16T14:55:16Z

kad-dht/README.md

+and nodes that store and serve provider records need to make sure that the Multihashes whose 
+records they store are still served by the content provider.


How do they do this? Is this happening today on the IPFS DHT?

This is done (somewhat indirectly) through the expiration of the provider record: if the content provider does not republish the record (within the republish interval), then nodes do not serve those provider records after the expiration interval (assuming that the content provider is not interested having this content live anymore).

I think the above phrasing implies this being an active process on the provider record storage node. What do you think of rephrasing this?

I've rephrased to the following - let me know if it reads better:

"Content needs to be reachable, despite peer churn;
and nodes that store and serve provider records should not serve records for stale content,
i.e., content that the original provider does not wish to make available anymore."

aschmahmann · 2022-09-16T15:03:41Z

kad-dht/README.md

+remain online when clients ask for the record. In order to 
+guarantee this, while taking into account the peer churn, content providers
+republish the records they want to provide every 24 hours.
+2. **Provider Record Expiration Interval (48hrs):** The network needs to provide


This should be harmonized with the PUT_VALUE expiration time as well, no?

@yiannisbot while we're changing the times any reason not to do this too? Seems like it'd be reasonable since expiration is based on the same properties. It'd also make things easier to reason about.

You mean in the PR? Yes, the intention is to change both the republish and the expiration interval. Otherwise it would make no sense (or well, it would be confusing). Or do you mean something else?

cc: @cortze who is working to submit the relevant PR.

I agree with changing them together. However, unless I am missing something in the code, they will have to be two separate PRs:

Update kubo's default Reprovider.Interval here

Update go-libp2p-kad-dht's ProvideValidity here

Linking here the PR libp2p/go-libp2p-kad-dht#793 to increase the expiration time of the PRs to 48h

You mean in the PR

No, I mean to change the expiration time for PUT_VALUE records in addition to ADD_PROVIDER records.

i.e. for the IPFS Public DHT the expiration time for PUT_VALUE (i.e. IPNS and the deprecated public key records) is 36hrs
https://github.com/libp2p/go-libp2p-kad-dht/blob/dae5a9a5bd9c7cc8cfb5073c711bc308efad0ada/internal/config/config.go#L117. It seems like this could be 48hrs as well rather than 36.

@cortze should we change that too then? It makes sense. Do you want to create the PR and set it to 48hrs?

Just created it! here is the link to the PR -> go-libp2p-kad-dht#794

kad-dht/README.md

aschmahmann · 2022-09-16T15:14:23Z

kad-dht/README.md

+remain online when clients ask for the record. In order to 
+guarantee this, while taking into account the peer churn, content providers
+republish the records they want to provide every 24 hours.


Perhaps this has already been considered, but given this is an implicit protocol change it might help implementers to know what they should use as the republish interval. 24hrs matches the current expiration time so reproviding every 24hrs while the network is (slowly) upgrading might not be great. Perhaps it's fine in networks like the IPFS Public DHT since some nodes will upgrade quickly (e.g. Hydras, people autodeploying the latest kubo Docker containers, etc.) but just wanted to flag this.

Right, so you suggest not having the new republish interval the same as the old expiration interval as this will become confusing and might have side effects as well? I guess a valid workaround is to have the republish interval set to something a bit smaller (say, 20hrs?) for the transition period? Any better approaches?

I've set this to 22hrs. @mxinden @aschmahmann let me know if that works for you.

kad-dht/README.md

yiannisbot · 2022-09-29T06:36:31Z

@lidel @mxinden @aschmahmann I've addressed all comments, apart from the one suggesting to have the PR on IPFS before merge this. Can you have another look to see if everything is ready? In the meantime, we'll work to get the PR ready on the IPFS side of things - feel free to do so as well if you wish :)

mxinden

Thanks for the follow-ups @yiannisbot.

Other than the specification revision bump and the kubo pull request, this looks good to me.

mxinden · 2022-10-03T18:13:51Z

kad-dht/README.md

+It is also worth noting that the keys for provider records are multihashes. This
+is because:
+
+- Provider records are used as a rendezvous point for all the parties who have
+advertised that they store some piece of content.
+- The same multihash can be in different CIDs (e.g. CIDv0 vs CIDv1 of a SHA-256 dag-pb object,
+or the same multihash but with different codecs such as dag-pb vs raw).
+- Therefore, the rendezvous point should converge on the minimal thing everyone agrees on,
+which is the multihash, not the CID.


yiannisbot · 2022-10-05T12:38:45Z

Here is the PR to change the republish interval in kubo: ipfs/kubo#9326
The expiration interval change is coming soon.

cortze · 2022-10-05T16:04:21Z

Here is the second PR libp2p/go-libp2p-kad-dht#793 to increase the expiration time of the PRs from 24h to 48h.

License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>

lidel

@mxinden @marten-seemann bumped revision in the header, mind giving this final review?

Kubo team wants to ship and with next Kubo 0.18 release, but want to make sure we have specs merged first

reprovide interval in kubo is merged: feat: increase default Reprovider.Interval ipfs/kubo#9326
expiration interval in feat: increase expiration time for Provider Records to 48h (RFM17) go-libp2p-kad-dht#793 and feat: increase the max record age to 48h (PUT_VALUE, RFM17) go-libp2p-kad-dht#794 is waiting for review/release
- @marten-seemann lmk if it is ok for me to do it
this spec is in sync with the two above

fwiw I've updated revision as requested, and made it easier to find/eyeball both numbers:

Stebalien · 2022-12-09T21:18:33Z

Heil Hydra, I guess. If the records are staying around that long, we should do this.

lidel · 2022-12-11T21:56:15Z

There is also a general DX/UX improvement that comes with raising the ceiling of expiration.

Trying to keep IPNS website alive with spotty internet access is tricky.
It is not hard to imagine a situation when the publisher of an IPNS record can't be online exactly every 24h (living on a boat, or having to visit a library/school to access internet).
Expiration set to 48h makes a more “humane” default (“publish once a day, exact hour does not matter”).

In effort to include this in Kubo 0.18.0-rc1 before holidays, I've released go-libp2p-kad-dht v0.20.0 with 48h expiration.
Kubo 0.18 will also have the default Republish Interval set to 22h (ipfs/kubo#9389), being fully compliant with the spec from this PR.

@mxinden

should I open PR to adjust defaults in rust-libp2p/protocols/kad/src/behaviour.rs, or are there other places?
ok to merge this PR?

p-shahi · 2022-12-11T23:28:49Z

Should we go ahead and update js-libp2p-kad-dht (and js-ipfs' reprovide interval)?
cc: @achingbrain

mxinden

Thanks to everyone involved here.

mxinden · 2022-12-12T17:09:04Z

@mxinden

1. should I open PR to adjust defaults in [rust-libp2p/protocols/kad/src/behaviour.rs](https://github.com/libp2p/rust-libp2p/blob/4fe518de86ecb0805421cca6e291db168c10eac1/protocols/kad/src/behaviour.rs#L182-L196), or are there other places?

2. ok to merge this PR?

Tracked here libp2p/rust-libp2p#3229. Contributions are always welcome. That said, not a requirement to move forward here. Thanks for stewarding this @lidel.

Applies changes from libp2p/specs#451 New defaults are: Record Expiration: 48h Record Republish Interval: 22h Closes #3229

This patch applies changes from libp2p/specs#451. In particular, the new defaults are: - Record Expiration: 48h - Record Republish Interval: 22h Closes #3229. Pull-Request: #3230.

Provider Record Settings Description

b5fd7cc

yiannisbot commented Sep 14, 2022

View reviewed changes

kad-dht/README.md Show resolved Hide resolved

yiannisbot changed the title ~~Provider Record Settings Description~~ New value proposal for Provider Record settings Sep 14, 2022

lidel requested review from mxinden and raulk September 14, 2022 14:28

lidel reviewed Sep 14, 2022

View reviewed changes

kad-dht/README.md Outdated Show resolved Hide resolved

kad-dht/README.md Outdated Show resolved Hide resolved

kad-dht/README.md Outdated Show resolved Hide resolved

yiannisbot and others added 3 commits September 15, 2022 05:43

Update kad-dht/README.md

9f7f275

Co-authored-by: Marcin Rataj <lidel@lidel.org>

Update kad-dht/README.md

7f64613

Co-authored-by: Marcin Rataj <lidel@lidel.org>

Update kad-dht/README.md

094094d

Co-authored-by: Marcin Rataj <lidel@lidel.org>

mxinden reviewed Sep 16, 2022

View reviewed changes

aschmahmann reviewed Sep 16, 2022

View reviewed changes

yiannisbot added 2 commits September 29, 2022 07:25

addressing editorial comments

2318d76

provider advert and discovery clarification

9464d50

mxinden reviewed Oct 3, 2022

View reviewed changes

cortze mentioned this pull request Oct 5, 2022

feat: increase default Reprovider.Interval ipfs/kubo#9326

Merged

cortze mentioned this pull request Oct 5, 2022

feat: increase expiration time for Provider Records to 48h (RFM17) libp2p/go-libp2p-kad-dht#793

Merged

cortze mentioned this pull request Oct 12, 2022

feat: increase the max record age to 48h (PUT_VALUE, RFM17) libp2p/go-libp2p-kad-dht#794

Merged

yiannisbot mentioned this pull request Nov 8, 2022

New defaults for the Provider Record Republish and Expiration (22h/48h) ipfs/kubo#9389

Closed

6 tasks

yiannisbot mentioned this pull request Nov 25, 2022

Milestone: Improve Provider Record Intervals probe-lab/roadmap#13

Closed

revision bump and cleanup

3ba82bb

License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>

lidel approved these changes Dec 8, 2022

View reviewed changes

lidel changed the title ~~New value proposal for Provider Record settings~~ New values for DHT Provider Record Republish and Expiration (22h/48h, RFM17) Dec 11, 2022

This was referenced Dec 11, 2022

release v0.20.0 libp2p/go-libp2p-kad-dht#803

Merged

feat: go-libp2p-kad-dht with expiration 48h ipfs/kubo#9491

Closed

p-shahi mentioned this pull request Dec 11, 2022

Update default constants for DHT provider validity and maximum record age libp2p/js-libp2p-kad-dht#408

Open

mxinden mentioned this pull request Dec 12, 2022

kad: Update republish and expiration interval to new spec libp2p/rust-libp2p#3229

Closed

mxinden approved these changes Dec 12, 2022

View reviewed changes

lidel added a commit to libp2p/rust-libp2p that referenced this pull request Dec 12, 2022

feat(dht): updated republish and expiration

1ef309e

Applies changes from libp2p/specs#451 New defaults are: Record Expiration: 48h Record Republish Interval: 22h Closes #3229

lidel mentioned this pull request Dec 12, 2022

feat(kad): update republish interval and expiration time defaults libp2p/rust-libp2p#3230

Merged

4 tasks

mxinden merged commit 9a646c0 into libp2p:master Dec 12, 2022

lidel mentioned this pull request Dec 12, 2022

feat: add namesys publish options ipfs/interface-go-ipfs-core#94

Merged

CHr15F0x mentioned this pull request Aug 26, 2024

P2P: update rust-libp2p dep to v0.54.1 eqlabs/pathfinder#2190

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New values for DHT Provider Record Republish and Expiration (22h/48h, RFM17) #451

New values for DHT Provider Record Republish and Expiration (22h/48h, RFM17) #451

yiannisbot commented Sep 14, 2022 •

edited by lidel

Loading

lidel left a comment

mxinden left a comment

mxinden Sep 16, 2022

yiannisbot Sep 18, 2022 •

edited

Loading

mxinden Sep 22, 2022

yiannisbot Sep 29, 2022

mxinden Sep 16, 2022

yiannisbot Sep 18, 2022

mxinden Sep 22, 2022

yiannisbot Sep 29, 2022

aschmahmann Sep 16, 2022

aschmahmann Oct 3, 2022

yiannisbot Oct 5, 2022

cortze Oct 5, 2022

cortze Oct 5, 2022

aschmahmann Oct 6, 2022

yiannisbot Oct 12, 2022

cortze Oct 12, 2022

aschmahmann Sep 16, 2022

yiannisbot Sep 18, 2022

yiannisbot Sep 29, 2022

yiannisbot commented Sep 29, 2022

mxinden left a comment

mxinden Oct 3, 2022

yiannisbot commented Oct 5, 2022

cortze commented Oct 5, 2022

lidel left a comment •

edited by p-shahi

Loading

Stebalien commented Dec 9, 2022

lidel commented Dec 11, 2022 •

edited

Loading

p-shahi commented Dec 11, 2022

mxinden left a comment

mxinden commented Dec 12, 2022

		and nodes that store and serve provider records need to make sure that the Multihashes whose
		records they store are still served by the content provider.

New values for DHT Provider Record Republish and Expiration (22h/48h, RFM17) #451

New values for DHT Provider Record Republish and Expiration (22h/48h, RFM17) #451

Conversation

yiannisbot commented Sep 14, 2022 • edited by lidel Loading

lidel left a comment

Choose a reason for hiding this comment

mxinden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yiannisbot Sep 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yiannisbot commented Sep 29, 2022

mxinden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yiannisbot commented Oct 5, 2022

cortze commented Oct 5, 2022

lidel left a comment • edited by p-shahi Loading

Choose a reason for hiding this comment

Stebalien commented Dec 9, 2022

lidel commented Dec 11, 2022 • edited Loading

p-shahi commented Dec 11, 2022

mxinden left a comment

Choose a reason for hiding this comment

mxinden commented Dec 12, 2022

yiannisbot commented Sep 14, 2022 •

edited by lidel

Loading

yiannisbot Sep 18, 2022 •

edited

Loading

lidel left a comment •

edited by p-shahi

Loading

lidel commented Dec 11, 2022 •

edited

Loading