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

Menu items for enabling pubsub and automatic GC #1647

Closed
lidel opened this issue Sep 16, 2020 · 9 comments · Fixed by #1740
Closed

Menu items for enabling pubsub and automatic GC #1647

lidel opened this issue Sep 16, 2020 · 9 comments · Fixed by #1740
Labels
area/go-ipfs Issues related to go-ipfs daemon (config, orchestration, features) effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked topic/design-front-end Front-end implementation of UX/UI work
Milestone

Comments

@lidel
Copy link
Member

lidel commented Sep 16, 2020

I was looking into starting go-ipfs with additional flags: --enable-pubsub-experiment (pubsub) and --enable-namesys-pubsub (IPNS over pubsub) and the main concern I've found was use of resources:

In order to enable pubsub by default in go-ipfs, we need to find some way for pubsub to not take up a bunch of resources when not in-use.
libp2p/go-libp2p-pubsub#332

So if we want pubsub and fast IPNS enabled, we need to pay the cost of higher resource utilization.
We could measure it and decide if its worth the added value, or prioritize a fix.

Updated: or at the very least, we could give user UI for enabling that with one click

Updated again: See mockup for placement of menu items for pubsub and automatic GC (note this also includes not-yet-implemented "Ask when opening" menu item, likely to land first in #1646)
image

@aschmahmann are there any other concerns that I don't see?

@autonome FYSA this also impacts go-ipfs in Brave, especially the speed of ipns://

@lidel lidel added need/triage Needs initial labeling and prioritization area/go-ipfs Issues related to go-ipfs daemon (config, orchestration, features) need/analysis Needs further analysis before proceeding labels Sep 16, 2020
@shaikh-shahid
Copy link

I think if we have a UI based switch to turn on/off the flags such as Pubsub and GC etc then that would be better for end users as well. I am currently working on a framework that does require IPFS in the backend and needs the PUBSUB to be enabled. Since the framework also consists of demos for general public as well, it's difficult for users to find the configuration and update the flags.

@lidel
Copy link
Member Author

lidel commented Sep 21, 2020

@shaikh-shahid that is very useful feedback, thank you.

I think we could avoid enabling pubsub by default by adding on/off checkbox settings for pubsub (and automatic GC) to the Desktop app menu. Toggling would automatically restart the deamon with the new setting.

@lidel lidel changed the title Enable pubsub by default Menu item for enabling pubsub Sep 21, 2020
@jessicaschilling jessicaschilling changed the title Menu item for enabling pubsub Menu items for enabling pubsub and automatic GC Sep 21, 2020
@jessicaschilling
Copy link
Contributor

Note: Added mockup of menu item placement in original comment.

Unless @aschmahmann or @autonome have objections, I believe this is ready to work.

@jessicaschilling jessicaschilling added exp/intermediate Prior experience is likely helpful effort/hours Estimated to take one or several hours good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked topic/design-front-end Front-end implementation of UX/UI work and removed need/analysis Needs further analysis before proceeding need/triage Needs initial labeling and prioritization labels Sep 21, 2020
@jessicaschilling jessicaschilling added this to the v0.13 milestone Sep 21, 2020
@lidel lidel modified the milestones: v0.13, v0.14 Sep 25, 2020
@andrew
Copy link
Contributor

andrew commented Jan 4, 2021

Here to echo @shaikh-shahid's comment, I'm working on a project that needs pubsub to be enabled and if they are using ipfs desktop, it's is not a friendly user experience.

@lidel
Copy link
Member Author

lidel commented Jan 12, 2021

@andrew I don't think we can enable it by default when its marked as experiment in go-ipfs, but we could prioritize adding the opt-in via menu visible in #1647 (comment).
Would that work for you?

@andrew
Copy link
Contributor

andrew commented Jan 13, 2021

Yeah not asking for it on by default but for a user to be able to set the option via desktop

@lidel lidel added P0 Critical: Tackled by core team ASAP P1 High: Likely tackled by core team if no one steps up and removed P0 Critical: Tackled by core team ASAP P2 Medium: Good to have, but can wait until someone steps up labels Jan 13, 2021
@andrew
Copy link
Contributor

andrew commented Jan 19, 2021

Opened a pull request to add the pubsub option: #1735

If that goes well then I'll open another with the automatic GC option.

@lidel
Copy link
Member Author

lidel commented Jan 19, 2021

Two thoughts:

  • We should keep pubsub items under Experiments section (its still listed as an experiment in go-ipfs docs)
  • There is a separate experiment called "IPNS over PubSub" (docs), but I think it should this be a separate flag, as it comes with additional overhead (afaik subscriptions are not purged atm)

andrew added a commit to andrew/ipfs-desktop that referenced this issue Jan 20, 2021
andrew added a commit to andrew/ipfs-desktop that referenced this issue Jan 20, 2021
@andrew
Copy link
Contributor

andrew commented Jan 22, 2021

PRs for the other to menu items mentioned here: #1739 and #1740

lidel added a commit that referenced this issue Jan 25, 2021
* feat: disable/enable gc via settings menu

Closes #1647
Very similar approach to #1735

* fix: ensure checkbox follows cli flag config

- setting default state to on for new and pre-existing config
- remove keysize (not needed since go-ipfs 0.7.0 uses ED25519 keys by
  default)

Co-authored-by: Marcin Rataj <lidel@lidel.org>
lidel added a commit that referenced this issue Jan 25, 2021
* feat: enable ipns over pubsub via settings menu 

Part of #1647

Very similar approach to #1735

* refactor: move pubsub experiments to exp. section

making it easier to find if someone edits config by hand

Co-authored-by: Marcin Rataj <lidel@lidel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/go-ipfs Issues related to go-ipfs daemon (config, orchestration, features) effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked topic/design-front-end Front-end implementation of UX/UI work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants