Invert the service discovery

[jscheinblum]

This simplifies the service discovery and also is a little more flexible/generic.

The idea here is that we have one required param ("target") that defines which vtgate pool we're filtering from, then we can filter on any other discovery attribute.

This changes the logic to filter in the name resolution side, where we now filter down to the smallest possible list in the name resolution stage, then round robin between them later in the gate.

The purpose of this is that we only establish connections to the hosts we pick in name resolution - previously what was happening was we'd resolve to any matching host, but then only pick from matching hosts. That mean a larger than expected number of TCP connections, even if only a few were ever selected.

The new flow is:

grpc outbound connection: vtgate://<pool>/?filters....
-> vtgate:// discovery for $pool == conn
  -> get all matching service discovery entries == subconns
    [ wait for subconns to become active, then create a new picker]
    -> picker round robins between subconns

When the service discovery list changes, a new set of subconns is picked and used in the next round of outbound requests once they're active

			if (Str\contains($dbname, 'vifl')) {
				$target = 'vifl';
			} else if (Str\contains($dbname, 'aux')) {
				$target = 'aux';
			} else {
				$target = 'web';
			}
			$opts = new \AsyncMysqlConnectionOptions();
			$opts->setConnectTimeout($timeout_micros);
			$opts->setConnectionAttributes(dict[
				'target' => $target,
				'filter_az_id' => 'use1-az2',
			]);

[demmer]

Style nit -- we don't usually include the type, e.g.Int in var names

[demmer]

Another naming nit -- I think pairs is misleading. Can we change to hostInfo or something like that?

[demmer]

Nit - comment above needs to be changed

[demmer]

This is totally fine for the Slack use case, but I'm not sure is something we'll want to always have in the code for when we upstream it.

More fundamentally, I feel like the code as-written is trying to shoehorn two different behaviors into the same filters list.

The type is, in fact, a filter, since if that's specified and there aren't enough hosts that match, we will fail to route.

The other field(s) are actually an affinity but we're of course willing to route across AZs if there aren't enough available. Also we don't actually have any use case for more than one affinity field, and I'm having a hard time finding any reason for anyone upstream to really want that.

So, my suggestion is that we actually make this less generic, and rewrite this so that instead of the combined filters list, we change the signature of the resolver to something where we take in explicit configuration of something like:

type JSONGateConfigResolver struct {
        // If a field name is specified then filter the set of hosts to only those that match 
        // the given target, e.g. if targetTypeField is set to "type", then a route target of
        // vtgate://foo only matches hosts with "type: foo" in the host list
	targetTypeField string

        // If a field name is specified (e.g. "az_id"), then prefer routing to hosts that match
        // the affinity expressed in the target, e.g. if targetAffinityField is set to "az_id", 
        // then a target of vtgate://foo?az_id=1a will prefer hosts with "az_id: 1a" in the
        // host list, but may route to others if there are not enough candidates 
	targetAffinityField  hostFilters

	target   resolver.Target
	cc       resolver.ClientConn
	ticker   *time.Ticker
	rand     *rand.Rand // safe for concurrent use.

I think not only the comments but the more explicit separation between the two would make a lot of sense to me.

[demmer]

Hmm... what if you want 4 connections but there are only 2 in the same AZ. Seems like you would want to make sure to include those 2, but my read of the code below would ignore that and just fall back to the full set.

Implementation-wise I kinda think we could make this all a bit simpler if we did an approach like what I did here https://github.com/slackhq/vitess/pull/41/files#diff-7bddb8b37e422b4f434e7a7e7e55ee318a195c1235ca3284c18d5d11a640f3a4R186

Basically, take the whole json list, and first filter it to do the type matching, since that (as said above) is a real filter.

Then shuffle the whole thing randomly.

Then do another pass where we take the shuffled list and boost the ones that match the AZ affinity to the front of the list.

Then grab the first N.

This way you handle all the cases a bit more elegantly I think. Yes, it's a 2nd pass on the list, but this is small and since we only do this for the first connection to a given target I think it should be fine. If this is actually done each time we dial then we have bigger problems.