Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Merge pull request #670 from sonatype/nexus-5368-stale-connections

NEXUS-5368: Stale connections in pool
  • Loading branch information...
commit e4b6124c6b861ab53d7e1bd0d76c5c713661fbfc 2 parents f24c205 + e96d142
Tamas Cservenak authored November 23, 2012
96  nexus/nexus-proxy/src/main/java/org/sonatype/nexus/apachehttpclient/Hc4ProviderImpl.java
@@ -68,7 +68,7 @@
68 68
 
69 69
 /**
70 70
  * Default implementation of {@link Hc4Provider}.
71  
- * 
  71
+ *
72 72
  * @author cstamas
73 73
  * @since 2.2
74 74
  */
@@ -78,6 +78,7 @@
78 78
     extends AbstractLoggingComponent
79 79
     implements Hc4Provider
80 80
 {
  81
+
81 82
     /**
82 83
      * Key for customizing connection pool maximum size. Value should be integer equal to 0 or greater. Pool size of 0
83 84
      * will actually prevent use of pool. Any positive number means the actual size of the pool to be created. This is a
@@ -103,15 +104,15 @@
103 104
     private static final int CONNECTION_POOL_SIZE_DEFAULT = 20;
104 105
 
105 106
     /**
106  
-     * Key for customizing connection pool keep-alive. In other words, how long open connections (sockets) are kept in
107  
-     * pool before evicted and closed. Value is milliseconds.
  107
+     * Key for customizing connection pool idle time. In other words, how long open connections (sockets) are kept in
  108
+     * pool idle (unused) before they get evicted and closed. Value is milliseconds.
108 109
      */
109  
-    private static final String CONNECTION_POOL_KEEPALIVE_KEY = "nexus.apacheHttpClient4x.connectionPoolKeepalive";
  110
+    private static final String CONNECTION_POOL_IDLE_TIME_KEY = "nexus.apacheHttpClient4x.connectionPoolIdleTime";
110 111
 
111 112
     /**
112  
-     * Default pool keep-alive: 1 minute.
  113
+     * Default pool idle time: 30 seconds.
113 114
      */
114  
-    private static final long CONNECTION_POOL_KEEPALIVE_DEFAULT = TimeUnit.MINUTES.toMillis( 1 );
  115
+    private static final long CONNECTION_POOL_IDLE_TIME_DEFAULT = TimeUnit.SECONDS.toMillis( 30 );
115 116
 
116 117
     /**
117 118
      * Key for customizing connection pool timeout. In other words, how long should a HTTP request execution be blocked
@@ -120,9 +121,20 @@
120 121
     private static final String CONNECTION_POOL_TIMEOUT_KEY = "nexus.apacheHttpClient4x.connectionPoolTimeout";
121 122
 
122 123
     /**
123  
-     * Default pool timeout: equals to {@link #CONNECTION_POOL_KEEPALIVE_DEFAULT}.
  124
+     * Default pool timeout: 30 seconds.
  125
+     */
  126
+    private static final long CONNECTION_POOL_TIMEOUT_DEFAULT = TimeUnit.SECONDS.toMillis( 30 );
  127
+
  128
+    /**
  129
+     * Key for customizing default (and max) keep alive duration when remote server does not state anything,
  130
+     * or states some unreal high value. Value is milliseconds.
  131
+     */
  132
+    private static final String KEEP_ALIVE_MAX_DURATION_KEY = "nexus.apacheHttpClient4x.keepAliveMaxDuration";
  133
+
  134
+    /**
  135
+     * Default keep alive max duration: 30 seconds.
124 136
      */
125  
-    private static final long CONNECTION_POOL_TIMEOUT_DEFAULT = CONNECTION_POOL_KEEPALIVE_DEFAULT;
  137
+    private static final long KEEP_ALIVE_MAX_DURATION_DEFAULT = TimeUnit.SECONDS.toMillis( 30 );
126 138
 
127 139
     // ==
128 140
 
@@ -158,36 +170,38 @@
158 170
 
159 171
     /**
160 172
      * Constructor.
161  
-     * 
  173
+     *
162 174
      * @param applicationConfiguration the Nexus {@link ApplicationConfiguration}.
163  
-     * @param userAgentBuilder UA builder component.
164  
-     * @param eventBus the event multicaster
165  
-     * @param jmxInstaller installer to expose pool information over JMX.
  175
+     * @param userAgentBuilder         UA builder component.
  176
+     * @param eventBus                 the event multicaster
  177
+     * @param jmxInstaller             installer to expose pool information over JMX.
166 178
      */
167 179
     @Inject
168 180
     public Hc4ProviderImpl( final ApplicationConfiguration applicationConfiguration,
169  
-                            final UserAgentBuilder userAgentBuilder,
170  
-                            final EventBus eventBus,
171  
-                            final PoolingClientConnectionManagerMBeanInstaller jmxInstaller )
  181
+        final UserAgentBuilder userAgentBuilder,
  182
+        final EventBus eventBus,
  183
+        final PoolingClientConnectionManagerMBeanInstaller jmxInstaller )
172 184
     {
173 185
         this.applicationConfiguration = Preconditions.checkNotNull( applicationConfiguration );
174 186
         this.userAgentBuilder = Preconditions.checkNotNull( userAgentBuilder );
175 187
         this.jmxInstaller = Preconditions.checkNotNull( jmxInstaller );
176 188
         this.sharedConnectionManager = createClientConnectionManager();
177  
-        this.evictingThread = new EvictingThread( sharedConnectionManager, getConnectionPoolKeepalive() );
  189
+        this.evictingThread = new EvictingThread( sharedConnectionManager, getConnectionPoolIdleTime() );
178 190
         this.evictingThread.start();
179 191
         this.eventBus = Preconditions.checkNotNull( eventBus );
180 192
         this.eventBus.register( this );
181 193
         this.jmxInstaller.register( sharedConnectionManager );
182  
-        getLogger().info( "{} started up (keep-alive {} millis), listening for shutdown.", getClass().getSimpleName(),
183  
-            getConnectionPoolKeepalive() );
  194
+        getLogger().info(
  195
+            "{} started (connectionPoolMaxSize {}, connectionPoolSize {}, connectionPoolIdleTime {} ms, connectionPoolTimeout {} ms, keepAliveMaxDuration {} ms)",
  196
+            getClass().getSimpleName(), getConnectionPoolMaxSize(), getConnectionPoolSize(),
  197
+            getConnectionPoolIdleTime(), getConnectionPoolTimeout(), getKeepAliveMaxDuration() );
184 198
     }
