I think we should do a few things: 1. POST: receive a full list of ordered backups and create the fallback list. If the fallback list already exists, render a failure. 2. PUT: receive a full list of ordered backups, delete the existing fallback list, and create a new fallback list. If the fallback list does not exist yet, render a failure. 3. DELETE: delete the existing fallback list for a cachegroup. If the fallback list does not exist, render a failure. 4. GET: return the existing fallback list for a cachegroup. If the fallback list does not exist, render a failure.
Then the client behavior would be: GET the current fallbacks for a cachegroup. If the list exists, make updates and PUT the entire updated list. If the list does not exist, create a list and POST the entire list. To empty the list, use the DELETE endpoint (or do a PUT with an empty list). Does that sound good? Right now it doesn't make sense for a client to do a PUT for just a single fallback entry, unless we add some sort of insert/append/prepend PUT APIs which would seem superfluous. - Rawlin On Mon, Mar 19, 2018 at 11:29 PM, Vijay Anand <[email protected]> wrote: > Shall i take this format :[{"name": "grp1", "order": 1},{"name": "grp3", > "order: 2}] > > @Rawlin >>Perhaps the POST/PUT endpoints are basically the same and always take the > full list of backups. > > Wanted to make sure the behavior of PUT/ POST endpoints; POST will delete > an existing set of backups and insert the new ones. While PUT will update > them. > > For example, I have grp2(1) and grp3(2) as backups for grp1: > > Case #1: I get a PUT with grp2 (3), I will update grp2 with new order 3. > grp1 still has two backup cache groups. > > Case #2: I get a POST with gr2(3), I will delete all the existing entries > and insert grp2 making grp2 as the only backup for grp1 with order 3. > > Isnt that right? > > -Vijayanand S > > On Tue, Mar 20, 2018 at 8:06 AM, Chris Lemmons <[email protected]> wrote: > >> You can keep each object without the order parameter: >> >> [{"name": "grp1"}, {"name":"grp3"}] >> >> Nevertheless, it sounds like I'm a minority opinion on this one. It's >> not that important of an issue, I think. Either way will work. >> >> On Mon, Mar 19, 2018 at 5:50 PM, Rawlin Peters <[email protected]> >> wrote: >> > Yeah, maybe it doesn't make sense to update just one backup entry at a >> > time because you'd have to update the order of the rest of the backups >> > as well. Perhaps the POST/PUT endpoints are basically the same and >> > always take the full list of backups. But I think we should still keep >> > each entry as an object with an explicit "order" so that we can extend >> > it more easily in the future. I could easily see us adding a "weight" >> > to each entry, so that a particular cachegroup could split the >> > fallback traffic between multiple cachegroups at a time rather than >> > falling back through them sequentially. >> > >> > - Rawlin >> > >> > On Mon, Mar 19, 2018 at 2:30 PM, Chris Lemmons <[email protected]> >> wrote: >> >> So, to continue the conversation, it looks like the list of backup >> >> groups is stored as a List<String>. It's currently loaded by iterating >> >> the elements of the json array in order. That looks great to me. >> >> >> >> It seems odd to me to have a separate order parameter in the API. >> >> Since order has to be unique, it's unlikely that you'd be able to >> >> update the order component to do anything other than "move to an end" >> >> without updating about half the rows in the db anyway. It just feels >> >> like we're asking more of the API user. >> >> >> >> On Mon, Mar 19, 2018 at 2:04 PM, Chris Lemmons <[email protected]> >> wrote: >> >>> 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) >>
