search

ssbc / secret-stack

connect peers to each other using secret-handshakes
MIT License
90 stars 19 forks source link

Support plugin dependencies and ordering #70

Closed devinivy closed 3 years ago

devinivy commented 3 years ago

It is fairly common for secret-stack plugins to have interdependencies. The simplest example is how so many ssb plugins rely on ssb-db. However, there are also far more interesting and complex cases, especially as you progressively add plugins to your application. See all the comments here in Manyverse, for example. Or this little workaround in ssb-friends. There are other challenges to managing this too, such as maintaining ordering while using user-defined plugins via ssb-plugins.

One way of dealing with this is to allow plugins to declare which other plugins they know they should come before or after. With the full description of all the interdependencies, you run a topological sort to obtain a linear ordering. Ideally this sort would maintain the original ordering of dependencies when possible. In the case of secret-stack, the sorting would occur when you call App(), since at that point it's known that all the plugins have been declared with use().

While this solves some problems, I also intend to propose a solution that I feel fits with with ssb's principles. In particular, of sufficiency, independence meeting interdependence, and having a local-first mindset— in this case plugins "locally" describing what conditions are sufficient for them to operate.

If this feature is welcome, I would be happy to contribute it.

API Proposal

The goal is to introduce this in a backwards-compatible way. If a plugin begins making use of these features it may be considered a breaking change for that plugin, but not for secret-stack itself.

Plugin Format

We'd add a new key dependencies to the plugin format.

App.use(plugin, [dependencies]) => App

A second argument dependencies would be added, with the same meaning as described above. This would allow applications to make use of this feature even if the plugins themselves do not. Hopefully this could immediately benefit applications with complex dependencies. If plugin also specifies plugin.dependencies, both sets of dependencies would be merged together.

App(config) => app

In the spirit of failing early and often, calling App() would have two new failure modes:

mixmix commented 3 years ago

This is a sweet suggestion. I'm torn between "hell yes go for it" and "can we pause and review all the needs of a good plugin system and prioritize?"

I'm leaning towards the former, because you've suggested something pretty tidy and backwards compatible.

Questions :

  1. What is the unique ID of a plugin?
    • is it the namespace (eg friends) or its name (eg ssb-friends)
    • how do we specify a version requirement?
  2. How do you ensure the endpoints you want are present?
    • eg ok something is registering the replicate namespace, but there are a few plugins which could do that. specifically I need to know ssb.replicate.request is present
    • I ask this because if we're checking dependancies, we might as well be specific... we could check the manifest...
  3. Is before needed?
    • it feels like it a plugin A shouldn't know that plugin B needs it, it should rather be up to plugin B to declare after: [A]
    • we can always add this later if it is proven needed
mixmix commented 3 years ago

Re (2), this feels like the spirit of deject (the module).

Another way of saying this is I think we could be writing multiple manifests :

  1. How can we support async plugin initialization?
    • it's quite common that a plugin has some database or state it wants to load up before being "ready". I wish this plugin system had a way to declare when a plugin was ready.
    • this feels like it might be useful with after, as I want to start my plugin after a plugin is I init'd and it's ready (see ssb-tribes for an example)
    • may be out of scope!
staltz commented 3 years ago

I'm a bit less diplomatic than mix, in that I think this kind of solution will have negative side effects, and doesn't guarantee this problem will be solved, to the point where I wouldn't want to see this merged and adopted. Also, this problem is currently solved, and it's not a high priority problem.

There's two ways of solving this problem: (1) at "compile-time" (what we do today), (2) at runtime (what you propose).

(1) means that we manually order the .use() calls until it all works. Note that many plugins throw errors if the ordering is incorrect, see example. So currently, the app developer can just use whatever ordering, run the app, see these thrown errors, adjust the .use() calls, and repeat until all of them are satisfied. Reordering dependencies is a common programming chore, we do it with variables and functions all the time. Only dataflow languages (Lucid) allow you to define variables in whatever order you want.

Doing (2) means more complex library code (think about the effort in maintaining this, not only the effort in submitting a PR) and a non-zero performance cost during startup when loading all the plugins.

Biggest issue in my opinion is that solution (2) still requires all plugins out there to define new dependencies field, and there are many plugins out there. Not only is it effortful to update all of them, you may be unaware of some, they may be hosted on git-ssb or gitlab, and you never know if the maintainers are responsive to merge a PR. Forking the module isn't going to help, because people might still use the original module. This is a lot of work, for a problem that I already consider solved via method (1).

Basically, it boils down to who do you want to cater to. (1) is a bad experience for an app developer unfamiliar with the modules, but (2) is bad for users and maintainers.

mixmix commented 3 years ago

I wasn't being diplomatic, I was exploring the problem space!

I think @staltz raises good points about it adding more costs to development in different ways. It's also a case whare staltz and i are familiar with reordering so we're not greatly effected and are probably not the right people to assess that cost for new devs.

@staltz cash you think of something more high priority which you'd rather solved?

christianbundy commented 3 years ago

Quickfix for when you don't want to read the source code to determine the correct order: shuffle your dependency order and figure out which ones fail tests. 😆

https://github.com/fraction/flotilla/blob/master/index.js

🌶️ Spicy take: It's a noble goal to try to improve this plugin system, but it would be lower-cost and higher-reward to switch to require() / import and ditch secret-stack's plugin system the same way that we ditched depject. Shared mutable state and spaghetti dependencies are pain points, and we don't need to reinvent JavaScript dependency management. 🌶️

staltz commented 3 years ago

@christianbundy Counter spice: secret-stack is not concerned merely with interdependent code, it's concerned with interdependent state (i.e. think of all the logic currently in each plugin's init section, running timers, etc). So if we "just use require" we'd have to reinvent state management (and by state I don't mean merely Redux-like big object, I mean all the timers, all the different muxrpc instances to remote peers, etc).

