routing fixes/refactor #5007

Stebalien · 2018-05-09T13:35:14Z

Cleans up namesys a bit (don't use a map for fields, avoid casting).
Update for routing system refactor (replace GetValues with a more flexible
GetValue).
Don't expect the DHT to keep outdated IPNS records.

Note: I'm not entirely happy with this patch but it's good enough for now.
Eventually, I'd prefer to pass the IpnsPublisher to the Republisher as I'm not a
fan of having the Republisher dig through the IpnsPublisher's datastore entries.

However, this unblocks us.

Depends on:

update for the routing refactor libp2p/go-libp2p-kad-dht#148
make routing less DHT specific libp2p/go-libp2p-routing#21
refactor record validation libp2p/go-libp2p-record#17
add fs repo migration v1.4.0 distributions#190
Add migration for IPNS record move fs-repo-migrations#72
update for routing interface changes go-ipfs-routing#5
go-libp2p-pusbub-router -- extraction of the pubsub router.
go-libp2p-routing-helpers -- helpers for composing multiple routers. Somewhat complicated but it has >97% test coverage.

Stebalien · 2018-05-09T13:56:43Z

namesys/publisher.go

+//
+// This method will not search the routing system for records published by other
+// nodes.
+func (p *IpnsPublisher) ListPublished(ctx context.Context) (map[peer.ID]*pb.IpnsEntry, error) {


Should probably have a test. This isn't actually used right now but I'd like to expose it via a command (and use it from the republisher).

Stebalien · 2018-05-09T13:57:06Z

namesys/publisher.go

+//
+// If `checkRouting` is true and we have no existing record, this method will
+// check the routing system for any existing records.
+func (p *IpnsPublisher) GetPublished(ctx context.Context, id peer.ID, checkRouting bool) (*pb.IpnsEntry, error) {


checkRouting... (bleh)

Stebalien · 2018-05-09T14:28:18Z

test/sharness/t0090-get.sh

@@ -20,7 +20,7 @@ test_ipfs_get_flag() {
  '

  test_expect_success "ipfs get $flag output looks good" '
-    printf "%s\n\n" "Saving archive to $HASH$ext" >expected &&


This seems strictly better but is technically a breaking change. Any idea why this has changed?

dirkmc · 2018-05-16T17:29:08Z

namesys/publisher.go

-}
+	seqno := rec.GetSequence() // returns 0 if rec is nil
+	if rec != nil && value != path.Path(rec.GetValue()) {
+		// Don't bother incriminating the sequence number unless the


incriminating? :)

aspell has a surprisingly small english dictionary...

I am the author of said dictionary. I assumed you meant "incrementing", that is not a word found in most dictionaries, for example https://www.merriam-webster.com/dictionary/incrementing return no results. Nevertheless it is a word I will consider adding. Let's continue this discussion in an issue at https://github.com/en-wl/wordlist/issues.

@kevina you are! Awesome! Will do.

whyrusleeping · 2018-05-31T06:35:42Z

core/core.go

+			n.Floodsub,
+			validator,
+		)
+		n.Routing = rhelpers.Tiered{


kevina · 2018-05-31T08:51:02Z

What is the status of this? Is it likely to go in mostly as is?

Just FYI: I have not looked at this closely but this looks like it might be making API changes to the routing code. Some additional changes may be required to support sym-links.

Stebalien · 2018-06-01T00:44:41Z

What is the status of this? Is it likely to go in mostly as is?

That's my plan. I'm almost done with the migration but that's the only thing left.

Just FYI: I have not looked at this closely but this looks like it might be making API changes to the routing code. Some additional changes may be required to support sym-links.

Symlinks? This should only be touching namesys/routing code (not unixfs path resolution or anything that should handle symlinks). Am I missing something?

kevina · 2018-06-01T00:54:54Z

Symlinks? This should only be touching namesys/routing code (not unixfs path resolution or anything that should handle symlinks). Am I missing something?

The current routing API makes adding support for symlinks difficult, espacally the handling of ..

See #3508.

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien · 2018-06-01T01:28:06Z

Does this patch make something worse?

And: * Update for DHT changes. * Switch to the new record validation system. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

The current convention is to return the concrete type instead of an interface so let's go with that and have one constructor. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

fixes #4749 License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Remove ~50 lines of code, some casting, and a superfluous map (when go starts looking like python, something's wrong). License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

kevina · 2018-06-01T01:33:04Z

I have not looked closely but I don't think so. Since this should hopefully go in soon I will wait until it does before I tackle the API changes needed for symbolic links.

whyrusleeping · 2018-06-01T08:19:56Z

there shouldnt be any shared code between routing and symlinks... You might be thinking of the path resolver.

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

We've added a new file to the flatfs datastore. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

whyrusleeping

This is a pretty complicated change, but I think i've reviewed it thoroughly and don't see any issues. Let's get this shipped and let's forge ahead and make IPNS great again!

whyrusleeping · 2018-06-02T07:01:56Z

namesys/publisher.go

+		_, ipnskey := IpnsKeysForID(id)
+		value, err = p.routing.GetValue(ctx, ipnskey)
+		if err != nil {
+			// Not found or other network issue. Can't really do


dropping errors makes me nervous, would appreciate maybe a debug or info log of this

Fair enough.

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien · 2018-06-02T07:40:31Z

~~DO NOT MERGE UNTIL: ipfs/infra#408~~

~~(well, actually, it's not that big of an issue as go-ipfs refers to that dist path directly by hash; but still).~~

Done.

routing fixes/refactor This commit was moved from ipfs/kubo@c3e011b

Stebalien requested a review from Kubuxu as a code owner May 9, 2018 13:35

ghost assigned Stebalien May 9, 2018

ghost added the status/in-progress In progress label May 9, 2018

Stebalien mentioned this pull request May 9, 2018

update for routing interface changes ipfs/go-ipfs-routing#5

Merged

Stebalien commented May 9, 2018

View reviewed changes

Stebalien force-pushed the feat/routing-refactor branch 3 times, most recently from 96ee5d7 to 2192077 Compare May 9, 2018 14:12

Stebalien commented May 9, 2018

View reviewed changes

Stebalien force-pushed the feat/routing-refactor branch from f0aa157 to fb318b6 Compare May 10, 2018 16:47

dirkmc reviewed May 16, 2018

View reviewed changes

Stebalien force-pushed the feat/routing-refactor branch 2 times, most recently from 262f464 to adedd02 Compare May 17, 2018 20:07

whyrusleeping reviewed May 31, 2018

View reviewed changes

core/core.go

n.Floodsub,

validator,

)

n.Routing = rhelpers.Tiered{

Copy link

Member

whyrusleeping May 31, 2018

Choose a reason for hiding this comment

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

👍

Stebalien force-pushed the feat/routing-refactor branch from adedd02 to 9f5cd94 Compare June 1, 2018 00:46

update deps

1e9e2f4

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien force-pushed the feat/routing-refactor branch from 9f5cd94 to 31e4fa3 Compare June 1, 2018 01:21

Stebalien added 8 commits May 31, 2018 18:29

extract IPNS over pubsub as a ValueStore

5dc0b73

And: * Update for DHT changes. * Switch to the new record validation system. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

simplify routing resolution a bit

56232ba

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

consolidate dns resolver constructors

55cb76d

The current convention is to return the concrete type instead of an interface so let's go with that and have one constructor. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

store IPNS records *outside* of the DHT

60708ea

fixes #4749 License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

cleanup namesys a bit

1089eda

Remove ~50 lines of code, some casting, and a superfluous map (when go starts looking like python, something's wrong). License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

fix key_test for peer ID inlining patch

a030455

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

remove extra newline from expected command output

b4ed464

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

fix keystore tests for peer ID inlining patch

916572f

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien force-pushed the feat/routing-refactor branch from 31e4fa3 to 916572f Compare June 1, 2018 01:29

This was referenced Jun 1, 2018

Add migration for IPNS record move ipfs/fs-repo-migrations#72

Merged

add fs repo migration v1.4.0 ipfs/distributions#190

Merged

bump repo version and update migration dist path

c2c49c8

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien force-pushed the feat/routing-refactor branch from 3c5a547 to c2c49c8 Compare June 1, 2018 18:22

Stebalien added need/review Needs a review and removed status/in-progress In progress labels Jun 1, 2018

ghost added the status/in-progress In progress label Jun 1, 2018

Stebalien removed the status/in-progress In progress label Jun 1, 2018

Stebalien mentioned this pull request Jun 1, 2018

Bump Repo Version and Provide Migration before strict Cid code lands #4766

Open

fix the repo verify test

8293e20

We've added a new file to the flatfs datastore. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien force-pushed the feat/routing-refactor branch from e0f05a2 to 8293e20 Compare June 2, 2018 06:39

ghost added the status/in-progress In progress label Jun 2, 2018

whyrusleeping approved these changes Jun 2, 2018

View reviewed changes

log on network error when resolving the last published IPNS record

807f28e

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien added RFM and removed status/in-progress In progress need/review Needs a review labels Jun 2, 2018

whyrusleeping merged commit c3e011b into master Jun 3, 2018

whyrusleeping deleted the feat/routing-refactor branch June 3, 2018 05:30

This was referenced Jun 3, 2018

gx: update go-log #4974

Closed

Windows: cli colour codes #2124

Closed

This was referenced Jun 3, 2018

cli: Color on windows doesn't work #3068

Closed

Windows initiative 2018 #4808

Closed

whyrusleeping mentioned this pull request Jun 4, 2018

Efficient "repo stat" (DiskUsage) and "--size-only" flag #5010

Merged

hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023

Merge pull request ipfs/kubo#5007 from ipfs/feat/routing-refactor

1b95066

routing fixes/refactor This commit was moved from ipfs/kubo@c3e011b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

routing fixes/refactor #5007

routing fixes/refactor #5007

Stebalien commented May 9, 2018 •

edited

Loading

Stebalien May 9, 2018

Stebalien May 9, 2018

Stebalien May 9, 2018

dirkmc May 16, 2018

Stebalien May 16, 2018

kevina May 16, 2018

Stebalien May 17, 2018

whyrusleeping May 31, 2018

kevina commented May 31, 2018

Stebalien commented Jun 1, 2018

kevina commented Jun 1, 2018 •

edited

Loading

Stebalien commented Jun 1, 2018

kevina commented Jun 1, 2018 via email

whyrusleeping commented Jun 1, 2018

whyrusleeping left a comment

whyrusleeping Jun 2, 2018

Stebalien Jun 2, 2018

Stebalien Jun 2, 2018

Stebalien commented Jun 2, 2018 •

edited

Loading

routing fixes/refactor #5007

routing fixes/refactor #5007

Conversation

Stebalien commented May 9, 2018 • 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

kevina commented May 31, 2018

Stebalien commented Jun 1, 2018

kevina commented Jun 1, 2018 • edited Loading

Stebalien commented Jun 1, 2018

kevina commented Jun 1, 2018 via email

whyrusleeping commented Jun 1, 2018

whyrusleeping left a 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

Stebalien commented Jun 2, 2018 • edited Loading

Stebalien commented May 9, 2018 •

edited

Loading

kevina commented Jun 1, 2018 •

edited

Loading

Stebalien commented Jun 2, 2018 •

edited

Loading