Rawlin, That sounds like a solid plan, however, keep in mind that although it's unlikely, we may have users that have elected to change the default routing name for DNS delivery services. Thus, step 2 should not be run as defined, and steps 3 and 4 should be applied to the DNS routing name case in addition to the HTTP routing name case. This also applies to the pre-upgrade steps. I think this should be acceptable as long as we make sure to call it out in the release notes.
Along the lines of what Jeremy suggested, I think we can make the new delivery service routing name column not-null and not break the existing API, so long as we have suitable defaults. This should only apply to the create case, as it can remain optional once set for any future update. If we can leave the http.routing.name and dns.routing.name profile parameters in place, we will have a configurable default, and can fall back to something that's hardcoded (tr, edge) if the parameters are missing. -- Thanks, Jeff On Mon, Aug 14, 2017 at 5:25 PM, Peters, Rawlin <[email protected]> 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" <[email protected]> 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 <[email protected]> > 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 <[email protected]> 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 < > > [email protected]> > > > 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" <[email protected]> > > 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" <[email protected]> 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) < > > > > [email protected]> > > > > 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 < > > > > [email protected] > > > > > <mailto:[email protected]>> 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" < > > > [email protected]< > > > > mailto: > > > > > [email protected]>> 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