185 199
 
186 200
     // configuration
187 201
 
188 202
     /**
189 203
      * Returns the pool max size.
190  
-     * 
  204
+     *
191 205
      * @return pool max size
192 206
      */
193 207
     protected int getConnectionPoolMaxSize()
@@ -197,7 +211,7 @@ protected int getConnectionPoolMaxSize()
197 211
 
198 212
     /**
199 213
      * Returns the pool size per route.
200  
-     * 
  214
+     *
201 215
      * @return pool per route size
202 216
      */
203 217
     protected int getConnectionPoolSize()
@@ -206,18 +220,18 @@ protected int getConnectionPoolSize()
206 220
     }
207 221
 
208 222
     /**
209  
-     * Returns the keep alive (idle open) time in milliseconds.
210  
-     * 
211  
-     * @return keep alive in milliseconds.
  223
+     * Returns the connection pool idle (idle as unused but pooled) time in milliseconds.
  224
+     *
  225
+     * @return idle time in milliseconds.
212 226
      */
213  
-    protected long getConnectionPoolKeepalive()
  227
+    protected long getConnectionPoolIdleTime()
214 228
     {
215  
-        return SystemPropertiesHelper.getLong( CONNECTION_POOL_KEEPALIVE_KEY, CONNECTION_POOL_KEEPALIVE_DEFAULT );
  229
+        return SystemPropertiesHelper.getLong( CONNECTION_POOL_IDLE_TIME_KEY, CONNECTION_POOL_IDLE_TIME_DEFAULT );
216 230
     }
217 231
 
218 232
     /**
219 233
      * Returns the pool timeout in milliseconds.
220  
-     * 
  234
+     *
221 235
      * @return pool timeout in milliseconds.
222 236
      */
223 237
     protected long getConnectionPoolTimeout()
@@ -226,8 +240,18 @@ protected long getConnectionPoolTimeout()
226 240
     }
227 241
 
228 242
     /**
  243
+     * Returns the maximum Keep-Alive duration in milliseconds.
  244
+     *
  245
+     * @return default Keep-Alive duration in milliseconds.
  246
+     */
  247
+    protected long getKeepAliveMaxDuration()
  248
+    {
  249
+        return SystemPropertiesHelper.getLong( KEEP_ALIVE_MAX_DURATION_KEY, KEEP_ALIVE_MAX_DURATION_DEFAULT );
  250
+    }
  251
+
  252
+    /**
229 253
      * Returns the connection timeout in milliseconds. The timeout until connection is established.
230  
-     * 
  254
+     *
231 255
      * @param context
232 256
      * @return the connection timeout in milliseconds.
233 257
      */
