We should also have some uniqueness constraints on the new table
{primary, fallback} and {primary, order}. 




—Eric

> On Mar 13, 2018, at 12:59 PM, Rawlin Peters <rawlin.pet...@gmail.com> wrote:
> 
> To clarify, if we got a hit in the CZF for the client's IP, then we
> should *not* fail when all specified backup CGs are unavailable,
> fallbackToClosest is set to True, and the DS is set to "CZF only". In
> that case we should find the closest available CG (and fail if there
> are none). If your current implementation does not follow that
> behavior, it should be fixed to do so.
> 
> Feel free to close your existing PR and create a new one if that's
> easier for you. Just be sure to add a comment in the old PR that
> references your new PR, and I'll continue to review.
> 
> Also, you can check traffic_ops_golang/routes.go [1] to see if
> existing Perl API endpoints have been rewritten into Go yet. I don't
> see the /cachegroup endpoints in that file yet, so you should be
> alright to update the existing Perl endpoints. However, any *new*
> cachegroup API endpoints needed should be written in
> traffic_ops_golang.
> 
> As for DB schema updates for this, I was thinking one new column
> (fallback_to_closest - nullable, default true) in the cachegroup table
> and a new cachegroup_fallback table with at least 3 columns (primary -
> FK of the primary CG, fallback - FK of the fallback CG, order -
> integer specifying the order of the fallback list).
> 
> So given that, I imagine we'll need a new API endpoint like
> `/api/1.3/cachegroups/{:id}/fallbacks` that we can use to add, remove,
> or update ordering of fallbacks for a particular CG. This API should
> be restricted to cachegroups of type EDGE_LOC only.
> 
> However, I haven't done much new API work yet, so hopefully the
> contributors who've done a lot of API work can chime in and make sure
> the new API endpoint is in line with things like Swagger and other
> requirements.
> 
> - Rawlin
> 
> [1] 
> https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/traffic_ops_golang/routes.go
> 
> On Tue, Mar 13, 2018 at 10:05 AM, David Neuman <david.neuma...@gmail.com> 
> wrote:
>> What happens when Geo Limit is set to "CZF Only"  with all backup Caches
>> are unavailable and fallbackToClosest is set to True. Current
>> implementation will fail this. Should we do Geo lookup now in this change?
>> 
>> In this case we should fail. If the Geo Limit is set to “CZF Only” then
>> that is all we use.
>> 
>> 
>> On Tue, Mar 13, 2018 at 8:17 AM, Vijay Anand <
>> vijayanand.jayaman...@gmail.com> wrote:
>> 
>>> @Rawlin,
>>> 
>>> Let me try and get the changes done for TR according to your suggestions.
>>> This change would be like:
>>> 1. CZF only contains cache groups which should map back to TO's Cache Group
>>> configurations (cr-config)
>>> 2. Backup configurations should reach TR via cr-config in the format you
>>> detailed.
>>> 3. fallbackToClosest will be True by default. If backupLocation config is
>>> present, it will be assumed as false unless otherwise it is stated as TRUE
>>> explicitly.
>>> 4. This will work irrespective of the coverage Zones in CZF as long as the
>>> backup Cache Group specified is in cr-config.
>>> 
>>> I have a doubt in this as well.
>>> 
>>> What happens when Geo Limit is set to "CZF Only"  with all backup Caches
>>> are unavailable and fallbackToClosest is set to True. Current
>>> implementation will fail this. Should we do Geo lookup now in this change?
>>> 
>>> Shall i delete my existing PR and create a new one with these changes?
>>> 
>>> I will try to get the necessary changes for TO (Perl Mojo) along with this.
>>> Would require your help in TO (Golang) and TP. Will keep you posted.
>>> 
>>> Thanks,
>>> Vijayanand S
>>> 
>>> 
>>> 
>>> 
>>> On Tue, Mar 13, 2018 at 3:04 AM, Rawlin Peters <rawlin.pet...@gmail.com>
>>> wrote:
>>> 
>>>> If we start by putting this in the Cache Group API first, then later
>>>> we really only have to worry about adding the CIDRs to the API. The
>>>> backup config is really just relationships between cache groups, which
>>>> makes perfect sense to model in a relational DB rather than the CZF.
>>>> Why put something in the CZF to just tear it out later?
>>>> 
>>>> - Rawlin
>>>> 
>>>> On Mon, Mar 12, 2018 at 3:12 PM, Dave Neuman <neu...@apache.org> wrote:
>>>>> Good point Rawlin, but I think it does answer your questions.  CZF for
>>>> now,
>>>>> whatever the new CZF thing is after that.
>>>>> 
>>>>> On Mon, Mar 12, 2018 at 1:44 PM, Rawlin Peters <
>>> rawlin.pet...@gmail.com>
>>>>> wrote:
>>>>> 
>>>>>> The original scope of this thread was determining where the Backup
>>>>>> Cache Group config should live (API vs CZF), not necessarily about
>>>>>> building the entire CZF in the database, although I'm +1 on that idea
>>>>>> as well. I think any decisions made about doing that should probably
>>>>>> be started in a separate thread.
>>>>>> 
>>>>>> - Rawlin
>>>>>> 
>>>>>> On Mon, Mar 12, 2018 at 1:11 PM, Dave Neuman <neu...@apache.org>
>>> wrote:
>>>>>>> +1 on building the CZF in the database.  Jan tried to go down that
>>>> rabbit
>>>>>>> hole but realized it was a pretty hard problem to solve.  I think
>>>> this is
>>>>>>> something we might want to re-visit.  Maybe this is something we
>>>> should
>>>>>>> discuss at our meetup and then update this thread with our
>>> decisions?
>>>>>>> 
>>>>>>> On Mon, Mar 12, 2018 at 11:25 AM, Rawlin Peters <
>>>> rawlin.pet...@gmail.com
>>>>>>> 
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> @VijayAnand:
>>>>>>>> 
>>>>>>>> Right, a Coverage Zone that doesn't map to a Cache Group in TO
>>> won't
>>>>>>>> be chosen as a backup in case of failure, but you could have a
>>>>>>>> Coverage-Zone-not-in-TO that configures Coverage-Zones-in-TO as
>>>>>>>> backups. But, I think the general sentiment right now is that all
>>>>>>>> Coverage Zones in the CZF should map back to Cache Groups in TO, so
>>>>>>>> the backup config should also be done via the Cache Group API.
>>>>>>>> 
>>>>>>>> So from the Traffic Router perspective, the process should become:
>>>>>>>> 1. Rather than parsing from the CZF into the NetworkNode class,
>>> parse
>>>>>>>> Cache Group backup config from the CRConfig into the existing
>>>>>>>> CacheLocation class
>>>>>>>> 2. in the DS request flow, the NetworkNode will map back to a
>>>>>>>> registered CacheLocation which would contain the backup CG config
>>>>>>>> 
>>>>>>>> The rest of the PR's behavior should stay the same, it's just a
>>>> matter
>>>>>>>> of the config being located in a different class. To give you an
>>> idea
>>>>>>>> of how I would format it in the CRConfig (so you know how to parse
>>> it
>>>>>>>> out), here is a snippet of "edgeLocations" from CRConfig.json:
>>>>>>>> 
>>>>>>>> "edgeLocations": {
>>>>>>>>    "edge-cg-1": {
>>>>>>>>      "latitude": 1.00,
>>>>>>>>      "longitude": 2.00,
>>>>>>>>      "backupLocations": {
>>>>>>>>          "list": ["edge-cg-2"],
>>>>>>>>          "fallbackToClosest": false
>>>>>>>>      }
>>>>>>>>    },
>>>>>>>>    "edge-cg-2": {
>>>>>>>>      "latitude": 3.00,
>>>>>>>>      "longitude": 4.00
>>>>>>>>    },
>>>>>>>> }
>>>>>>>> 
>>>>>>>> The "backupLocations" section would still remain optional (if
>>>> missing,
>>>>>>>> follow existing behavior of falling back to next closest CG).
>>>> Existing
>>>>>>>> defaults in the current PR should remain the same.
>>>>>>>> 
>>>>>>>> How would you feel about making those changes in your PR? Feel free
>>>> to
>>>>>>>> tackle the new TO API and Traffic Portal changes too if you want,
>>> but
>>>>>>>> I don't want to burden you with this unexpected work if you don't
>>>> want
>>>>>>>> it. I (or another willing contributor) could work on the necessary
>>> TO
>>>>>>>> API and Traffic Portal changes sometime in the near future and
>>>>>>>> integrate them with your Traffic Router enhancement.
>>>>>>>> 
>>>>>>>> - Rawlin
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On Mon, Mar 12, 2018 at 7:39 AM, vijayanand.jayaman...@gmail.com
>>>>>>>> <vijayanand.jayaman...@gmail.com> wrote:
>>>>>>>>> 
>>>>>>>>> Rawlin,
>>>>>>>>> 
>>>>>>>>> I believe the following statement is not correct.
>>>>>>>>> 
>>>>>>>>> <Snip>
>>>>>>>>> However, after reading your initial proposal below, it sounds
>>> like
>>>> you
>>>>>>>>> might have Coverage Zones in your CZF that don't necessarily map
>>>> back
>>>>>>>>> to Cache Groups in TO. Might that be the case?
>>>>>>>>> </Snip>
>>>>>>>>> 
>>>>>>>>> We can have Coverage Zones in CZF which don’t necessarily map in
>>> to
>>>>>> TO’s
>>>>>>>> configured list of Cache Groups. But then , it won’t be chosen as a
>>>>>> valid
>>>>>>>> backup in case of failure.
>>>>>>>>> 
>>>>>>>>> For example:
>>>>>>>>> GROUP1 and GROUP2 are Cache Groups configured in TO (and hence
>>>>>>>> cr-config) , where GROUP3 is not in TO. Even though GROUP3 is
>>>> specified
>>>>>> as
>>>>>>>> a backup for GROUP1, it wont be  chosen in case of GROUP1 failure ,
>>>>>> since
>>>>>>>> it is not in TO.
>>>>>>>>> {
>>>>>>>>>  "coverageZones": {
>>>>>>>>>     "GROUP3": {
>>>>>>>>>      "network6": [
>>>>>>>>>        "1234:567a::\/64",
>>>>>>>>>        "1234:567b::\/64"
>>>>>>>>>      ],
>>>>>>>>>      "network": [
>>>>>>>>>        "10.197.89.0\/24"
>>>>>>>>>      ]
>>>>>>>>>    },
>>>>>>>>> 
>>>>>>>>>     "GROUP2": {
>>>>>>>>>      "network6": [
>>>>>>>>>        "1234:567a::\/64",
>>>>>>>>>        "1234:567b::\/64"
>>>>>>>>>      ],
>>>>>>>>>      "network": [
>>>>>>>>>        "10.197.69.0\/24"
>>>>>>>>>      ]
>>>>>>>>>    },
>>>>>>>>>    "GROUP1": {
>>>>>>>>>   "backupZones":{
>>>>>>>>>      "list": ["GROUP3"],? This wont be chosen as backup Cache
>>>> Group
>>>>>> in
>>>>>>>> case of failure , since it is not in crconfig.
>>>>>>>>>      "fallbackToClosestGroup":false
>>>>>>>>>   },
>>>>>>>>>      "network6": [
>>>>>>>>>        "1234:5677::\/64",
>>>>>>>>>        "1234:5676::\/64"
>>>>>>>>>      ],
>>>>>>>>>      "network": [
>>>>>>>>>        "10.126.250.0\/24"
>>>>>>>>>      ]
>>>>>>>>>    }
>>>>>>>>>  }
>>>>>>>>> }
>>>>>>>>> 
>>>>>>>>> So, i feel, the existing implementation of specifying backupZones
>>>>>>>> configuratioin in CZF should be fine.
>>>>>>>>> 
>>>>>>>>> Thanks,
>>>>>>>>> Vijayanand S
>>>>>>>>> 
>>>>>>>>> On 2018/03/09 18:31:56, Rawlin Peters <rawlin.pet...@gmail.com>
>>>>>> wrote:
>>>>>>>>>> Hey Eric (and others),
>>>>>>>>>> 
>>>>>>>>>> I'm resurrecting this thread because the PR [1] implementing
>>> this
>>>>>>>>>> proposed functionality is just about ready to be merged. The
>>> full
>>>>>>>>>> mailing list discussion can be read here [2] if interested.
>>>>>>>>>> 
>>>>>>>>>> I've discussed this PR a bit more with my colleagues here at
>>>> Comcast,
>>>>>>>>>> and while it provides the functionality we need, we think in the
>>>>>>>>>> long-term this configuration should live in the Cache Group API
>>> in
>>>>>>>>>> Traffic Ops rather than just the Coverage Zone File.
>>>>>>>>>> 
>>>>>>>>>> However, after reading your initial proposal below, it sounds
>>> like
>>>>>> you
>>>>>>>>>> might have Coverage Zones in your CZF that don't necessarily map
>>>> back
>>>>>>>>>> to Cache Groups in TO. Might that be the case? That scenario
>>>> seems to
>>>>>>>>>> be allowed by Traffic Router but might not necessarily be
>>>> "supported"
>>>>>>>>>> given the CZF docs [3] that state:
>>>>>>>>>>> "The Coverage Zone File (CZF) should contain a cachegroup name
>>>> to
>>>>>>>> network prefix mapping in the form:"
>>>>>>>>>> 
>>>>>>>>>> If we do indeed "support" this scenario, that would mean that
>>>> having
>>>>>>>>>> the backupZone config only in TO wouldn't solve all your use
>>>> cases if
>>>>>>>>>> your CZF heavily uses Coverage Zones that don't directly map to
>>> a
>>>>>>>>>> Cache Group in TO.
>>>>>>>>>> 
>>>>>>>>>> If we should officially support this scenario, then maybe we
>>> merge
>>>>>> the
>>>>>>>>>> PR [1] as is, then later we can augment the feature so that we
>>> can
>>>>>> use
>>>>>>>>>> the Cache Group API to provide the backupZone config as well as
>>> in
>>>>>> the
>>>>>>>>>> CZF. If the config was provided in both the API and the CZF,
>>> then
>>>> the
>>>>>>>>>> API would take precedent.
>>>>>>>>>> 
>>>>>>>>>> If this scenario should NOT officially be supported, then I
>>> think
>>>> we
>>>>>>>>>> should update the PR [1] to have Traffic Router parse the config
>>>> from
>>>>>>>>>> CRConfig.json rather than the CZF and augment the Cache Group
>>> API
>>>> to
>>>>>>>>>> support the backupZone config. I think this would be the most
>>>> ideal
>>>>>>>>>> solution, but I also don't want to sign up our contributors for
>>>> extra
>>>>>>>>>> work that they weren't planning on doing. I'd be happy to help
>>>>>> augment
>>>>>>>>>> this feature on the TO side.
>>>>>>>>>> 
>>>>>>>>>> What do you all think of this proposal? TO-only or both TO and
>>>> CZF?
>>>>>>>>>> 
>>>>>>>>>> - Rawlin
>>>>>>>>>> 
>>>>>>>>>> [1] https://github.com/apache/incubator-trafficcontrol/pull/
>>> 1908
>>>>>>>>>> [2] https://lists.apache.org/thread.html/
>>>>>> b033b3943c22a606370ad3981fa05f
>>>>>>>> b0e7039161b88bbc035bc49b25@%3Cdev.trafficcontrol.apache.org%3E
>>>>>>>>>> [3] http://traffic-control-cdn.readthedocs.io/en/latest/
>>>>>>>> admin/traffic_ops/using.html#the-coverage-zone-file-and-asn-table
>>>>>>>>>> 
>>>>>>>>>> On 2016/12/22 19:28:17, Eric Friedrich (efriedri) <
>>>>>> efrie...@cisco.com>
>>>>>>>> wrote:
>>>>>>>>>>> The current behavior of cache group selection works as follows
>>>>>>>>>>> 1) Look for a subnet match in CZF
>>>>>>>>>>> 2) Use MaxMind/Neustar for GeoLocation based on client IP.
>>>> Choose
>>>>>>>> closest cache group.
>>>>>>>>>>> 3) Use Delivery Service Geo-Miss Lat/Long. Choose closest
>>> cache
>>>>>> group.
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> For deployments where IP addressing is primarily private (say
>>>>>>>> RFC-1918 addresses), client IP Geo Location (#2) is not useful.
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> We are considering adding another field to the Coverage Zone
>>>> File
>>>>>>>> that configures an ordered list of backup cache groups to try if
>>> the
>>>>>>>> primary cache group does not have any available caches.
>>>>>>>>>>> 
>>>>>>>>>>> Example:
>>>>>>>>>>> 
>>>>>>>>>>> "coverageZones": {
>>>>>>>>>>> "cache-group-01": {
>>>>>>>>>>> “backupList”: [“cache-group-02”, “cache-group-03”],
>>>>>>>>>>> "network6": [
>>>>>>>>>>> "1234:5678::\/64”,
>>>>>>>>>>> "1234:5679::\/64"],
>>>>>>>>>>> "network": [
>>>>>>>>>>> "192.168.8.0\/24",
>>>>>>>>>>> "192.168.9.0\/24”]
>>>>>>>>>>> }
>>>>>>>>>>> 
>>>>>>>>>>> This configuration could also be part of the per-cache group
>>>>>>>> configuration, but that would give less control over which clients
>>>>>>>> preferred which cache groups. For example, you may have cache
>>> groups
>>>> in
>>>>>> LA,
>>>>>>>> Chicago and NY. If the Chicago Cache group fails, you may want some
>>>> of
>>>>>> the
>>>>>>>> Chicago clients to go to LA and some to go to NY. If the backup CG
>>>>>>>> configuration is per-cg, we would not be able to control where
>>>> clients
>>>>>> are
>>>>>>>> allocated.
>>>>>>>>>>> 
>>>>>>>>>>> Looking for opinions and comments on the above proposal, this
>>> is
>>>>>>>> still in idea stage.
>>>>>>>>>>> 
>>>>>>>>>>> Thanks All!
>>>>>>>>>>> Eric
>>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>> 
>>> 

Reply via email to