Our APIs will be much easier to use, if we can stop
exposing join tables to the API altogether (e.g. cachegroup_fallback,
deliveryservice_regex), and only insert them into their proper objects
(cachegroup, deliveryservice), and then manipulate them within the
object
via POST/PUT/PATCH on the larger object.
+1 on this strategy, we already applied a join table this way for roles and
capabilities:
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/traffic_ops_golang/role/roles.go#L153
On 7/5/18, 9:49 AM, "Gray, Jonathan" <[email protected]> wrote:
And to be clear, this applies only to cachegroups and not delivery
services. Delivery services have a similar issue with "Deep Caching",
"Geolocation Provider", & "Geo Miss Default Lat/Lon".
- Jonathan
On 7/5/18, 9:36 AM, "Eric Friedrich (efriedri)"
<[email protected]> wrote:
Would the “permissions” field be better titled as “localizationMethods”
or “localizationPolicies"? Permissions typically relates to security and access
control, so it seems a bit out of place here
Also, should we consider adding the fallback list to the allowed
localization methods/policies?
This might allow us to remove the “fallbackToGeo” flag
If a backup list is specified and geo-location is not allowed, that
can be managed through this table rather than the one-off flag
Definitely +1 to keep this extensible, as there will be more
localization types in the future (Anycast, etc,… We at Cisco also have a
network hops localization policy engine too, although its not open-sourced)
—Eric
> On Jul 5, 2018, at 11:24 AM, Robert Butts <[email protected]>
wrote:
>
> +1 on listing them as an array. JSON arrays are defined to be
ordered, so
> that lets us use the array order as the lookup order, making it easy
to add
> that as a feature in the future.
>
> -1 on array types in the database. It's unnecessary denormalization,
and
> will be harder to deal with in the future. We can do the same thing
with a
> normal join table:
>
> cg | localization | priority
> ----------------------------
> foo | coverage_zone | 1
> foo | maxmind | 2
> foo | deep | 0
> bar | coverage_zone | 0
>
> Which maps to the config `"permissions": [ "deep", "coverage-zone",
> "geo"],` and also addresses your points on future types (we just add
a new
> enum value to the localization type). The additional `priority`
column lets
> us order it in the JSON array via SELECT ORDER BY.
>
> +1 on rethinking the cachegroup fallback API, too. We should have
more join
> tables, and better data normalization, but it's painful to use
HTTP/JSON
> APIs like that. Our APIs will be much easier to use, if we can stop
> exposing join tables to the API altogether (e.g. cachegroup_fallback,
> deliveryservice_regex), and only insert them into their proper objects
> (cachegroup, deliveryservice), and then manipulate them within the
object
> via POST/PUT/PATCH on the larger object.
>
>
> On Thu, Jul 5, 2018 at 9:23 AM, Mark Torluemke <[email protected]>
> wrote:
>
>> Inline.
>>
>> On Thu, Jul 5, 2018 at 9:00 AM, Rawlin Peters
<[email protected]>
>> wrote:
>>
>>> You're not mistaken, Jonathan, that is how the logic basically flows
>>> today; however, with cachegroup fallback configuration now it can be
>>> deep[if enabled] -> cz -> geo[if fallback config allows].
>>>
>>> Marking a cachegroup as "CZ-only" is really about filtering possible
>>> cachegroups in selection *after* a client's location is discovered
(by
>>> geoIP DB). So it can really be described as "given the method of
>>> client localization, filter out certain cachegroup targets".
>>>
>>> "Deep-only": filtered out after the "CZ localization" and "post-Geo
>>> localization" steps
>>> "CZ-only": filtered out after the "Geo localization" step
>>> "Geo-only": filtered out after the "deep/regular-CZ" steps
>>>
>>> Right now, those all seem somewhat mutually exclusive (except CZ
kind
>>> of includes both deep and regular CZ), unless you wanted to do
>>> something like "Deep and Geo only". I don't know if that's a valid
use
>>> case, but maybe "CZ and Future-thing only" would make sense. So
maybe
>>> cachegroups should really be described as:
>>>
>>> "Deep-enabled": allowed after "deep-CZ localization" step
>>> "CZ-enabled": allowed after the "regular-CZ localization" step
>>> "Geo-enabled": allowed after the "geo localization" step
>>> "Future-thing-enabled": allowed after the "future-thing" step
>>>
>>
>> +1. Well said.
>>
>>
>>>
>>> Then we could add multiple permissions to a cachegroup rather than
>>> making them mutually exclusive. For the initial use case I proposed
>>> this for, I would just be marking the cachegroups that aren't
directly
>>> connected to the internet as "Deep-enabled, CZ-enabled,
>>> future-thing-enabled" rather than "CZ-only". "Deep-only" cachegroups
>>> would be marked "deep-enabled".
>>>
>>> If no permissions are configured, all permissions are given.
Enabling
>>> one permission means all other permissions are disabled.
>>>
>>> I think this could be done by adding an array-type column to the
>>> cachegroup table/API like so:
>>>
>>> {
>>> "name": "cache_group_edge",
>>> "shortName": "cg_edge",
>>> "latitude": 12,
>>> "longitude": 45,
>>> "permissions": [
>>> "DEEP",
>>> "CZ",
>>> "GEO"
>>> ],
>>> "typeId": 6
>>> }
>>>
>>
>> The array feels a little fragile, perhaps. Should we decorate each
entry
>> with an "order", for completeness? (It's obvious to us that the
order is
>> DEEP->CZ->GEO, but might not be for new users.)
>>
>>
>>> The "permissions" field would remain optional, meaning that no
>>> permissions chosen assumes *all* permissions are enabled (giving you
>>> the existing default behavior). Postgres has array-type columns
which
>>> would map to this API quite nicely. In fact, I'm also kind of
>>> rethinking the "Cachegroup Fallback" API after going over this,
>>> because listing a cachegroup's fallbacks in an array like this could
>>> make sense and remove the need for the new cachegroup_fallbacks
API...
>>
>>
>> +1 here as well (without knowing the ins-and-outs of the Cachegroup
>> Fallback API).
>>
>>
>>
>>> On Wed, Jul 4, 2018 at 3:47 PM Gray, Jonathan
<[email protected]
>>>
>>> wrote:
>>>>
>>>> If I'm not mistaken, today the logic is fall through on miss
(deep[if
>>> enabled] -> cz -> geo). The suggestion to make a 1:* relationship
moves
>>> that from being implicit in the code to explicit choice by the DS
owner.
>>> Changing the database/apis is expensive enough that maybe we can't
make
>>> that jump near term. If you do only add a single field for this
using an
>>> enum, there should be a default 'fallthrough' or 'native' option to
>>> replicate the old behavior. Also, I misspoke earlier, this isn't
about
>> the
>>> routing methods it's about localization methods.
>>>>
>>>> On 7/4/18, 11:18 AM, "Jeremy Mitchell" <[email protected]>
wrote:
>>>>
>>>> Can a cache group have many "routing methods". If not, would one
>>> column
>>>> (cachegroup.routing_method) suffice where acceptable values are
an
>>> enum? (
>>>> CZ, DEEP, GEO)
>>>>
>>>> On Tue, Jul 3, 2018 at 7:05 PM, Mark Torluemke <
>>> [email protected]>
>>>> wrote:
>>>>
>>>>> Feels like columns in the cachegroup table could go a long way in
>>> the short
>>>>> term -- routing_cz, routing_geo, routing_deep?
>>>>>
>>>>> On Tue, Jul 3, 2018 at 1:04 PM, Rawlin Peters <
>>> [email protected]>
>>>>> wrote:
>>>>>
>>>>>> Thanks for the suggestion, Jonathan. I wasn't even thinking
>>> about the
>>>>>> possibility of "deep-only" cachegroups, but I could definitely
>>> see
>>>>>> that as a future use case.
>>>>>>
>>>>>> Do you envision something like a table of cachegroup
>> permissions
>>>>>> (cachegroup_id, cachegroup_permission enum type), where the
>> enums
>>>>>> would be DEEP_ONLY and CZ_ONLY to start with? That would
>>> definitely
>>>>>> increase initial dev time a bit and might require some new API
>>>>>> endpoints to add/remove permissions unless we can make the new
>>> API
>>>>>> field use an array value.
>>>>>> On Tue, Jul 3, 2018 at 12:23 PM Gray, Jonathan
>>>>>> <[email protected]> wrote:
>>>>>>>
>>>>>>> Rather than a cz_only flag, it might be more powerful to have
>>> a join
>>>>>> table with allowed routing methods. You could cover the same
>>> use case,
>>>>> but
>>>>>> it would also allow you to do certain things like deep_only or
>>>>> effectively
>>>>>> not-geo.
>>>>>>>
>>>>>>> - Jonathan
>>>>>>>
>>>>>>> On 7/3/18, 12:03 PM, "Rawlin Peters" <
>> [email protected]>
>>> wrote:
>>>>>>>
>>>>>>> Hey Traffic Controllers,
>>>>>>>
>>>>>>> I'd like to be able to mark a cachegroup as "CZ-only" via
>>> the API
>>>>> so
>>>>>>> that off-net clients (i.e. IPs that aren't included in
>> the
>>> Coverage
>>>>>>> Zone File) don't get routed to "CZ-only" cachegroups.
>> This
>>> is
>>>>> because
>>>>>>> some cachegroups aren't directly connected to the
>> internet
>>> and
>>>>>> routing
>>>>>>> off-net clients to those cachegroups is expensive. This
>>> proposal
>>>>>> would
>>>>>>> change Traffic Router behavior to route these clients to
>>> the
>>>>> closest
>>>>>>> available cachegroup that's NOT marked as "CZ-only".
>>>>>>>
>>>>>>> My proposal is to add a boolean column to the cachegroup
>>> table -
>>>>>>> cz_only - which is then included in the CRConfig's
>>> "edgeLocations"
>>>>>> for
>>>>>>> Traffic Router to parse and look up when routing off-net
>>> clients.
>>>>> By
>>>>>>> default it will be false to keep the existing behavior.
>>> The new
>>>>> field
>>>>>>> will be optional in the API but NOT NULL in the DB, so
>>> null values
>>>>> in
>>>>>>> requests will explicitly be set to false in the DB.
>>>>>>>
>>>>>>> Questions/concerns?
>>>>>>>
>>>>>>> - Rawlin
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