@@ -246,7 +270,7 @@ protected int getConnectionTimeout( final RemoteStorageContext context )
246 270
 
247 271
     /**
248 272
      * Returns the SO_SOCKET timeout in milliseconds. The timeout for waiting for data on established connection.
249  
-     * 
  273
+     *
250 274
      * @param context
251 275
      * @return the SO_SOCKET timeout in milliseconds.
252 276
      */
@@ -268,7 +292,7 @@ public synchronized void shutdown()
268 292
         jmxInstaller.unregister();
269 293
         sharedConnectionManager.shutdown();
270 294
         eventBus.unregister( this );
271  
-        getLogger().info( "{} shut down.", getClass().getSimpleName() );
  295
+        getLogger().info( "{} stopped.", getClass().getSimpleName() );
272 296
     }
273 297
 
274 298
     @Subscribe
@@ -325,6 +349,7 @@ public DefaultHttpClient createHttpClient( final RemoteStorageContext context )
325 349
     private static class DefaultHttpClientImpl
326 350
         extends DefaultHttpClient
327 351
     {
  352
+
328 353
         private DefaultHttpClientImpl( final ClientConnectionManager conman, final HttpParams params )
329 354
         {
330 355
             super( conman, params );
@@ -340,7 +365,7 @@ protected BasicHttpProcessor createHttpProcessor()
340 365
     }
341 366
 
342 367
     protected DefaultHttpClient createHttpClient( final RemoteStorageContext context,
343  
-                                                  final ClientConnectionManager clientConnectionManager )
  368
+        final ClientConnectionManager clientConnectionManager )
344 369
     {
345 370
         final DefaultHttpClient httpClient =
346 371
             new DefaultHttpClientImpl( clientConnectionManager, createHttpParams( context ) );
@@ -348,9 +373,11 @@ protected DefaultHttpClient createHttpClient( final RemoteStorageContext context
348 373
         configureProxy( httpClient, context.getRemoteProxySettings() );
349 374
         // obey the given retries count and apply it to client.
350 375
         final int retries =
351  
-            context.getRemoteConnectionSettings() != null ? context.getRemoteConnectionSettings().getRetrievalRetryCount()
  376
+            context.getRemoteConnectionSettings() != null
  377
+                ? context.getRemoteConnectionSettings().getRetrievalRetryCount()
352 378
                 : 0;
353 379
         httpClient.setHttpRequestRetryHandler( new StandardHttpRequestRetryHandler( retries, false ) );
  380
+        httpClient.setKeepAliveStrategy( new NexusConnectionKeepAliveStrategy( getKeepAliveMaxDuration() ) );
354 381
         return httpClient;
355 382
     }
356 383
 
@@ -388,7 +415,7 @@ protected PoolingClientConnectionManager createClientConnectionManager()
388 415
     // ==
389 416
 
390 417
     protected void configureAuthentication( final DefaultHttpClient httpClient, final RemoteAuthenticationSettings ras,
391  
-                                            final HttpHost proxyHost )
  418
+        final HttpHost proxyHost )
392 419
     {
393 420
         if ( ras != null )
394 421
         {
@@ -413,14 +440,15 @@ else if ( ras instanceof NtlmRemoteAuthenticationSettings )
413 440
                 authorisationPreference.add( 0, AuthPolicy.NTLM );
414 441
                 getLogger().info( "... {} authentication setup for NTLM domain '{}'", authScope, nras.getNtlmDomain() );
415 442
                 credentials =
416  
-                    new NTCredentials( nras.getUsername(), nras.getPassword(), nras.getNtlmHost(), nras.getNtlmDomain() );
  443
+                    new NTCredentials( nras.getUsername(), nras.getPassword(), nras.getNtlmHost(),
  444
+                                       nras.getNtlmDomain() );
417 445
             }
418 446
             else if ( ras instanceof UsernamePasswordRemoteAuthenticationSettings )
419 447
             {
420 448
                 final UsernamePasswordRemoteAuthenticationSettings uras =
421 449
                     (UsernamePasswordRemoteAuthenticationSettings) ras;
422 450
                 getLogger().info( "... {} authentication setup for remote storage with username '{}'", authScope,
423  
-                    uras.getUsername() );
  451
+                                  uras.getUsername() );
424 452
                 credentials = new UsernamePasswordCredentials( uras.getUsername(), uras.getPassword() );
425 453
             }
426 454
 
74  nexus/nexus-proxy/src/main/java/org/sonatype/nexus/apachehttpclient/NexusConnectionKeepAliveStrategy.java
... ...
@@ -0,0 +1,74 @@
  1
