You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficcontrol.apache.org by Chris Lemmons <al...@gmail.com> on 2018/03/19 20:04:23 UTC
Backup Cache Group Selection API Format
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)
Re: Backup Cache Group Selection API Format
Posted by Vijay Anand <vi...@gmail.com>.
Here is the PR for the initial version of the code changes discussed for
https://github.com/apache/incubator-trafficcontrol/issues/1907
https://github.com/apache/incubator-trafficcontrol/pull/2029
Did minimal testing. Hosting it before testing it fully to get early review
comments.
Thanks,
Vijayanand S
On Tue, Mar 20, 2018 at 9:51 PM, Rawlin Peters <ra...@gmail.com>
wrote:
> 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
> <vi...@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 <al...@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.peters@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 <al...@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 <al...@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)
> >>
>
Re: Backup Cache Group Selection API Format
Posted by Rawlin Peters <ra...@gmail.com>.
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
<vi...@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 <al...@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 <ra...@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 <al...@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 <al...@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)
>>
Re: Backup Cache Group Selection API Format
Posted by "Hongfei Zhang (hongfezh)" <ho...@cisco.com>.
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" <vi...@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 <al...@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 <ra...@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 <al...@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 <al...@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)
>
Re: Backup Cache Group Selection API Format
Posted by Vijay Anand <vi...@gmail.com>.
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 <al...@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 <ra...@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 <al...@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 <al...@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)
>
Re: Backup Cache Group Selection API Format
Posted by Chris Lemmons <al...@gmail.com>.
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 <ra...@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 <al...@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 <al...@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)
Re: Backup Cache Group Selection API Format
Posted by Rawlin Peters <ra...@gmail.com>.
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 <al...@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 <al...@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)
Re: Backup Cache Group Selection API Format
Posted by Chris Lemmons <al...@gmail.com>.
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 <al...@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)