changeset 24:2ee28064a04a 0.4

Keepalive: correctly close connections on gracefull shutdown. On gracefull shutdown nginx calls read handler on all idle connections with c->close set. Make sure we don't confuse such read handler calls with stale events and actually close connections. This fixes "open socket ... left in connection ..." alerts.
author Maxim Dounin <mdounin@mdounin.ru>
date Wed, 15 Dec 2010 21:12:36 +0300
parents cad8bc39d98d
children c6396fef9295
files CHANGES ngx_http_upstream_keepalive_module.c t/memcached-keepalive.t t/stale.t
diffstat 4 files changed, 21 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,10 @@
+
+Changes with upstream keepalive module 0.4 (2010-12-15):
+
+    *) Bugfix: the "[alert] ... open socket ... left in connection ..."
+       messages were logged on nginx worker process gracefull exit for
+       each cached connection; the bug had appeared in 0.3.
+
 
 Changes with upstream keepalive module 0.3 (2010-09-14):
 
--- a/ngx_http_upstream_keepalive_module.c
+++ b/ngx_http_upstream_keepalive_module.c
@@ -395,6 +395,10 @@ ngx_http_upstream_keepalive_close_handle
 
     c = ev->data;
 
+    if (c->close) {
+        goto close;
+    }
+
     n = recv(c->fd, buf, 1, MSG_PEEK);
 
     if (n == -1 && ngx_socket_errno == NGX_EAGAIN) {
--- a/t/memcached-keepalive.t
+++ b/t/memcached-keepalive.t
@@ -20,12 +20,11 @@ select STDOUT; $| = 1;
 eval { require Cache::Memcached; };
 plan(skip_all => 'Cache::Memcached not installed') if $@;
 
-my $t = Test::Nginx->new()->has('rewrite')->has_daemon('memcached')->plan(16)
+my $t = Test::Nginx->new()->has('rewrite')->has_daemon('memcached')->plan(17)
 	->write_file_expand('nginx.conf', <<'EOF');
 
 %%TEST_GLOBALS%%
 
-master_process off;
 daemon         off;
 
 events {
@@ -202,4 +201,8 @@ is($memd1->stats()->{total}->{total_conn
 	$memd2->stats()->{total}->{total_connections}, $total + 2,
 	'connection per backend');
 
+$t->stop();
+
+like(`grep -F '[alert]' ${\($t->testdir())}/error.log`, qr/^$/s, 'no alerts');
+
 ###############################################################################
--- a/t/stale.t
+++ b/t/stale.t
@@ -20,7 +20,7 @@ select STDOUT; $| = 1;
 eval { require Cache::Memcached; };
 plan(skip_all => 'Cache::Memcached not installed') if $@;
 
-my $t = Test::Nginx->new()->has('rewrite')->has_daemon('memcached')->plan(1)
+my $t = Test::Nginx->new()->has('rewrite')->has_daemon('memcached')->plan(2)
 	->write_file_expand('nginx.conf', <<'EOF');
 
 %%TEST_GLOBALS%%
@@ -107,4 +107,8 @@ for (1 .. 100) {
 cmp_ok($memd1->stats()->{total}->{total_connections}, '<=', $total + 2,
 	'only one connection per worker used');
 
+$t->stop();
+
+like(`grep -F '[alert]' ${\($t->testdir())}/error.log`, qr/^$/s, 'no alerts');
+
 ###############################################################################