Moving a conversation started in Slack to the mailing list: VijayAnand [8:06 AM] Hi
This is regarding TR's Backup Cache Group Selection which is https://github.com/apache/incubator-trafficcontrol/issues/1907 GitHub Deterministic Cachegroup failover · Issue #1907 · apache/incubator-trafficcontrol Currently, if all caches in a cache group are unavailable Traffic Router will route clients to the next closest cache group. This works great for some networks but could cause problems in others. T... Based on Rawlin's review comments on the PR https://github.com/apache/incubator-trafficcontrol/pull/1908 GitHub Changes for Backup Edge Cache Group by Vijay-1 · Pull Request #1908 · apache/incubator-trafficcontrol This PR implements solution for the issue: #1907 It places the backup policy in the CZF file { "coverageZones": { "GROUP2": { "backupList": ["GROUP1"], &quo... I have the following Schema to support this which is in-sync with Rawlin's comments CREATE TABLE cachegroup_fallbacks ( primary_cg bigint, backup_cg bigint CHECK (primary_cg != backup_cg), set_order bigint DEFAULT 0, CONSTRAINT fk_primary_cg FOREIGN KEY (primary_cg) REFERENCES cachegroup(id) ON DELETE CASCADE, CONSTRAINT fk_backup_cg FOREIGN KEY (backup_cg) REFERENCES cachegroup(id) ON DELETE CASCADE, UNIQUE (primary_cg, backup_cg), UNIQUE (primary_cg, set_order) ); ALTER TABLE cachegroup ADD COLUMN fallback_to_closest BOOLEAN DEFAULT TRUE; Would like to get your views before i start coding for the same Eric Friedrich [8:15 AM] why does the set_order get a default? VijayAnand [8:17 AM] aah. that is not needed assuming there is a valid input for order Rawlin Peters [9:15 AM] yeah a null value there could be interpreted as zero in the API, but maybe we should add a NOT NULL constraint to the columns as well? I can't think of any reasons why any column should be optional Eric Friedrich [9:18 AM] any preference in the API between ```{"list": ["grp1", "grp2"]}``` vs ```{"list": [{"name": "grp1", "order": 1}, {"name": "grp3", "order: 2}, // or 5 or some other positive integer ] }``` Rawlin Peters [9:36 AM] 2nd option would make it easier to extend in the future with weighting for instance, we could just add another key/value in that object Eric Friedrich [9:37 AM] good point- i hadn’t thought about that Chris Lemmons [1:16 PM] The second is way harder to parse and handle. I strongly prefer the first. Is there a reason to expect such extension? (edited) Jeff Elsloo [1:30 PM] presumably we’re using a library to handle it, so why is it way harder? Dan Kirkwood [1:33 PM] +1 -- it's all just JSON.. Rob Butts [1:39 PM] I don’t agree that we shouldn’t care about the human-readability, because we have libraries. Someone might want to write an app in a different language. I wouldn’t say “way” harder, but the second definitely is harder to read, and unintuitive. I’m +1 on making our APIs easy to read and work with natively :confused: Jeff Elsloo [1:41 PM] but a list is just a container it contains “things” in this case it’s a list of objects with properties Dylan Volz [1:41 PM] agree we shouldn't not care, but most of our apis are lists of json objects, and if we need to add a key the the second definition is far more easily extensible Chris Lemmons [1:42 PM] True. Are they actually objects with properties? Or are they string values? Jeff Elsloo [1:42 PM] that’s implied by the structure Chris Lemmons [1:42 PM] Either way, I'd prefer not to overload the order as a parameter, unless there is a particular reason? Jeff Elsloo [1:43 PM] `{"name": "grp3", "order: 2}` is not a string in JSON Chris Lemmons [1:43 PM] Aye. Jeff Elsloo [1:43 PM] I don’t follow what you mean about order Chris Lemmons [1:43 PM] Are there other parameters (or might there reasonably be in the future)? Jeff Elsloo [1:43 PM] order is just a property and has nothing to do with overloading anything in this context, if it has to do with Rawlin’s steering work, yes there certainly could be other properties which is why the format was suggested Chris Lemmons [1:45 PM] So, for example, using objects: I prefer `[{"name": "grp1"}, {"name": "grp3"}]` to `[{"name": "grp1", "order": 1},{"name": "grp3", "order: 2}]`. In both cases, you can easily add a new parameter, for example, if groups had an owner. But arrays already have orders. Jeff Elsloo [1:45 PM] no not in JSON Chris Lemmons [1:46 PM] Yes, they do. Jeff Elsloo [1:46 PM] I’m not going to implicitly trust the order of what’s in the source JSON we don’t have the ability to control that Chris Lemmons [1:46 PM] Arrays in JSON are ordered. Objects in JSON are not ordered. Jeff Elsloo [1:46 PM] no they aren’t not unless they’re systematically created that way which in this case they are not we have a property called order and a value that could appear in any order of the underlying array we would have to do a lot more work to build it properly and then just make the assumption in downstream components that the order is implicitly correct in JSON that is a risky assumption Chris Lemmons [1:47 PM] https://tools.ietf.org/html/rfc7159 > An array is an ordered sequence of zero or more values. Jeff Elsloo [1:47 PM] seriously Chris? do you even know why or how such things are used in Traffic Ops? did you know that the orders could be negative? I’m not arguing about whether or not JSON supports order in its arrays I’m arguing about how the data is put into JSON and how users configure the properties and what the values can be Chris Lemmons [1:49 PM] > do you even know why or how such things are used in Traffic Ops? Not in detail, no. Hence my question: > unless there is a particular reason? Jeff Elsloo [1:49 PM] having to order that in the JSON adds another layer of complexity that is unnecessary I’ve given you some but you dropped an RFC on me in reply which is implicitly saying you don’t agree and won’t so for cache group ordering, which is separate from the steering ordering issue, I think that stating order specifically will give us more options down the road just as it did with steering we have multiple ways to order things in the steering case, and we’ve extended that, so I don’t see why that precedent wouldn’t also apply here backup cache group ordering that is Chris Lemmons [1:55 PM] Ah... I think there was a bit of a miscommunication there. To the point of the discussion, though... I missed which data this is storing. Yeah, we have to store the ordering data explicitly in the database, since rows are unordered. Jeff Elsloo [1:55 PM] yes, that’s what I was getting at.. so we’d have to pull the values out of the database, sort them properly, then ensure they are written that way to the snapshot table Rawlin Peters [1:55 PM] fwiw the particular JSON in question isn't for the steering stuff I'm working on, it's for the Cachegroup Failover stuff (i.e. CG1 specifically fails over to CG2, CG3, in that order) Jeff Elsloo [1:55 PM] it’s easier to just have the downstream consumer worry about that, as they likely would need to worry about it anyway yeah Rawlin, I saw that eventually :slightly_smiling_face: Rawlin Peters [1:56 PM] sorry just catching up too Jeff Elsloo [1:56 PM] but to me it’s a similar concept as steering we have an ordering mechanism based on an int value today maybe that changes in the future as things in our world tend to I agree with keeping the content the API spits out as simple as possible, but not to the extent that it makes us have to do more work just for sake of readability Chris Lemmons [1:58 PM] So, what happens if ordering is equivalent? Rawlin Peters [1:58 PM] yeah plus we need a way to update the order of a particular relation, which is why we can't just keep it as a simple list of strings there's a uniqueness constraint on (primary_cg, order), so a particular cachegroup can only declare one backup CG at that particular ordering Chris Lemmons [2:01 PM] Ok. So if you want to change ordering other than "move to end" or "move to beginning", you'll probably need to update half of the relations? Dave Neuman [2:01 PM] All of this ^^ should be done on the mailing list (edited) any of the decisions made here don't count unless you want to come to some consensus here, put it on the mailing list, and have everyone +1 which is against the spirit of the mailing list (edited)
