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.

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

Pubsub: fix config settings #39

Merged
merged 3 commits into from
Oct 18, 2017
Merged

Pubsub: fix config settings #39

merged 3 commits into from
Oct 18, 2017

Conversation

AquiGorka
Copy link
Contributor

As referenced here: ipfs/js-ipfs#1029 (comment)

This PR adds a random name to the IPFS repo name and set config params for a change in the mutliaddr dependency.

@AquiGorka AquiGorka requested a review from chadoh October 16, 2017 09:25
@AquiGorka AquiGorka mentioned this pull request Oct 16, 2017
@chadoh
Copy link
Contributor

chadoh commented Oct 16, 2017

I haven't had time to dig in too deeply. Will this cause the amount of data stored locally to bloat? What does the repo setting do, exactly?

@AquiGorka
Copy link
Contributor Author

AquiGorka commented Oct 17, 2017

For this specific issue, the random name is used to bust the IPFS cached configuration.

From what I've learned, repos (the name we are giving it) is how they identify nodes in the network and data can be shared between them.

At some point I think we should name the repo using the user information (and maybe some device information too).

A repository uniquely identifies a node. Running two different ipfs programs with identical repositories -- and thus identical identities -- WILL cause problems.

This warning is important. I am not sure of the full implications or randomly naming repos with each refresh but I'll keep researching.

Reference: https://github.com/ipfs/specs/tree/master/repo#notes

@chadoh
Copy link
Contributor

chadoh commented Oct 17, 2017

It looks like every page refresh will duplicate all of our IPFS data. Note especially that blocks is duplicated between the repo created with each page refresh—this is the actual GIF data, which is going to be HUGE. I don't know what the solution is to busting the cache when we need to, but creating a brand new repo on each refresh seems wildly wasteful.

indexedDB bloat

@chadoh
Copy link
Contributor

chadoh commented Oct 17, 2017

I tried looking into this briefly. I'm feeling too dumb to make sense of it at the moment. Sorry I couldn't offer anything more constructive than "this doesn't seem quite right"!

@AquiGorka AquiGorka changed the title Pubsub: fix config settings and bust cache Pubsub: fix config settings Oct 18, 2017
@AquiGorka
Copy link
Contributor Author

AquiGorka commented Oct 18, 2017

Ok.

I removed the cache busting (I suppose a clear site data or delete database in the Chrome dev console can can achieve the same reslt if we need to).

Added Bootstrap config param to remove the wss error we were seeing (reference here).

I notice the GIFs take less time to render, maybe the IPFS network received an update :)

Copy link
Contributor

@chadoh chadoh left a comment

Choose a reason for hiding this comment

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

I'm uncertain why we need this, as an IPFS team member says that the bug was fixed. But this doesn't seem problematic, outside of introducing possibly-superfluous code. If you think we do, in fact, need this code, then I'll trust you on it.

@AquiGorka AquiGorka merged commit 1cd1b39 into master Oct 18, 2017
@AquiGorka AquiGorka deleted the bug/fix-ipfs-pubsub branch October 18, 2017 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants