RE: Memory usage fix (patch)
> -Original Message- > From: Henrik Nordstrom [mailto:[EMAIL PROTECTED] > Sent: Friday, March 18, 2005 4:56 AM > To: Steven Wilton > Cc: 'Squid Developers' > Subject: RE: Memory usage fix (patch) > > On Thu, 17 Mar 2005, Steven Wilton wrote: > > > The problem is that once squid starts hitting swap we start getting > > complaints. We have also noticed that certain clients have > an unusual usage > > pattern that seems to cause squid to ue lots of memory, > obviously bypassing > > the checks in fwdCheckDefer. I'll see if I can track this down. > > Either your use of the defer function is not working, or you > have clients > triggering the race condition I indicated yesterday when the original > client disconnects from the request. > You're correct, the defer function was not being used correctly. The problem was that enabling it caused a big increase in the CPU usage of squid. I think I've come up with an acceptable solution (CPU usage increases with network i/o). I'll update the epoll-2_5 tree with this change. -- No virus found in this outgoing message. Checked by AVG Anti-Virus. Version: 7.0.308 / Virus Database: 266.7.3 - Release Date: 3/15/2005
RE: Memory usage fix (patch)
On Thu, 17 Mar 2005, Steven Wilton wrote: The problem is that once squid starts hitting swap we start getting complaints. We have also noticed that certain clients have an unusual usage pattern that seems to cause squid to ue lots of memory, obviously bypassing the checks in fwdCheckDefer. I'll see if I can track this down. Either your use of the defer function is not working, or you have clients triggering the race condition I indicated yesterday when the original client disconnects from the request. Regards Henrik
RE: Memory usage fix (patch)
On Thu, 17 Mar 2005, Steven Wilton wrote: Actually.. How is the fwdCheckDefer function meant to slow this down? See the first part of comm_poll/select. A quick fix for epoll or other event driven loops is to defer calling the defer handler until activity is indicated on the fd. If the defer handler then indicates the connection should be paused then take the fd out of the read event handling and place it in a low priority queue processed before waiting for new events. Note: remember to remove the fd from this queue on fd_close. The proper way is to redesign defer handling to be a two-way communication, allowing the action which resolves the defer condition (i.e. store client reading data, delay pools refilling etc) to kick the connection alive again if possible, but this requires a somewhat bigger redesign of things. Regards Henrik
Re: Memory usage fix (patch)
On Thu, 17 Mar 2005, Adrian Chadd wrote: The defer handler is set when the FD is created and stays for the lifetime of the socket. Actually it is reset between requests, installed when the protocol is ready to start receiving the response. This because it needs to know the current request, but the general idea is the same. The defer handler is installed by commSetDefer() calls found in each protocol, and a quick grep for this function will tell you that there is three different defer handlers a) fwdCheckDeferRead, the main one for most forwarded requests b) clientReadDefer, to pause reading client request data when having to wait for the origin server first.. c) sslDeferServerRead, implementing delay pools on CONNECT requests. Regards Henrik
Re: Memory usage fix (patch)
On Thu, Mar 17, 2005, Steven Wilton wrote: > Actually.. How is the fwdCheckDefer function meant to slow this down? The > way I read the code is that it follows this logic: Each time through the select IO loop, the code will call commDeferRead() for each FD marked for read /before/ its added to the FD set passed to select() or poll(). This function pointer, which for http server FDs, points to fwdCheckDeferRead(). This returns "ok" if: * there's no mem object * some delay pools checking (where the return meaning is overloaded (-1) - check the code) * if its waiting for headers * if the gap between the highest offset read in the response and the lowest store client reader is smaller than READ_AHEAD_GAP Otherwise it will return 1 (defer). If defer is returned then the fd is NOT added to the fd set passwd to poll()/select(). > httpReadReply > - check aborted && return > - read data from socket > - if length > 0 and we have processed headers > - storeAppend > - switch httpPconnTransferDone (check whether the transfer is comlete) > - if transfer not complete, queue fd for read with callback to > httpReadReply > See, it happens /before/ this! :) The defer handler is set when the FD is created and stays for the lifetime of the socket. Adrian -- Adrian Chadd"To believe with certainty we must first <[EMAIL PROTECTED]> begin by doubting."
RE: Memory usage fix (patch)
> -Original Message- > From: Steven Wilton [mailto:[EMAIL PROTECTED] > Sent: Thursday, March 17, 2005 10:57 AM > > > -Original Message- > > From: Henrik Nordstrom > > Sent: Thursday, March 17, 2005 9:12 AM > > > > On Thu, 17 Mar 2005, Steven Wilton wrote: > > > > > The problem is that uncacheable objects (ie size > > > maximum_object_size, > > > or download managers doing multiple partial requests on > > large files) are > > > always held in memory. Squid does free this memory as the > > data is sent > > > to the client, but it doesn't look like there's a backoff > > mechanism when > > > the data is arriving at a much faster rate than it is being > > sent to the > > > client. > > > > Normally this is dealt with by the fwdCheckDefer function. > Maybe your > > epoll implementation does not use the filedescriptors defer > > function to > > back off when needed? > > I did make some changes to my original epoll patch, and the > epoll patch now > work with the commSetDefer() function Actually.. How is the fwdCheckDefer function meant to slow this down? The way I read the code is that it follows this logic: httpReadReply - check aborted && return - read data from socket - if length > 0 and we have processed headers - storeAppend - switch httpPconnTransferDone (check whether the transfer is comlete) - if transfer not complete, queue fd for read with callback to httpReadReply So, if the headers have been procesed, and the read call returns data, we queue another read without checking whether to defer. The patch that I submitted checks that a swap object is not first. If entry->mem_obj->swapout.sio is not set, is it possible for another request to be fetching from this object? I was pretty sure that I had tested this case, and found that without a sio, there was no reference for squid to generate a cache hit. (It was 7 months ago, and I could be mistaken.) Having said this, the patch I originally posted did not seem to work well with my testing (100% cpu usage with epoll). I am going to do further work on it. Steven -- No virus found in this outgoing message. Checked by AVG Anti-Virus. Version: 7.0.308 / Virus Database: 266.7.3 - Release Date: 3/15/2005
RE: Memory usage fix (patch)
> -Original Message- > From: Henrik Nordstrom > Sent: Thursday, March 17, 2005 9:12 AM > > On Thu, 17 Mar 2005, Steven Wilton wrote: > > > The problem is that uncacheable objects (ie size > > maximum_object_size, > > or download managers doing multiple partial requests on > large files) are > > always held in memory. Squid does free this memory as the > data is sent > > to the client, but it doesn't look like there's a backoff > mechanism when > > the data is arriving at a much faster rate than it is being > sent to the > > client. > > Normally this is dealt with by the fwdCheckDefer function. Maybe your > epoll implementation does not use the filedescriptors defer > function to > back off when needed? I did make some changes to my original epoll patch, and the epoll patch now work with the commSetDefer() function. I had this new version running overnight, and within a couple of hours squid had grown to double it's normal size, and we were getting negative hit rates on bytes. This was caused by the fwdCheckDefer function not stopping the download as expected. The squid has been patched with the lfs patch (downloaded from http://devel.squid-cache.org/cgi-bin/diff2/lfs-2.5.patch?s2_5 yesterday), and a couple of the 2.5.STABLE9 patches from http://www.squid-cache.org/Versions/v2/2.5/bugs/. The problem is that once squid starts hitting swap we start getting complaints. We have also noticed that certain clients have an unusual usage pattern that seems to cause squid to ue lots of memory, obviously bypassing the checks in fwdCheckDefer. I'll see if I can track this down. -- No virus found in this outgoing message. Checked by AVG Anti-Virus. Version: 7.0.308 / Virus Database: 266.7.3 - Release Date: 3/15/2005
Re: Memory usage fix (patch)
On Thu, 17 Mar 2005, Adrian Chadd wrote: Just as a FWIW, this is something thats been discussed to death a couple of years ago. Yup, event driven IO doesn't play well with the current defer processing of IO. Indeed. There needs to be a soft event triggering the fd alive again, driven by the consumers. Should be trivial to add ontop of the changes already done in the lfs branch.. There is already a notifier going from the store clients to the store to clean up the cache vm. Just need to kill some of the optimizations. it would. I'd like to finally get back into this now - doubly so if there's someone else here who is interested in fixing the network IO code path in squid-2.5. It certainly would be good to have it fixed in the epoll branch.. But we should first fix it in Squid-3 I think.. Note: The lfs branch is out of necessity in core functionality as multi-GB files is starting to become increasingly common on FTP servers etc, not performance. I had to touch this area due to race windows opened by the large object changes while fixing and old bug involving joined requests where Squid would to try to allocate virtually unlimited amount of space in the on-disk cache. Without the large object changes this bug was not so bad as it was limited to allocate at most 2GB per object, but this is no longer true... Regards Henrik
Re: Memory usage fix (patch)
On Thu, Mar 17, 2005, Henrik Nordstrom wrote: > On Thu, 17 Mar 2005, Steven Wilton wrote: > > >The problem is that uncacheable objects (ie size > maximum_object_size, > >or download managers doing multiple partial requests on large files) are > >always held in memory. Squid does free this memory as the data is sent > >to the client, but it doesn't look like there's a backoff mechanism when > >the data is arriving at a much faster rate than it is being sent to the > >client. > > Normally this is dealt with by the fwdCheckDefer function. Maybe your > epoll implementation does not use the filedescriptors defer function to > back off when needed? Just as a FWIW, this is something thats been discussed to death a couple of years ago. Yup, event driven IO doesn't play well with the current defer processing of IO. > Other issues is that memory is only freed on swapout, which results in > that for async cache_dirs many cached object will end up using more memory > than intended. > > I have a partial fix for this in my lfs branch, and quite a bit of fixes > in how the cache vm is managed. However, I now discovered a race window > similar to the above which would still make Squid loose the bandwidth cap > while the object is cached. Fixing this should not be too hard. *grin* > The design of this area in the code has quite a bit of potential for > improvement... it would. I'd like to finally get back into this now - doubly so if there's someone else here who is interested in fixing the network IO code path in squid-2.5. adrian -- Adrian Chadd"To believe with certainty we must first <[EMAIL PROTECTED]> begin by doubting."
Re: Memory usage fix (patch)
On Thu, 17 Mar 2005, Steven Wilton wrote: The problem is that uncacheable objects (ie size > maximum_object_size, or download managers doing multiple partial requests on large files) are always held in memory. Squid does free this memory as the data is sent to the client, but it doesn't look like there's a backoff mechanism when the data is arriving at a much faster rate than it is being sent to the client. Normally this is dealt with by the fwdCheckDefer function. Maybe your epoll implementation does not use the filedescriptors defer function to back off when needed? However, there is a race in the logics used by this function if there is multiple clients accessing the same cacheable object and the initial client disconnects. If this happens then the cap of the connection ban be lost in certain situations. But memory should still be freed. Other issues is that memory is only freed on swapout, which results in that for async cache_dirs many cached object will end up using more memory than intended. I have a partial fix for this in my lfs branch, and quite a bit of fixes in how the cache vm is managed. However, I now discovered a race window similar to the above which would still make Squid loose the bandwidth cap while the object is cached. Fixing this should not be too hard. The design of this area in the code has quite a bit of potential for improvement... Regards Henrik