staltz commented 3 years ago

@staltz cash you think of something more high priority which you'd rather solved?

Security and reliability of modules overall, getting muxrpc@7 into shape, getting push-stream into shape, refactoring ssb-replicate/ssb-friends/ss-ebt triangle of death, refactoring muxrpc, documentation, performance, interoperability with Go or Rust, etc.

mixmix commented 3 years ago

Thanks @christianbundy and @staltz

@devinivy I'm hearing a "no thank you" and"would you be interested in any of those things staltz mentioned?'

devinivy commented 3 years ago

Thanks for all the thoughts, yall! I think I understand all the feedback— I hope it's cool if I leave some closing thoughts. Somehow it got pretty long, oops! :see_no_evil:

I had two main takeaways: first, that the issues mentioned are not a major part of the day-to-day life of ssb application and plugin developers, at least the ones who chimed in here. And I assume workarounds for ssb-plugin ordering are considered not-so-bad. There's concern that this work would create a maintenance burden that outweighs any benefits it may have. Regarding André's "think about the effort in maintaining this, not only the effort in submitting a PR" I totally hear you 👍. I also am a maintainer of some projects used and contributed to by others, and I need to be sensitive to these kinds of issues too. Perhaps that concern comes partially from myself not being a familiar face around here, and that I could easily drop a couple hundred lines of code on today's maintainers then move on. That wouldn't be my intention, but I understand the caution if it is a consideration.

My second takeaway came from hearing everyone's feedback and especially mix's thoughts, then doing some more research. The takeaway is that the order of calls to init() isn't really the biggest issue here— it's relevant but it also sort of buries the lede. I think that if in the future there is any interest in digging back into these problems, focusing on the initialization process would be a really fruitful angle to take. I do believe that secret-stack is uniquely positioned to ensure setup (and teardown) are coordinated smoothly, and if we continue to ask plugins solve these funky/stateful/timey issues on their own then applications aren't going to be quite as robust. If each plugin has to do work like this then subtle bugs might accumulate in applications over time, not necessarily reliable bugs that are easy to confirm by starting an app (again, I think I sort of missed the mark with my proposal being so focused on the order of calls to init()).

With both takeaways in mind, I would definitely pursue a different design than the original proposal to solve the issues described by myself and by mix: ideally with a bit more of an eye toward managing plugin setup/teardown. I believe secret-stack is best positioned to do this work, and I think it's a reasonable thing for a plugin system like this to speak to these issues— but I also think that given the concerns that have been raised, that it might be best to investigate a solution decoupled from a secret-stack (maybe even into a plugin 🤪) first.

I also am left with a couple confusions. But everyone has been generous with their input, definitely no pressure to followup.

Biggest issue in my opinion is that solution (2) still requires all plugins out there to define new dependencies field

I wasn't able to totally follow this. If you're a plugin and you don't define your dependencies then there's no serious side-effect that I can think of: it would just continue to be the application's job to order other plugins around you. Plugins that do define their dependencies would still function with old versions of secret-stack, and vice versa too. As an application you could even nix the dependencies property from the plugins if you weren't a fan of that behavior.

a non-zero performance cost during startup when loading all the plugins

It would take time to do this work at runtime, but I am not sure that it would be noticeable to loop through a few dozen plugins to sort them (this kind of sort takes time proportional to the size of the dependency graph) one time during startup, particularly if you compare it to the time a user might wait for I/O during startup. It's definitely something that we'd have been able to measure and consider. This objection felt a little bit like it was meant to bury the idea in "bad stuff"— I recognize it may not have been the intent though, just how it hit me.

@devinivy I'm hearing a "no thank you" and"would you be interested in any of those things staltz mentioned?'

Yeah! I am interested in poking around this question a little bit more, just out of personal interest and experimentation, but I don't think there's any reason secret-stack needs to remain implicated. I will plan to look into some of those items too. Thanks for all the careful and occasionally spicy thoughts, all :)

mixmix commented 3 years ago

Thanks @devinivy have enjoyed this conversation. Love how you framed this issue in a way which surfaced deeper issues, and I feel like you listened graciously and perceptively.

What's your id on ssb? One of mine is @G98XybiXD/amO9S/UyBKnWTWZnSKYS3YVB/5osSRHvY=.ed25519

devinivy commented 3 years ago

Aw, cheers. I appreciate the ideas you brought, and how you facilitated us in a nice direction 🙏 One of mine is @g0kx4R4gGDmBD3sH6VI4tyjGGLuDzCaZTDrIwMNZt2s=.ed25519, catch you out there!

devinivy commented 3 years ago

I did some experimenting and figured it would be nice to follow-up here with what I learned and came-up with: https://github.com/devinivy/secret-stack-lifecycle

The idea ended-up being to enable talking really granularly about dependencies— not specifically of one plugin on another, but possibly of one plugin's function on another plugin's function(s). And then to bring an application lifecycle to that (the steps you can interact with are listening, ready, closing, and closed), and the state management that comes with it. In the end I think this allows the plugin registration order to be fairly unimportant. It also allows you to depend on alternate implementations or capabilities rather than on specific plugins.

To work with dependencies granularly in this way I ended-up with a special kind of pull-stream which I extracted out into its own package: https://github.com/devinivy/pull-readyable. Each lifecycle step is a "readyable" and when you hook into a readyable you get another readyable— in this way the lifecycle for the application, plugins, and individual functions are all made up of the same stuff.

Here's a more in-depth example I cooked-up than what you'll find in the readme: https://gist.github.com/devinivy/ccf697c4664622ab34115c252a8f9f90

staltz commented 3 years ago

I have to say, really neat solution and design you found @devinivy! Bravo!