>Just to be clear, the ordering of "cachegroup permissions" doesn't matter because it's really a Set not an Array
>we'd have to overhaul TR Right. I was just pointing out future extensibility, not suggesting we do that now. I do think we could debate whether cachegroups or deliveryservices are the right place for localization priority. There are advantages to both. But yeah, that's in the distant future. I'm fine with leaving off the `priority` column; it's easy to add a column in the future, if we end up needing it. On Thu, Jul 5, 2018 at 10:27 AM, Gray, Jonathan <jonathan_g...@comcast.com> wrote: > Agreed provided that your join table has no properties (i.e. TableA_id, > TableB_id, order). > > On 7/5/18, 9:55 AM, "Volz, Dylan" <dylan_v...@comcast.com> wrote: > > 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" <jonathan_g...@comcast.com> > 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)" > <efrie...@cisco.com.INVALID> 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 < > robert.o.bu...@gmail.com> 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 < > mtorlue...@apache.org> > > wrote: > > > >> Inline. > >> > >> On Thu, Jul 5, 2018 at 9:00 AM, Rawlin Peters < > rawlin.pet...@gmail.com> > >> 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 < > jonathan_g...@comcast.com > >>> > >>> 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" < > mitchell...@gmail.com> 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 < > >>> mtorlue...@apache.org> > >>>> 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 < > >>> rawlin.pet...@gmail.com> > >>>>> 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 > >>>>>> <jonathan_g...@comcast.com> 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" < > >> rawlin.pet...@gmail.com> > >>> 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 > >>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>> > >>>> > >>> > >> > > > > > > > >
