Make hops configurable

[christianbundy]

I've been really trying to avoid "stage" buttons like what Patchwork has, but connecting to 1 hop seems like it might not have enough connections. I was reading your code and noticed this:

https://github.com/staltz/ssb-conn/blob/3a3020a6e28515c8a96bc8d353557e00b409408b/src/conn-scheduler.ts#L184

What would be the best way to make this a configurable value?

[staltz]

@christianbundy I would prefer that the conn-scheduler is forked so that such modification could be done. The architecture was designed so that all opinions are in conn-scheduler, while the rest is (as much as possible) unopinionated, and it should be easy to swap out the scheduler:

var conn = require('ssb-conn/core')
var gossip = require('ssb-conn/compat')
var connScheduler = require('./my-scheduler')

module.exports = [conn, connScheduler, gossip]

The reason why I would avoid a simple config field in ssb-config is that there are plenty of other opinions and values embedded in this default scheduler, that if we would allow all of them to be configured, we would with config files that look as large as Kubernetes config files. And at that point it would feel like trying to code with a config language, which is not Turing complete, and would still fall short of allowing everything to be customized.

I've been really trying to avoid "stage" buttons like what Patchwork has

I'm in support if you create an alternative scheduler that does away with staging. Just remember the tradeoffs and try to communicate those to the users. The downside of staging is that it requires explicit user input, like a naggy firewall might do. On the other hand, not having staging means the user is not in control and may end up connecting to very undesirable peers, e.g. opposite side of the political spectrum peers.

[christianbundy]

Thanks for the quick response. I don't see a simple way of forking your scheduler, so I'll probably just hack something together above the scheduler. My concern is that if I just copy and paste your file I won't be able to receive updates from you, but if I fork the entire repo then I have to maintain a fork of SSB-CONN. I'd write a scheduler myself, but since yours is ~600 lines I'm hesitant to even start.

On the other hand, not having staging means the user is not in control and may end up connecting to very undesirable peers, e.g. 'opposite side of the political spectrum' peers.

Right, I'd like to have a user-defined number of hops to resolve this.

[staltz]

Hmm, it sounds like the solution is suboptimal. Can you think of another way of tweaking/configuring this scheduler? Like what does 'hack something above the scheduler' look like?

[christianbundy]

I was thinking something like:

const ssbClient = require("ssb-client");
const pull = require("pull-stream");

const maxHops = 2;

ssbClient().then(api => {
  api.friends.hops().then(hops => {
    pull(
      api.conn.stagedPeers(),
      pull.drain(x => {
        x.forEach(([address, data]) => {
          const key = data.key;
          const haveHops = typeof hops[key] === "number";
          const hopValue = haveHops ? hops[key] : Infinity;
          if (hopValue <= maxHops) {
            console.log(`${address}: ⏳`);
            api.conn
              .connect(address, data)
              .then(() => console.log(`${address}: 👍`))
              .catch(() => console.log(`${address}: 👎`));
          }
        });
      })
    );
  });
});

[staltz]

That's actually quite an okay solution. If you're comfortable with that, I am too. This kind of approach works quite well in this use case, I'm just a bit concerned that other use cases might not have such a fortunate easy path. For instance implementing "do not automatically connect to staged friends" is quite hard because it works in the opposite direction of the opinions in the default scheduler, and would require a fork of the scheduler.

[christianbundy]

I share your concern. I'd really like to be able to download your changes and merge my improvements back upstream (e.g. we could subscribe to hopStream() so we don't have to restart the app for hop changes) but I'm very hesitant to reach into your modules with require("ssb-conn/core") or similar.

If you want people to fork the scheduler, maybe it should be its own module?

[christianbundy]

I was reading this today and it seems maybe relevant: https://en.wikipedia.org/wiki/Strategy_pattern

[staltz]

Yep! CONN basically already uses a Strategy pattern. The "interface" of the scheduler is:

interface Scheduler {
  constructor(ssb, config);
  start(): void;
  stop(): void;
}

And CONN Core will call the scheduler (whatever it is) with start and stop.

[christianbundy]

Hmm, it looks like my solution above is running connect multiple times on each connection, my guess is that that's bad?

[staltz]

@christianbundy Running connect redundantly is harmless, so feel free to do it multiple times. See:

https://github.com/staltz/ssb-conn-hub/blob/7eb0cd17b5b5563fc46cec5bc12255bd7983f917/src/index.ts#L180-L183

