kubo icon indicating copy to clipboard operation
kubo copied to clipboard

Routing: `priority` and `mandatory` Router Params

Open ajnavarro opened this issue 2 years ago • 6 comments

Description

Coming from this discussion: https://github.com/ipfs/go-ipfs/pull/8997#discussion_r912546177 we need to figure out if we need and how to add a way to tweak router behavior.

  • priority – allows setting the order (when it matters)
  • mandatory – allows for marking some routers as mandatory and others as best-effort

ajnavarro avatar Jul 05 '22 14:07 ajnavarro

@ajnavarro : thanks for creating this issue. A few thing:

  1. I removed "Delegated" from the title. I think this is about routing configurability in general.
  2. I think we should update the issue to expand on the usecases we want to support. For example, mandatory makes sense to me. I assume that means that we don't return until all mandatory routers have returned (or timedout). Otherwise we return as soon as we get a response. Priority isn't clear to me.
  3. Can we put together a proposal for what the config of this will look like?

BigLep avatar Jul 25 '22 22:07 BigLep

Right now we have the following JSON format for routing configuration:

"Routers" : {
		"CidContact" : {
			"Type" : "reframe",
			"Enabled" : true
			"Parameters" : {
				"Address" : "<URL>"
				}
		}
}

Proposal:

  • Add a new Mandatory flag (false by default).
  • In my opinion, I would implement priority by order (this will simplify implementation and the configuration file). Even if maps in Go have no guaranteed order, we can parse the actual Routers format in a custom way to allow ordering.
"Routers" : {
		"CidContact1" : {
			"Type" : "reframe",
			"Enabled" : true,
			"Mandatory": true,
			"Parameters" : {
				"Address" : "<URL>"
				}
		},
       "CidContact2" : {
			"Type" : "dht",
			"Enabled" : false,
			"Parameters" : {
				"PARAM" : "VAL"
				}
		}
}

Internally, Routers will change from:

type Routing struct {
	Type *OptionalString `json:",omitempty"`
	Routers map[string]Router
}
type Router struct {
	Type string
	Enabled Flag `json:",omitempty"`
	Parameters map[string]string
}

To:

type Routing struct {
	Type *OptionalString `json:",omitempty"`
	Routers []Router
}
type Router struct {
	Type string
    Name string
	Enabled Flag `json:",omitempty"`
    Mandatory Flag `json:",omitempty"`
	Parameters map[string]string
}

The major workload here is not in the configuration changes, but in the new behavior that we will need to implement to support ordering and mandatory routers.

Right now we are using github.com/libp2p/go-libp2p-routing-helpers library, specifically the Tiered router to use all routers together. We need to make our own implementation of Tiered, supporting mandatory routers.

ajnavarro avatar Jul 26 '22 10:07 ajnavarro

@ajnavarro : thanks for the extra details. A few things on this:

  1. Why does order matter? Is this because we aren't requesting all the routers in parallel? Or is this about which router's results to prioritize?
  • If we aren't doing everything in parallel, then how long do we wait before we start executing the next in the priority list?
  1. How are results from multiple mandatory routers combined? This seems like an important thing to document.
  2. Where does someone specify the timeout value for a given router?

My initial thought is that we either:

  1. Keep this simple with logic like: "all routers are executed in parallel; we return as soon as all of the mandatory routers have returned or timed out; everything else that isn't mandatory has its response included if it completed in time; results are added to an array in the order corresponding with the router.

OR if that's good enough

  1. Go the full configurability route where you can have recursive "chains of routers" with different "strategies" for when to execute, how to combine results, etc. I assume there are good patterns we can follow for this, but am hopeful we don't need to go down this higher complexity path right now.

BigLep avatar Jul 26 '22 13:07 BigLep

Why does order matter?

It matters depending on how we implement the Routers Wrapper. Right now the actual behavior is:

  1. Execute method on all Routers in parallel. It fails if one fails:
    • PutValue
    • Provide
    • Bootstrap
  2. Execute GetValue on one Router at a time. Return the first non-nil result and stop the iteration.
    • GetValue
    • GetPublicKey
    • FindPeer
  3. Execute SearchValue on all Routers in parallel and mix the results.
    • SearchValue
    • FindProvidersAsync

All of these methods are using Context, so we have a timeout to stop the execution if some router is taking too long or we didn't have time to execute all of them.

How are results from multiple mandatory routers combined?

Maybe we need to define better what means mandatory. For me, mandatory will cause the execution to fail if it fails, but I don't know if it is the best thing to do or if it is useful to the user.

Where does someone specify the timeout value for a given router?

If we need it as a specific value per router, we can add it as a parameter (Parameters map[string]string), like reframe URL param.

I would go with option number 1, keeping things simple, only taking into account that not all methods are running in parallel (should we change actual method behaviors?). Do we know about specific use cases or personas?

ajnavarro avatar Jul 26 '22 15:07 ajnavarro

@ajnavarro : sorry not to know. Just for my own learning, can you please link me to the "Routers Wrapper" as it's implemented today?

BigLep avatar Aug 12 '22 07:08 BigLep

@ajnavarro : sorry not to know. Just for my own learning, can you please link me to the "Routers Wrapper" as it's implemented today?

Per 2022-08-12 verbal conversation, https://github.com/libp2p/go-libp2p-routing-helpers is what is being referred to here.

BigLep avatar Aug 12 '22 18:08 BigLep

Sorry for the delay @BigLep . We are specifically using https://github.com/libp2p/go-libp2p-routing-helpers/blob/master/tiered.go#L18

ajnavarro avatar Aug 15 '22 10:08 ajnavarro

Superseded by #9157 , #9188 and https://github.com/libp2p/go-libp2p-routing-helpers/issues/56

ajnavarro avatar Aug 23 '22 09:08 ajnavarro