Wednesday, June 12, 2013, 3:29:17 PM, you wrote:

>> +      else if (!strncmp(name, "proxy.config.http.flow_control.enabled", 
>> length))
>> +        cnf = TS_CONFIG_HTTP_FLOW_CONTROL_ENABLED;
>> +      break;

> Not a big deal, but I've seen some reason efforts to use {} consistently, 
> and I kinda like that (I'm as bad as anyone leaving them out though).

That was the style in that section of code (see 'case 37' for example). 

> I know I keep repeating myself, but lets stop formatting prototypes like 
> this. Put the comment in the section above, and put everything on one line 
> (up until 120 character limit, which is our standard).

Allright.

>> +  params->oride.flow_high_water_mark = m_master.oride.flow_high_water_mark;
>> +  params->oride.flow_low_water_mark = m_master.oride.flow_low_water_mark;
>> +  // If not set (zero) then make values the same.
>> +  if (params->oride.flow_low_water_mark <= 0)
>> +    params->oride.flow_low_water_mark = params->oride.flow_high_water_mark;
>> +  if (params->oride.flow_high_water_mark <= 0)
>> +    params->oride.flow_high_water_mark = params->oride.flow_low_water_mark;

> This seems a little odd. The default is "0", right? Which means, you first 
> assign high and low to 0 from the m_master, and then you do it again but 
> copying from the opposite oride config.

No, it's to make the two values the same if only one is set. The logic to check 
that before copying over is more complex than to copy first, and clean up if 
one of them is still the default.


> One further comment: I like the idea of records.config, and overridable 
> configs. However, for this feature, does it makes sense to allow a 
> transaction to change the low/high watermarks "mid transaction" ? What if I 
> want to start with 10Mbps, but after 10s, set it down to 1Mbps based on what 
> the content is ? That seems like a very common use case for e.g. streaming 
> content. E.g. You start with a quick burst, then switch to a throttling 
> that's suitable for the bit-rate of the media.

> As such, would it make sense to also have regular APIs that can be called in 
> here? Or is the idea that you'd just change the records.config settings 
> during the transaction? I think the latter would work too, right ?

Possibly. I was thinking primarily of making the decision during transaction 
setup. E.g., I want to throttle less for server A than server server B, so 
you'd tweak it in READ_REQUEST_HEADER or READ_RESPONSE_HEADER.

I would note, though, that this doesn't do any rate based shaping, it tracks 
only the amount of data in the transaction inside ATS. So there is no way to 
throttle the throughput to less than the external client is consuming.

Reply via email to