[PROPOSAL] Change *attempts_timeout to be in milliseconds

[ev...@gmail.com, ev...@gmail.com]

We started seeing some problems with some of our origins/parents and we believe that if we had finer control over the attempts timeout it might help alleviate some problems. Currently all of these timeouts are in seconds, and when it comes to live streaming with 2sec fragments that granularity may not be enough.

So proposing that we move these to millisecond based. I had discussed on IRC some other options such as switching to float, but that can bring in precision issues, and possibly adding units values but that would be a first for a record value and would seem to be more of a wide change to bring in units parsing where it has not been used before. So just changing to ms seemed the most straight forward.

Re: [PROPOSAL] Change *attempts_timeout to be in milliseconds

[Alan Carroll <so...@oath.com.INVALID>]

You don't have to do that - this is the point of the "source" value for
configuration variables. That enables the detection of whether a value was
actually in "records.config" or not. For instance, when I back ported the
recent HttpConnectionCount work to the Oath fork, it needed to be backwards
compatible in a way the changes on 9.0 didn't, so I added code like this -

  // Load 'em up by firing off the config update callback.
  // Special handling to maintain BC - only fire update if there is a
non-default value.
  RecSourceT source;
  if (REC_ERR_OKAY == RecGetRecordSource(BC_CONFIG_VAR_MAX.data(), &source,
true) && REC_SOURCE_DEFAULT != source) {
    RecLookupRecord(BC_CONFIG_VAR_MAX.data(), &Load_Config_Var, nullptr,
true);
  }
  if (REC_ERR_OKAY == RecGetRecordSource(CONFIG_VAR_MAX.data(), &source,
true) && REC_SOURCE_DEFAULT != source) {
    RecLookupRecord(CONFIG_VAR_MAX.data(), &Load_Config_Var, nullptr, true);
  }

This checks the new config var and if it's set by the admin, does that,
otherwise if the old one has been set, use that, otherwise use the new
defaults. REC_SOURCE_DEFAULT means the built in default.


On Fri, Aug 17, 2018 at 9:42 PM Zelkowitz, Evan <Ev...@comcast.com>
wrote:

> I just figured that you would never want an explicit zero _ms timeout, if
> you did well you could already do that with the existing one based in
> seconds. So the only time you want _ms would be non-zero so then it should
> override the non-_ms
> ________________________________________
> From: Leif Hedstrom <zw...@apache.org>
> Sent: Friday, August 17, 2018 5:59 PM
> To: dev@trafficserver.apache.org
> Subject: [EXTERNAL] Re: [PROPOSAL] Change *attempts_timeout to be in
> milliseconds
>
> Meh, I see the PR with treating non zero values taking precedence. That
> works, with the somewhat odd behavior that an explicit zero cannot override
> the old value . I think that’s fine though.
>
> I agree with amc that for 9.0.0 (current master) we should cleanup this
> consistently, floats seems fine.
>
> — Leif
>
> > On Aug 17, 2018, at 15:46, Leif Hedstrom <zw...@apache.org> wrote:
> >
> > As suggested elsewhere, having some new configs might be the safest
> transitions. We can then deprecate, and later remove, the 1s ones.
> >
> > They would have to take precedence, but that can be tricky to deal with
> as a undefined value :-/. Maybe the smallest non-zero value should be used ?
> >
> > — Leif
> >
> >> On Aug 15, 2018, at 08:00, "evan.zelkowitz@gmail.com" <
> evan.zelkowitz@gmail.com> wrote:
> >>
> >> We started seeing some problems with some of our origins/parents and we
> believe that if we had finer control over the attempts timeout it might
> help alleviate some problems. Currently all of these timeouts are in
> seconds, and when it comes to live streaming with 2sec fragments that
> granularity may not be enough.
> >>
> >> So proposing that we move these to millisecond based. I had discussed
> on IRC some other options such as switching to float, but that can bring in
> precision issues, and possibly adding units values but that would be a
> first for a record value and would seem to be more of a wide change to
> bring in units parsing where it has not been used before. So just changing
> to ms seemed the most straight forward.
> >
>


-- 
*Beware the fisherman who's casting out his line in to a dried up riverbed.*
*Oh don't try to tell him 'cause he won't believe. Throw some bread to the
ducks instead.*
*It's easier that way. *- Genesis : Duke : VI 25-28

Re: [PROPOSAL] Change *attempts_timeout to be in milliseconds