[christianbundy]

Hmm, maybe it's something else. When I run the above code it attempts dozens of connections per second and they only show up in the peer list of a tiny amount of time. My peer list jumps from 1 to 16 to 2 to 6 and continues jumping around every time that I refresh my peer list.

I can fix it with a done variable that tracks connection attempts and avoids repeating them, but there's something funky going on with the original script. Maybe it's hitting the connection limit and killing all of the connections?

Slightly better:

const ssbClient = require("ssb-client");
const pull = require("pull-stream");

const maxHops = 3;

const done = [];

ssbClient().then(api => {
  api.friends.hops().then(hops => {
    pull(
      api.conn.stagedPeers(),
      pull.drain(x => {
        x.filter(([address]) => done.includes(address) === false).forEach(
          ([address, data]) => {
            console.log(done.length);
            const key = data.key;
            const haveHops = typeof hops[key] === "number";
            const hopValue = haveHops ? hops[key] : Infinity;
            if (hopValue <= maxHops) {
              console.log(`${address}: ⏳`);
              done.push(address);
              api.conn
                .connect(address, data)
                .then(() => console.log(`${address}: 👍`))
                .catch(() => console.log(`${address}: 👎`));
            }
          }
        );
      })
    );
  });
});

[staltz]

Does the peer list show peers in state connecting or only those in connected? If allowing connecting, I would expect indeed a lot more churn.

And yes it's true that once there are many peers connected, the default scheduler will stay killing some.

[christianbundy]

Oh, it was showing all of them -- I didn't consider that peers we haven't finished connecting to would appear in the peer list. I've made a bug report for this as fraction/oasis#319, thanks!

[staltz]

I wonder why you reopened this issue? :)

[christianbundy]

Oops, guess I didn't send the comment!

Trying to make this work with a script means that I have to either:

• Attempt connections all the time with no backoff.
• Write my own backoff so that connections aren't retried so often.
• Only attempt connection to each peer once until the app is restarted.

Since the scheduler is bundled with SSB-CONN, is your suggestion to fork SSB-CONN and then maintain a downstream fork with my change?

[staltz]

is your suggestion to fork SSB-CONN and then maintain a downstream fork with my change?

No forks are required, you just need to make a package my-scheduler that will have ssb-conn as a dependency, then you import ssb-conn/core and ssb-conn/compat and then your plugin would be the array

var conn = require('ssb-conn/core')
var gossip = require('ssb-conn/compat')
var connScheduler = require('./my-scheduler')

module.exports = [conn, connScheduler, gossip]

I could have built this systems as three modules: ssb-conn-core, ssb-conn-compat and ssb-conn-default-scheduler, which ssb-conn aggregates as an array, but instead I just built it as ssb-conn/core, ssb-conn/compat and the default scheduler.

You pass this array to .use(require(...)) just like any other ssb plugin. Note that /core and /compat are not considered "internals" of SSB CONN, they are actually shortcuts to the internals. You can consider this a part of CONN's public API, I commit to not changing these.

[staltz]

you just need to make a package my-scheduler

Or, you could even just make your own scheduler as a (for instance) file in the Oasis codebase, and then that file gets bundled together with ssb-conn/core and ssb-conn/compat as the array. It's not strictly required to maintain your own library.

Also, it's not even required to use the same techniques that the default scheduler uses. You can get creative and build whatever timing functions to schedule connects and disconnects (as well as staging and unstaging).

[christianbundy]

Thanks, it's good to know that those are part of the public API. I'm very hesitant to reach into modules with relative require paths, but that makes it slightly better.

To be candid, I'm looking to collaborate on a scheduler rather than write my own from scratch or maintain a fork of your scheduler with just one change. It's just under 600 lines, which really isn't trivial for me to rewrite or casually fork. If that's not something you're interested in I suppose I can try to make a fork aimed at collaboration, but I think I'm just surprised to learn that the scheduler isn't open to contributions.

Anyway, I'll close this again, sorry.

[staltz]

Oh, the default scheduler is open to contributions, of course, as long as: (1) we don't disagree on the embedded opinions in the scheduler (the point is not to fight over which opinion is "right", but to recognize that they are just opinions and both sides should exist and be valid), (2) we don't go too deep into developing sophisticated configuration to tweak the embedded opinions.

[christianbundy]

Oh, I must have misunderstood, sorry about that. You wouldn't mind a PR to have config.conn.hops or something similar?