~rousskov/squid/3p2-plus : changes

~rousskov/squid/3p2-plus » Changes from revision 10854

From Revision 10854 to 10835

Rev	Summary	Authors	Tags	Date
10854	Polished request reading code to fix CONNECT double-read ass... Polished request reading code to fix CONNECT double-read assertion comm.cc:216: "fd_table[fd].halfClosedReader != NULL" ConnStateData::flags.readMoreRequests, do_next_read variables, and ClientSocketContext::mayUseConnection() methods were used (or unused!) incorrectly or inconsistently. This change removes all do_next_read variables to simplify the state. Instead, the renamed ConnStateData::flags.readMore indicates whether client_side.cc should call comm_read. The mayUseConnection() methods are now used to indicate whether the next client-sent byte (buffered or read) should be reserved for the current request rather than being interpreted as the beginning of the next request. Usually, flags.readMore mayUseConnection regular requests: true false requests with bodies: true true errors: false false tunnels: false true	Alex Rousskov	take06	13 years ago
10853	Merged from parent (trunk r11349, circa 3.2.0.6+). Merged from parent (trunk r11349, circa 3.2.0.6+).	Alex Rousskov		13 years ago
10852	Fixed chunked request forwarding in ICAP presence. Fixed chunked request forwarding in ICAP presence. ICAP prohibits forwarding of hop-by-hop headers in HTTP headers. If the virgin request has a "Transfer-Encoding: chunked" header, the ICAP server will not receive it. Thus, when the ICAP server responds with a 200 OK and what it thinks is a copy of the HTTP request, the adapted request will be missing the Transfer-Encoding header. One the server side, Squid used to test whether the request had a Transfer-Encoding header to determine whether request chunking is needed when talking to the next HTTP hop. That test would fail in ICAP presence. This change implements a more direct/robust check: if we do not know the request content length, we chunk the request. We also no longer forward the Content-Length header if we are chunking. It should not really be there in most cases, but an explicit check is safer and may also prevent request smuggling attacks via Connection: Content-Length tricks.	Alex Rousskov		13 years ago
10851	Balance SMP workers load by raising one worker accept priori... Balance SMP workers load by raising one worker accept priority per second. SMP workers are known to consume significantly different CPU time when the load cannot keep all workers busy. For example, here are the total CPU time results (plus CPU core IDs) from a 1M-transaction test with seven workers: 6:32 2 squid 5:31 7 squid 4:03 5 squid 2:42 4 squid 1:06 6 squid 0:19 1 squid 0:11 3 squid The overall imbalance stems from the fact that most workers often have nothing to do but to wait for the next TCP connection. When the next connection arrives, the worker that can accept(2) the connection first, gets to service it. Since all workers are about the same, the first worker to wake up from epoll(2) waiting wins. The shared listening descriptor inside the OS kernel has a queue of waiting workers. When the new connection arrives, all waiting workers are awaken (via their epoll_wait(2) calls), in the kernel-determined order. Apparently, that order is LIFO: the last worker to be placed in the wait queue is the one to be awaken first. The new "load balancing" code that gets activated every second and changes the listening queue order. During N-th second, N-th worker becomes the first in the queue and will most likely get more work. With time, every worker gets similar number of chances to be the busiest and the idlest one, leveling the load: 3:15 2 squid 2:58 3 squid 2:43 6 squid 2:43 4 squid 2:34 1 squid 2:32 7 squid 2:17 5 squid This specific algorithm seems to do a somewhat better job than some simpler schemes we have tried (e.g., change workers order every time Squid accepts a connection), possibly because it does not reshuffle workers at semi-random times, which may lead to groups of busier and "idler" workers. When a worker has multiple listening ports, all ports are moved to the front of their respective queues. We have tried moving just one port at a time, but that more complex algorithm did not produce significantly better results in short tests. There are other, secondary factors such as CPU affinity and shared CPU core caches. We are ignoring them for now, but they probably contribute to the uneven CPU time distribution among workers even when this load balancing algorithm is in place. This change works in epoll-based environments only.	Alex Rousskov		13 years ago
10850	Break all detected forwarding loops. Break all detected forwarding loops. The old code tried to avoid breaking loops for non-intercepted/accelerated requests (nobody on squid-dev could explain why). Unfortunately, the "is this an intercepted request?" question cannot be reliably answered by examining http->flags. We would have to check the port settings because sometimes the port is intercepting, but we do not flip the corresponding flag in the request (I do not know why). If breaking all loops does not cause problems in valid setups, then it is a better option that trying to figure out exactly what loops are safe to break. This change will break setups that use non-unique proxy names in Via headers. On the other hand, such setups can be considered invalid anyway, and the issue should not be tied to whether this Squid uses interception or direct forwarding. In other words, they were just lucky to work before this change. See also: the "Breaking forwarding loops" thread on squid-dev.	Alex Rousskov		13 years ago
10849	Merged from parent (trunk r11345, circa 3.2.0.6+). Merged from parent (trunk r11345, circa 3.2.0.6+).	Alex Rousskov		13 years ago
10848	Merged from parent (trunk r11302, circa 3.2.0.5+). Merged from parent (trunk r11302, circa 3.2.0.5+).	Alex Rousskov		13 years ago
10847	Avoid comm_read "!fd_table[fd].closing()" assertion after ad... Avoid comm_read "!fd_table[fd].closing()" assertion after adaptation ACL check The assertion was hit if Server fd was closed while we were checking adaptation ACLs, and we have not been notified of the closure yet (because the Adaptation::AccessCheck callback is not async while closure notification is). Same as trunk r11080.	Alex Rousskov		13 years ago
10846	Author: Stefan Fritsch <sf@sfritsch.de> Author: Stefan Fritsch <sf@sfritsch.de> Bug 3096: Squid destroys CbDataList<DeferredRead> objects too late When server download speed exceeds client download speed, Squid creates a CbDataList<DeferredRead> object and associates a comm_close handler with it. When the server kicks the deferred read, the comm_close handler is canceled. This create/cancel sequence happens every time the server-side code wants to read but has to wait for the client, which may happen hundreds of times per second. Before this change, those canceled comm_close handlers were not removed from Comm until the end of the entire server transaction, possibly accumulating thousands of CbDataList<DeferredRead> objects tied to the socket descriptor via the canceled but still stored close handler. comm_remove_close_handler now immediately removes canceled close handlers to avoid their accumulation. same as trunk r11073	Alex Rousskov		13 years ago
10845	If a worker process crashes during shutdown, dump core and p... If a worker process crashes during shutdown, dump core and prevent restarts. Before the change, if a worker process crashes during shutdown, death() handler would exit with code 1, and master process would restart the worker. Now workers send SIGUSR1 to master when shutting down. When master process gets the SIGUSR1 signal, it stops restarting workers. SIGUSR1 is already used for log rotation, but it is fine to use SIGUSR1 for master process shutdown notifications because master is never responsible for both log rotation and kid restarts. Terminate with abort(3) instead of exit(3) to leave a core dump if Squid worker crashes during shutdown. Also the patch fixes potential infinite loop in master process. Master finished only when all kids exited with success, or all kids are hopeless, or all kids were killed by a signal. But in cases like when part of kids are hopeless and other were killed, master process would not exit. After the change master exits when there are no running kids and no kids should be restarted. Add syslog notice if kid becomes hopeless.	Alex Rousskov	take05	13 years ago
10844	Fixed "memPoolAlloc calls" Cache Manager stats reporting. Fixed "memPoolAlloc calls" Cache Manager stats reporting.	Alex Rousskov		13 years ago
10843	Made cache.log less noisy about usually benign events outsid... Made cache.log less noisy about usually benign events outside of admin control. Raised reporting level from 1 to 2 for 'statusIfComplete: Request not yet fully sent' and 'clientProcessRequest: Invalid Request' messages.	Alex Rousskov		13 years ago
10842	Bug 3111: assertion failed: forward.cc:145: "err" Bug 3111: assertion failed: forward.cc:145: "err" Do not assert when a close handler destroys FwdState before we received anything from the server. Create an ERR_READ_ERROR response instead. TODO: For the long-term fix of this and similar problems, the whole FwdState/Server relationship and socket management must be polished.	Alex Rousskov		13 years ago
10841	Reduce the number of server network reads for an average res... Reduce the number of server network reads for an average response. Make the initial server read buffer size 16KB instead of 2KB (4KB in v3.0). Limit the server read buffer size to 256KB instead of 2GB (SO_RCVBUF in v3.0). Trunk r9766 made the default HTTP server buffer size 2KB instead of 4KB, slowing down Squid when kernel network buffers are full and can sustain larger reads. We never grow that buffer size if the client is fast enough so the initial value stays through the connection lifetime, causing twice as many reads in v3.2 (compared to v3.0). This is a mid-term fix. Dependency on TCP socket buffer size and other tuning changes may be available later, after more testing.	Alex Rousskov		13 years ago
10840	Fixed DNS query leaks and increased defense against DNS cach... Fixed DNS query leaks and increased defense against DNS cache poisoning. We were leaking (i.e. forgetting about) DNS queries under several conditions. The most realistic leak case would go like this: - We send UDP query1. No response. - We send UDP query2. The response for query1 comes, with TC bit. - We try to connect over TCP, sending TCP query3. The response for query2 comes, with TC bit, matching TCP query3 ID. Since we are waiting a response over TCP, we drop the UDP response, and delete the query from the queue. We leak. This change avoids forgetting the query under the above scenario. Moreover, the above steps are hiding another problem: we are accepting responses to timed out queries, making DNS cache poisoning easier. This change avoids that by using unique query ID for each sent query. We have also added an instance ID so that we still can track/identify a single "transaction" from Squid point of view, even when that transaction involves many DNS query messages. When we forget about a DNS query, the caller may get stuck, holding a cbdata lock. This is typical for ACLs that require domain name resolution, for example. On a busy server with a long ACL list, the lock counter keeps growing due to forgotten requests and may overflow, causing a "c->locks < 65535" assertion. This change fixes the assertion unless there are more DNS leaks or different lock leaks present. Same as trunk r11015.	Alex Rousskov		13 years ago
10839	Prevent memory leaks when Adaptation::AccessCheck callback e... Prevent memory leaks when Adaptation::AccessCheck callback ends the job. The AccessCheckCallbackWrapper is used in nonBlockingCheck() and is called from the ACL code, using legacy function-based API. If the job ends during the callback processing, there are no AsyncCall wrappers to destroy the job object. We now convert legacy to async call to enable proper wrapping and job destruction. Same as trunk r11067.	Alex Rousskov		13 years ago
10838	Minimized post-merge differences with parent (trunk r10898). Minimized post-merge differences with parent (trunk r10898). No runtime changes expected.	Alex Rousskov		13 years ago
10837	Merged from parent (trunk r10999). Merged from parent (trunk r10999).	Alex Rousskov		13 years ago
10836	Removed force_http_1p1_reply squid.conf option. Removed force_http_1p1_reply squid.conf option. Squid already sends HTTP/1.1 in all responses so there is no reason to force that version.	Alex Rousskov		13 years ago
10835	Merged from parent (trunk r10988). Merged from parent (trunk r10988).	Alex Rousskov		13 years ago

Older »