[squid-dev] Care and feeding of ConnStateData

[Christos Tsantilas]

On 07/07/2016 10:22 PM, Alex Rousskov wrote:
> On 07/06/2016 10:52 PM, Amos Jeffries wrote:
>> On 7/07/2016 10:24 a.m., Alex Rousskov wrote:
>>> Q2. Where does the pending Downloader class belong?
>
>> In overview I think if we have a good Downloader design those other
>> things and ESIInclude should probably be converted into Downloader or
>> children of it.
>
> Yes, but if and only if ESI needs to download small SBuf-storable
> things. If ESI needs potentially large HTTP response bodies, then ESI
> cannot be refactored to use Downloader.


My sense is that the ESI uses simple buffers to store data. However not 
sure, we need to review the esi code.  But the Downloader as is now is a 
small class, can be extended in the future to handle large response 
bodies.

If I am not wrong we fixed recently a bug in esi which was related. 
Maybe there are still others bugs in esi too. Using a class like the 
Downloader, which used by other subsystems where there is active 
development and higher users base, may eliminate this type of bugs.


On 07/08/2016 01:16 AM, Amos Jeffries wrote:
> On 8/07/2016 7:22 a.m., Alex Rousskov wrote:
>>    2. Move that remaining generic code into src/servers/Server and src/http/.
>>
>> We all have been doing step #1 since then, but we are not yet ready for
>> step #2 because ConnStateData still has HTTP server-specific code in it.
>
> Apart from auditing all I'm seeing from Factory is more and more
> non-Server things being added to ConnStateData. First it was 1xx message
> handling, then ssl-bump, then all the FTP translation logics, then
> Downloader (thanks for reversing on that one).
The 1xx message handling is used by FTP code too.

The ssl-bump my opinion is that it is normal to be handled by 
ConnStateData because it is near the connection level, not near the 
protocol level. We may want to split ConnStateData to a new 
TlsConnStateData class.

There is not a lot of Ftp related code inside ConnStateData. In the 
future we can completely remove the remaining "if (HTTP protocol)" or 
"if (FTP protocol)" which are not many.

Also we may want to move the ProxyProtocol related code under a new 
class inside servers/ProxyProtocol.[cc,h].