["Zelkowitz, Evan" <Ev...@comcast.com>]

I just figured that you would never want an explicit zero _ms timeout, if you did well you could already do that with the existing one based in seconds. So the only time you want _ms would be non-zero so then it should override the non-_ms
________________________________________
From: Leif Hedstrom <zw...@apache.org>
Sent: Friday, August 17, 2018 5:59 PM
To: dev@trafficserver.apache.org
Subject: [EXTERNAL] Re: [PROPOSAL] Change *attempts_timeout to be in milliseconds

Meh, I see the PR with treating non zero values taking precedence. That works, with the somewhat odd behavior that an explicit zero cannot override the old value . I think that’s fine though.

I agree with amc that for 9.0.0 (current master) we should cleanup this consistently, floats seems fine.

— Leif

> On Aug 17, 2018, at 15:46, Leif Hedstrom <zw...@apache.org> wrote:
>
> As suggested elsewhere, having some new configs might be the safest transitions. We can then deprecate, and later remove, the 1s ones.
>
> They would have to take precedence, but that can be tricky to deal with as a undefined value :-/. Maybe the smallest non-zero value should be used ?
>
> — Leif
>
>> On Aug 15, 2018, at 08:00, "evan.zelkowitz@gmail.com" <ev...@gmail.com> wrote:
>>
>> We started seeing some problems with some of our origins/parents and we believe that if we had finer control over the attempts timeout it might help alleviate some problems. Currently all of these timeouts are in seconds, and when it comes to live streaming with 2sec fragments that granularity may not be enough.
>>
>> So proposing that we move these to millisecond based. I had discussed on IRC some other options such as switching to float, but that can bring in precision issues, and possibly adding units values but that would be a first for a record value and would seem to be more of a wide change to bring in units parsing where it has not been used before. So just changing to ms seemed the most straight forward.
>

Re: [PROPOSAL] Change *attempts_timeout to be in milliseconds

[Leif Hedstrom <zw...@apache.org>]

Meh, I see the PR with treating non zero values taking precedence. That works, with the somewhat odd behavior that an explicit zero cannot override the old value . I think that’s fine though.

I agree with amc that for 9.0.0 (current master) we should cleanup this consistently, floats seems fine.

— Leif 

> On Aug 17, 2018, at 15:46, Leif Hedstrom <zw...@apache.org> wrote:
> 
> As suggested elsewhere, having some new configs might be the safest transitions. We can then deprecate, and later remove, the 1s ones.
> 
> They would have to take precedence, but that can be tricky to deal with as a undefined value :-/. Maybe the smallest non-zero value should be used ?
> 
> — Leif 
> 
>> On Aug 15, 2018, at 08:00, "evan.zelkowitz@gmail.com" <ev...@gmail.com> wrote:
>> 
>> We started seeing some problems with some of our origins/parents and we believe that if we had finer control over the attempts timeout it might help alleviate some problems. Currently all of these timeouts are in seconds, and when it comes to live streaming with 2sec fragments that granularity may not be enough.
>> 
>> So proposing that we move these to millisecond based. I had discussed on IRC some other options such as switching to float, but that can bring in precision issues, and possibly adding units values but that would be a first for a record value and would seem to be more of a wide change to bring in units parsing where it has not been used before. So just changing to ms seemed the most straight forward.
> 

Re: [PROPOSAL] Change *attempts_timeout to be in milliseconds

[Leif Hedstrom <zw...@apache.org>]

As suggested elsewhere, having some new configs might be the safest transitions. We can then deprecate, and later remove, the 1s ones.

They would have to take precedence, but that can be tricky to deal with as a undefined value :-/. Maybe the smallest non-zero value should be used ?

— Leif 

> On Aug 15, 2018, at 08:00, "evan.zelkowitz@gmail.com" <ev...@gmail.com> wrote:
> 
> We started seeing some problems with some of our origins/parents and we believe that if we had finer control over the attempts timeout it might help alleviate some problems. Currently all of these timeouts are in seconds, and when it comes to live streaming with 2sec fragments that granularity may not be enough.
> 
> So proposing that we move these to millisecond based. I had discussed on IRC some other options such as switching to float, but that can bring in precision issues, and possibly adding units values but that would be a first for a record value and would seem to be more of a wide change to bring in units parsing where it has not been used before. So just changing to ms seemed the most straight forward.