Hi Vijayanand,
Should case #2 render a failure if the named primary already has backup(s)? I
don't think overwrite (delete then recreate) is intuitive. I assume we support
DELETE.
Thanks,
-Hongfei
On 3/20/18, 1:29 AM, "Vijay Anand" <vijayanand.jayaman...@gmail.com> 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 <alfic...@gmail.com> 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 <rawlin.pet...@gmail.com>
> 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 <alfic...@gmail.com>
> 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 <alfic...@gmail.com>
> 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)
>
