I don't think you can add a new not null column to the ds table because that would break the DS api. For example, on DS create/update you are saying routing_name is now required, right? This is an API breaking change that would require a new version of the API, no?
Hence my suggestion for a default. But, yes, i forgot about the per CDN thing. Jeremy On Mon, Aug 14, 2017 at 5:25 PM, Peters, Rawlin <rawlin_pet...@comcast.com> wrote: > Jeremy’s suggestion could work, but the param would probably be created in > a TR_PROFILE per-CDN. However, that still wouldn’t fix the visibility > problem. If a CDN isn’t using the default “tr” HTTP routing name, operators > would still need to know that there is a new profile parameter that needs > updating post-upgrade but before a snap/queue. So either way there needs to > be sufficient upgrade notes, but personally I still prefer keeping the > routing_name column non-null. > > That said, this is my current proposal for the DB migration which also > gets us past the upgrade issue: > 1. Add a routing_name column to the DeliveryService table. > 2. Update the routing_name for DNS Delivery Services to “edge”. > 3. Update the routing_name of non-DNS Delivery Services to the value of a > temporary upgrade parameter associated with the Delivery Service’s CDN (if > the upgrade parameter doesn’t exist, the routing_names will remain null). > 4. Update the remaining null routing_names to “tr”. > 5. Make the routing_name column non-null and add a non-empty constraint. > > So these would be an operator’s pre-upgrade steps: > 1. Verify if a custom http.routing.name has been configured for Traffic > Routers in their CDNs. > 2. If custom http.routing.name, do the following. Otherwise, no > pre-upgrade steps needed (for per-DS routing names at least): > a. create a parameter named “upgrade_http_routing_name” with the value > of the target CDN’s custom http.routing.name > b. associate this parameter to the TR_PROFILE belonging to the target > CDN > c. repeat steps 2a and 2b for each CDN using a custom > http.routing.name > > This would keep everything working the same post-upgrade as it did > pre-upgrade, and from that point on you’d be able to change a Delivery > Service’s routing name to any arbitrary hostname (without periods). > > --Rawlin > > On 8/14/17, 4:22 PM, "Dave Neuman" <neu...@apache.org> wrote: > > I don't think that solves the issue Rawlin was describing. The issue > that > Rawlin was describing is that someone has already defined a different > routing name in traffic_router/http.properties which is no longer > going to > be used after the upgrade. The upgrade needs to somehow know about > this > other routing name and use that when it creates the database column and > populates it with the pre-defined defaults (edge, tr). > > Also, creating a global param doesn't help those with more than 1 CDN. > > On Mon, Aug 14, 2017 at 4:09 PM, Jeremy Mitchell < > mitchell...@gmail.com> > wrote: > > > Adding a temp parameter would work but I worry that someone won't > read the > > upgrade documentation and forget to create this temporary parameter > before > > running the upgrade. > > > > Here's another option. > > > > Create 2 global TO parameters (http.routing.name and > dns.routing.name > > <http://http.routing.name/>) that default to tr and edge > respectively and > > make the ds.routing_name an optional field. > > > > in seeds.sql > > > > insert into parameter (name, config_file, value) values (' > > http.routing.name', > > 'global', 'tr') ON CONFLICT (name, config_file, value) DO NOTHING; > > insert into parameter (name, config_file, value) values (' > dns.routing.name > > ', > > 'global', 'edge') ON CONFLICT (name, config_file, value) DO NOTHING; > > > > in code (warning. ugly pseudo code to follow): > > > > function getRoutingName(ds) { > > return ds.routing_name if not null > > if (ds.type = HTTP) { > > return parameter.http.routing.name > > } else > > return parameter.dns.routing.name > > } > > } > > > > Just my 2 cents. > > > > On Mon, Aug 14, 2017 at 10:55 AM, Dave Neuman <neu...@apache.org> > wrote: > > > > > Good info Rawlin. > > > My vote would be for a parameter to be used during the upgrade. > We can > > set > > > a param called `upgrade_routing_name` or something similiar so > that it is > > > obvious that it is a param used for upgrade only. We should also > > document > > > that A) the param needs to be set before upgrade and B) TR will now > > ignore > > > the setting in the config file. Ideally we would remove the param > from > > the > > > default config and the need for the param in the code. > > > > > > On Wed, Aug 9, 2017 at 4:28 PM, Peters, Rawlin < > > rawlin_pet...@comcast.com> > > > wrote: > > > > > > > Hey all, > > > > > > > > I’ve dug through this a bit more, and defaulting a new > > > > DeliveryService.routing_name > > > > column to ‘tr’ for HTTP delivery services presents an upgrade > issue if > > a > > > > CDN has > > > > chosen to use a custom “http.routing.name” parameter for the > Traffic > > > > Routers > > > > in that CDN (by editing the http.properties files of the Traffic > > > Routers). > > > > > > > > For instance, if “http.routing.name” has been set to “ccr”, the > new > > > > routing name > > > > “tr” will break all of the clients using the old “ccr” delivery > service > > > > URL. > > > > > > > > Basically we need to provide a one-time upgrade step to allow > CDNs > > using > > > a > > > > custom > > > > “http.routing.name” to default the new routing_name column to > that > > value > > > > for > > > > existing HTTP delivery services. What would be the best way to > do this? > > > > Some options > > > > might be: > > > > 1. Add a profile parameter to the TR_PROFILE for that CDN. On > upgrade, > > > > read that > > > > parameter and use it to update the routing_name for existing > HTTP > > > > delivery services. > > > > After upgrade, you can safely remove the profile parameter. > > > > 2. Let the upgrade automatically default the routing_name to > ‘tr’ or > > > > ‘edge’. After > > > > upgrading, manually update each HTTP delivery service to use > the > > > > current > > > > “http.routing.name” in use (we could provide an API > endpoint to > > > “bulk > > > > update” the > > > > routing names for all HTTP delivery services in a CDN). > > > > > > > > Note this is not an exhaustive list, this is a just a couple > options > > that > > > > have come up in > > > > discussion so far. Feel free to add any more ideas to this list. > > > > > > > > After the upgrade has been completed, the “http.routing.name” > > parameter > > > > in the > > > > Traffic Router’s “http.properties” file will be ignored (same > with the > > “ > > > > dns.routing.name” > > > > parameter in “dns.properties” which I’m not sure can even be > changed > > > > safely today). > > > > > > > > Thoughts? > > > > > > > > --Rawlin > > > > > > > > On 8/4/17, 10:19 AM, "Peters, Rawlin" <rawlin_pet...@comcast.com > > > > wrote: > > > > > > > > @Dave @JvD > > > > > > > > Thanks for the feedback. I think I can get on board with > defaulting > > > > the DS columns to ‘edge’ and ‘tr’. I was thinking the CDN > columns might > > > be > > > > useful if the user just wants to set it CDN-wide and not > individually > > on > > > > each DS, but I guess if we default it as part of the upgrade > migration, > > > we > > > > should also provide an API endpoint to set the routing names on > all > > DSes > > > in > > > > a CDN to a single value, thus still providing a “per-CDN” option. > > Would a > > > > “bulk” update also be useful, in case a user wants to update a > handful > > of > > > > DSes to the same routing names at once? > > > > > > > > @JvD Re: TR_PROFILE vs. DS_PROFILE > > > > The default would come from a TR_PROFILE linked to the CDN, > and the > > > > override would come from a DS_PROFILE linked to that specific > DS. I > > > looked > > > > into those options to cover all my bases, but I think adding > columns to > > > at > > > > least the DS table and not touching profiles at all is the better > > option. > > > > > > > > -Rawlin > > > > > > > > On 8/4/17, 8:04 AM, "Jan van Doorn" <j...@knutsel.com> wrote: > > > > > > > > Agree with Dave on > > > > > > > > [*DN] we should default the database column to "edge" > for DNS > > and > > > > "tr" for* > > > > *http. Then we don't have to do the null check.* > > > > > > > > If we do that, we can make the columns mandatory, and it > makes > > > > sense > > > > they're not in the DS_PROFILE. Also makes it so we don't > have > > to > > > > have a CDN > > > > wide setting. (and Rawlin, I think you mean to say > DS_PROFILE > > > > rather than > > > > TR_PROFILE type to add the param to if we chose to do > that?? Or > > > > was it the > > > > default that goes into TR_PROFILE and the override into > > > > DS_PROFILE?). > > > > In any case - if we make the columns NOT NULL and > default them > > to > > > > "tr" and > > > > "edge", I'm +1 on columns in the deliveryservice table. > > > > > > > > > > > > > > > > Cheers, > > > > JvD > > > > > > > > On Fri, Aug 4, 2017 at 7:12 AM Eric Friedrich (efriedri) > < > > > > efrie...@cisco.com> > > > > wrote: > > > > > > > > > Hey Rawlin- > > > > > Zhilin has also been working on a very similar > feature > > which > > > > was > > > > > proposed on this mailer last month: > > > > > https://lists.apache.org/thread.html/ > > > > 51d7ed1ae65a3697c39edd00236e6f3897da37ef5b24ac452a17cabb@% > > > > 3Cdev.trafficcontrol.apache.org%3E > > > > > < > > > > > https://lists.apache.org/thread.html/ > > > > 51d7ed1ae65a3697c39edd00236e6f3897da37ef5b24ac452a17cabb@ > > > > > <dev.trafficcontrol.apache.org>> > > > > > > > > > > Can you please work him to ensure we don’t duplicate > work and > > > > that if both > > > > > solutions are needed they will work together? > > > > > > > > > > On Aug 3, 2017, at 3:57 PM, Peters, Rawlin < > > > > rawlin_pet...@comcast.com > > > > > <mailto:rawlin_pet...@comcast.com>> wrote: > > > > > > > > > > Sorry, Outlook converted my numbered list poorly. I’ve > > > corrected > > > > the > > > > > numbering (items 1-3) below. > > > > > > > > > > On 8/3/17, 1:52 PM, "Peters, Rawlin" < > > > rawlin_pet...@comcast.com< > > > > mailto: > > > > > rawlin_pet...@comcast.com>> wrote: > > > > > > > > > > Hello All, > > > > > > > > > > I’ve been working on adding support for configurable > > per-CDN > > > > and > > > > > per-DeliveryService routing names [1] (what are > currently > > > > > hardcoded/defaulted to ‘edge’ and ‘tr’ for DNS and HTTP > > > Delivery > > > > Services, > > > > > respectively), and I have a few things to propose. > > > > > > > > > > > > > > > 1. Add a column to the CDN table for the DNS and > HTTP > > > > routing names. > > > > > > > > > > > > > > > > > > > > I’ve currently been working off the assumption that > > per-CDN > > > > routing > > > > > names would be configurable by adding ‘ > http.routing.name’ > > and > > > ‘ > > > > > dns.routing.name’ parameters to a profile of type > TR_PROFILE > > > > using the > > > > > ‘CRConfig.json’ config file. To me this seems like bad > UX > > > > because the user > > > > > has to click through multiple steps and fill in > multiple > > fields > > > > in the UI > > > > > just to change the CDN’s routing names. It also > requires > > > joining > > > > a few > > > > > different tables in the DB just to find the parameters > > per-CDN. > > > > For that > > > > > reason, I think it would be better if > ‘dns_routing_name’ and > > > > > ‘http_routing_name’ were added as columns of the ‘cdn’ > table, > > > > and changing > > > > > them via the UI would follow the same process as > choosing the > > > > CDN’s domain > > > > > name. Because the routing names would be the CDN-wide > > defaults, > > > > the ‘Edit > > > > > CDN’ window feels like the most natural place to put > it. > > > > > > > > > > > > > > > 2. Values for per-DeliveryService routing names > could > > > live > > > > in one of > > > > > a couple different areas: > > > > > * New columns in the delivery_service table > > > > > * Parameters in a DS Profile > > > > > > > > > > As the developer, my vote would be for Option A > because it > > > > seems like > > > > > it would lead to cleaner code in Traffic Ops because > the > > > routing > > > > names > > > > > would be readily-available when handling a > DeliveryService. > > You > > > > wouldn’t > > > > > have to also fetch its profile then dig through it to > find > > the > > > > routing > > > > > names. A downside could be that adding columns to an > > > > already-overcrowded > > > > > table isn’t ideal. > > > > > > > > > > Option B is less appealing to me but might have some > > > > advantages such as > > > > > keeping the number of columns down in the > DeliveryService > > > table. > > > > However, > > > > > DS Profiles currently seem to be geared more towards > the > > > > Multi-site Origin > > > > > feature in generating specific ATS configuration > > > (parent.config) > > > > and less > > > > > towards a “junk drawer for optional config”. As the > routing > > > > names would > > > > > affect the entire DS and multiple config files, it > doesn’t > > seem > > > > right to > > > > > have it as a profile parameter using ‘CRConfig.json’ > as the > > > > config file. I > > > > > wasn’t around when DS Profiles were introduced, so if > you are > > > > more familiar > > > > > with their purpose/origin and think this is a good fit > for > > > them, > > > > I’d like > > > > > to hear your advice. > > > > > > > > > > > > > > > 3. If per-DeliveryService routing names are not > set > > > > explicitly for a > > > > > DS (i.e. the column is null), then the DS will use the > > per-CDN > > > > routing > > > > > names as a default. If the per-CDN routing names are > unset, > > > they > > > > will > > > > > default to the current values of ‘edge’ and ‘tr’. So > the > > lookup > > > > hierarchy > > > > > would be DS.routing_names -> CDN.routing_names -> > default > > > > ‘edge/tr’. > > > > > > > > > > I’d like to know what you think of these proposals, > and > > any > > > > > advice/feedback is welcome. > > > > > > > > > > Best regards, > > > > > Rawlin > > > > > > > > > > [1] https://issues.apache.org/jira/browse/TC-287 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