+/**
  2
+ * Sonatype Nexus (TM) Open Source Version
  3
+ * Copyright (c) 2007-2012 Sonatype, Inc.
  4
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
  5
+ *
  6
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
  7
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
  8
+ *
  9
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
  10
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
  11
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
  12
+ */
  13
+package org.sonatype.nexus.apachehttpclient;
  14
+
  15
+import org.apache.http.HttpResponse;
  16
+import org.apache.http.impl.client.DefaultConnectionKeepAliveStrategy;
  17
+import org.apache.http.protocol.HttpContext;
  18
+import com.google.common.base.Preconditions;
  19
+
  20
+/**
  21
+ * Nexus connection keep alive strategy, that differs from the HC4 default one only in one thing: when server does
  22
+ * not state timeout, it never says "indefinite" (meaning pool it forever), but instead a finite amount of time.
  23
+ *
  24
+ * @author cstamas
  25
+ * @since 2.3
  26
+ */
  27
+public class NexusConnectionKeepAliveStrategy
  28
+    extends DefaultConnectionKeepAliveStrategy
  29
+{
  30
+
  31
+    /**
  32
+     * The max duration for how long to pool a connection in milliseconds. Used as default too, instead of
  33
+     * "indefinite" case.
  34
+     */
  35
+    private final long maxKeepAliveDuration;
  36
+
  37
+    /**
  38
+     * Constructor.
  39
+     *
  40
+     * @param maxKeepAliveDuration the max duration in millis for how long to pool the connection.
  41
+     */
  42
+    public NexusConnectionKeepAliveStrategy( final long maxKeepAliveDuration )
  43
+    {
  44
+        Preconditions.checkArgument( maxKeepAliveDuration > -1,
  45
+                                     "maxKeepAliveDuration must be 0 or higher, but is set to %s",
  46
+                                     maxKeepAliveDuration );
  47
+        this.maxKeepAliveDuration = maxKeepAliveDuration;
  48
+    }
  49
+
  50
+    /**
  51
+     * Returns the duration of time which this connection can be safely kept
  52
+     * idle. Nexus by default does not "believe" much to remote servers, and will never
  53
+     * keep connection pooled "forever", nor will keep it pooled for unreasonable long time.
  54
+     *
  55
+     * @param response
  56
+     * @param context
  57
+     * @return the duration of time which this connection can be safely kept idle in pool.
  58
+     */
  59
+    public long getKeepAliveDuration( HttpResponse response, HttpContext context )
  60
+    {
  61
+        // ask super class
  62
+        final long result = super.getKeepAliveDuration( response, context );
  63
+        if ( result < 0 )
  64
+        {
  65
+            // if "indefinite", use default
  66
+            return maxKeepAliveDuration;
  67
+        }
  68
+        else
  69
+        {
  70
+            // else "cap" it with max
  71
+            return Math.min( result, maxKeepAliveDuration );
  72
+        }
  73
+    }
  74
+}
146  .../nexus-proxy/src/test/java/org/sonatype/nexus/apachehttpclient/NexusConnectionKeepAliveStrategyTest.java
... ...
@@ -0,0 +1,146 @@
  1
+/**
  2
+ * Sonatype Nexus (TM) Open Source Version
  3
+ * Copyright (c) 2007-2012 Sonatype, Inc.
  4
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
  5
+ *
  6
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
  7
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
  8
+ *
  9
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
  10
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
  11
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
  12
+ */
  13
+package org.sonatype.nexus.apachehttpclient;
  14
+
  15
+import java.util.ArrayList;
  16
+import java.util.Collections;
  17
+import java.util.List;
  18
+
  19
+import org.apache.http.Header;
  20
+import org.apache.http.HttpResponse;
  21
+import org.apache.http.message.BasicHeader;
  22
+import org.apache.http.message.BasicListHeaderIterator;
  23
+import org.apache.http.protocol.HttpContext;
  24
+import org.hamcrest.MatcherAssert;
  25
+import org.hamcrest.Matchers;
  26
+import org.junit.Before;
  27
+import org.junit.Test;
  28
+import org.mockito.Mock;
  29
+import org.mockito.Mockito;
  30
+import org.sonatype.sisu.litmus.testsupport.TestSupport;
  31
+
  32
+/**
  33
+ * Test for NexusConnectionKeepAliveStrategy.
  34
+ */
  35
+public class NexusConnectionKeepAliveStrategyTest
  36
+    extends TestSupport
  37
