Changeset ca37e8

Timestamp:

01/16/09 19:58:58 (3 years ago)

Author:

George Potapov <nephrite@…>

Branches:

master-v0.7, upstream_count_limit

Children:

cd26d0

Parents:

3f2f24

git-author:

George Potapov <nephrite@…> (01/16/09 19:58:58)

git-committer:

George Potapov <nephrite@…> (01/16/09 19:58:58)

Message:

updated to latest upstream_keepalive (ngx_http_upstream_keepalive-067ddc059ee0.tar.gz) from Maxim Dounin

Location:

upstream_keepalive

Files:

• ngx_http_upstream_keepalive_module.c (modified) (4 diffs)
• t/memcached-keepalive.t (modified) (4 diffs)

upstream_keepalive/ngx_http_upstream_keepalive_module.c

@@ -26 +26 @@
     ngx_http_upstream_keepalive_srv_conf_t  *conf;
 
+    ngx_http_upstream_t               *upstream;
+
     void                              *data;
 
     ngx_event_get_peer_pt              original_get_peer;
     ngx_event_free_peer_pt             original_free_peer;
+
+    ngx_uint_t                         failed;       /* unsigned:1 */
 
 } ngx_http_upstream_keepalive_peer_data_t;
@@ -170 +174 @@
 
     kp->conf = kcf;
+    kp->upstream = r->upstream;
     kp->data = r->upstream->peer.data;
     kp->original_get_peer = r->upstream->peer.get;
@@ -194 +199 @@
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, pc->log, 0,
                    "get keepalive peer");
+
+    kp->failed = 0;
 
     /* single pool of cached connections */
@@ -265 +272 @@
     ngx_http_upstream_keepalive_cache_t      *item;
 
-    ngx_queue_t       *q;
-    ngx_connection_t  *c;
+    ngx_uint_t            status;
+    ngx_queue_t          *q;
+    ngx_connection_t     *c;
+    ngx_http_upstream_t  *u;
 
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, pc->log, 0,
                    "free keepalive peer");
 
-    if (!(state & NGX_PEER_FAILED)
-        && pc->connection != NULL)
+    /* remember failed state - peer.free() may be called more than once */
+
+    if (state & NGX_PEER_FAILED) {
+        kp->failed = 1;
+    }
+
+    /*
+     * cache valid connections
+     *
+     * For memcached this means status either 404 or 200.  For status 200 we
+     * should also check if all response body was read (u->length == 0) and
+     * make sure that u->length is valid (we use u->header_sent flag to test
+     * this).  Memcached is the only supported protocol for now.
+     *
+     * Some notes on other possibilities (incomplete):
+     *
+     * fastcgi: u->pipe->upstream_done should be sufficient
+     *
+     * proxy buffered: u->pipe->upstream_done, 304 replies, replies to head
+     * requests (see RFC 2616, 4.4 Message Length)
+     *
+     * proxy unbuffered: 200 as for memcached (with u->length == 0 and
+     * header_sent), 304, replies to head requests
+     *
+     * subrequest_in_memory: won't work as of now
+     *
+     * TODO: move this logic to protocol modules (NGX_PEER_KEEPALIVE?)
+     */
+
+    u = kp->upstream;
+    status = u->headers_in.status_n;
+
+    if (!kp->failed
+        && pc->connection != NULL
+        && (status == NGX_HTTP_NOT_FOUND
+            || (status == NGX_HTTP_OK && u->header_sent && u->length == 0)))
     {
         c = pc->connection;
+
+        ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0,
+                       "free keepalive peer: saving connection %p", c);
 
         if (ngx_queue_empty(&kp->conf->free)) {

upstream_keepalive/t/memcached-keepalive.t

@@ -21 +21 @@
 plain(skip_all => 'Cache::Memcached not installed') if $@;
 
-my $t = Test::Nginx->new()->has('rewrite')->has_daemon('memcached')->plan(10)
+my $t = Test::Nginx->new()->has('rewrite')->has_daemon('memcached')->plan(16)
     ->write_file_expand('nginx.conf', <<'EOF');
 
@@ -95 +95 @@
 EOF
 
-$t->run_daemon('memcached', '-l', '127.0.0.1', '-p', '8081');
-$t->run_daemon('memcached', '-l', '127.0.0.1', '-p', '8082');
+if (`memcached -h` =~ /repcached/) {
+    # repcached patches adds additional listen socket memcached
+    # that should be different too
+
+    $t->run_daemon('memcached', '-l', '127.0.0.1', '-p', '8081',
+        '-X', '8091');
+    $t->run_daemon('memcached', '-l', '127.0.0.1', '-p', '8082',
+        '-X', '8092');
+
+} else {
+    $t->run_daemon('memcached', '-l', '127.0.0.1', '-p', '8081');
+    $t->run_daemon('memcached', '-l', '127.0.0.1', '-p', '8082');
+}
+
 $t->run();
+
+$t->waitforsocket('127.0.0.1:8081')
+    or die "Unable to start memcached";
+$t->waitforsocket('127.0.0.1:8082')
+    or die "Unable to start second memcached";
 
 ###############################################################################
@@ -106 +123 @@
 $memd1->set('/', 'SEE-THIS');
 $memd2->set('/', 'SEE-THIS');
+$memd1->set('/big', 'X' x 1000000);
 
 my $total = $memd1->stats()->{total}->{total_connections};
@@ -120 +138 @@
     'only one connection used');
 
+# Since nginx doesn't read all data from connection in some situations (head
+# requests, post_action, errors writing to client) we have to close such
+# connections.  Check if we really do close them.
+
+$total = $memd1->stats()->{total}->{total_connections};
+
+unlike(http_head('/'), qr/SEE-THIS/, 'head request');
+like(http_get('/'), qr/SEE-THIS/, 'get after head');
+
+is($memd1->stats()->{total}->{total_connections}, $total + 1,
+    'head request closes connection');
+
+$total = $memd1->stats()->{total}->{total_connections};
+
+unlike(http_head('/big'), qr/XXX/, 'big head');
+like(http_get('/'), qr/SEE-THIS/, 'get after big head');
+
+is($memd1->stats()->{total}->{total_connections}, $total + 1,
+    'big head request closes connection');
+
 # two backends with 'single' option - should establish only one connection