+{
  38
+
  39
+    @Mock
  40
+    protected HttpResponse httpResponse;
  41
+
  42
+    @Mock
  43
+    protected HttpContext httpContext;
  44
+
  45
+    protected NexusConnectionKeepAliveStrategy subject;
  46
+
  47
+    @Before
  48
+    public void prepare()
  49
+    {
  50
+        subject = new NexusConnectionKeepAliveStrategy( 5000l );
  51
+        Mockito.when( httpResponse.headerIterator( Mockito.anyString() ) ).thenReturn( new BasicListHeaderIterator(
  52
+            Collections.<Header>emptyList(), null ) );
  53
+    }
  54
+
  55
+    @Test
  56
+    public void constructorValueValidationBad1()
  57
+    {
  58
+        doConstructorValueValidation( -100, true );
  59
+    }
  60
+
  61
+    @Test
  62
+    public void constructorValueValidationBad2()
  63
+    {
  64
+        doConstructorValueValidation( -1, true );
  65
+    }
  66
+
  67
+    @Test
  68
+    public void constructorValueValidationGood1()
  69
+    {
  70
+        doConstructorValueValidation( 0, false );
  71
+    }
  72
+
  73
+    @Test
  74
+    public void constructorValueValidationGood2()
  75
+    {
  76
+        doConstructorValueValidation( 30000, false );
  77
+    }
  78
+
  79
+    protected void doConstructorValueValidation( final long maxKeepAlive, final boolean shouldFail )
  80
+    {
  81
+        try
  82
+        {
  83
+            new NexusConnectionKeepAliveStrategy( maxKeepAlive );
  84
+            MatcherAssert.assertThat( "Value " + maxKeepAlive + " does not fails but should!", !shouldFail );
  85
+        }
  86
+        catch ( IllegalArgumentException e )
  87
+        {
  88
+            MatcherAssert.assertThat( "Value " + maxKeepAlive + " fails but should not!", shouldFail );
  89
+        }
  90
+    }
  91
+
  92
+    // ==
  93
+
  94
+    @Test
  95
+    public void keepAliveDefaulted()
  96
+    {
  97
+        // server response says nothing
  98
+        // nexus default wins
  99
+        final long keepAlive =
  100
+            subject.getKeepAliveDuration( httpResponse, httpContext );
  101
+        MatcherAssert.assertThat( keepAlive, Matchers.is( 5000l ) );
  102
+    }
  103
+
  104
+    @Test
  105
+    public void keepAliveServerValueIfLess()
  106
+    {
  107
+        // server response says 3s
  108
+        // server wins
  109
+        final List<Header> headers = new ArrayList<Header>( 1 );
  110
+        headers.add( new BasicHeader( "Keep-Alive", "timeout=3" ) );
  111
+        Mockito.when( httpResponse.headerIterator( Mockito.anyString() ) ).thenReturn( new BasicListHeaderIterator(
  112
+            headers, null ) );
  113
+        final long keepAlive =
  114
+            subject.getKeepAliveDuration( httpResponse, httpContext );
  115
+        MatcherAssert.assertThat( keepAlive, Matchers.is( 3000l ) );
  116
+    }
  117
+
  118
+    @Test
  119
+    public void keepAliveServerValueIfLessWithMaxConnCount()
  120
+    {
  121
+        // server response says 3s
  122
+        // server wins
  123
+        final List<Header> headers = new ArrayList<Header>( 1 );
  124
+        headers.add( new BasicHeader( "Keep-Alive", "timeout=3, max=100" ) );
  125
+        Mockito.when( httpResponse.headerIterator( Mockito.anyString() ) ).thenReturn( new BasicListHeaderIterator(
  126
+            headers, null ) );
  127
+        final long keepAlive =
  128
+            subject.getKeepAliveDuration( httpResponse, httpContext );
  129
+        MatcherAssert.assertThat( keepAlive, Matchers.is( 3000l ) );
  130
+    }
  131
+
  132
+    @Test
  133
+    public void keepAliveDefaultValueIfMore()
  134
+    {
  135
+        // server response says 8s
  136
+        // nexus wins (is capped)
  137
+        final List<Header> headers = new ArrayList<Header>( 1 );
  138
+        headers.add( new BasicHeader( "Keep-Alive", "timeout=8" ) );
  139
+        Mockito.when( httpResponse.headerIterator( Mockito.anyString() ) ).thenReturn( new BasicListHeaderIterator(
  140
+            headers, null ) );
  141
+        final long keepAlive =
  142
+            subject.getKeepAliveDuration( httpResponse, httpContext );
  143
+        MatcherAssert.assertThat( keepAlive, Matchers.is( 5000l ) );
  144
+    }
  145
+
  146
+}

0 notes on commit e4b6124

Please sign in to comment.
Something went wrong with that request. Please try again.