You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by se...@apache.org on 2021/08/02 12:45:55 UTC

[directory-ldap-api] branch master updated: Finer-grain timeouts (DIRAPI-378)

This is an automated email from the ASF dual-hosted git repository.

semancik pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/directory-ldap-api.git


The following commit(s) were added to refs/heads/master by this push:
     new c2fd0a0  Finer-grain timeouts (DIRAPI-378)
c2fd0a0 is described below

commit c2fd0a08f00888f31d773c46f0c6202537db7894
Author: Radovan Semancik <ra...@evolveum.com>
AuthorDate: Mon Aug 2 14:45:20 2021 +0200

    Finer-grain timeouts (DIRAPI-378)
---
 .../ldap/client/api/LdapConnectionConfig.java      | 143 +++++++++++++++++++++
 .../ldap/client/api/LdapNetworkConnection.java     | 100 +++++++++-----
 .../ldap/client/api/LdapNetworkConnectionTest.java |   4 +-
 3 files changed, 213 insertions(+), 34 deletions(-)

diff --git a/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapConnectionConfig.java b/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapConnectionConfig.java
index b958b15..c3a6a0c 100644
--- a/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapConnectionConfig.java
+++ b/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapConnectionConfig.java
@@ -73,6 +73,21 @@ public class LdapConnectionConfig
     /** The session timeout in milliseconds */
     private long timeout = DEFAULT_TIMEOUT;
 
+    /** Timeout for connect and bind operations */
+    private Long connectTimeout;
+
+    /** Timeout for write operations (add, modify, delete, ...) */
+    private Long writeOperationTimeout;
+
+    /** Timeout for read operations (search, compare) */
+    private Long readOperationTimeout;
+
+    /** Timeout for close and unbind operations */
+    private Long closeTimeout;
+
+    /** Timeout for I/O (TCP) writes */
+    private Long sendTimeout;
+
     /** A flag indicating if we are using TLS or not, default value is false */
     private boolean useTls = false;
 
@@ -306,6 +321,9 @@ public class LdapConnectionConfig
 
     /**
      * Gets the timeout in milliseconds.
+     * This is a "global" timeout.
+     * It is used when operation-specific timeouts are not specified.
+     * It is also used for extended operations.
      *
      * @return the timeout in milliseconds
      */
@@ -317,6 +335,9 @@ public class LdapConnectionConfig
 
     /**
      * Sets the timeout in milliseconds.
+     * This is a "global" timeout.
+     * It is used when operation-specific timeouts are not specified.
+     * It is also used for extended operations.
      *
      * @param timeout the timeout in milliseconds to set
      */
@@ -325,6 +346,128 @@ public class LdapConnectionConfig
         this.timeout = timeout;
     }
 
+    /**
+     * Gets connect timeout in milliseconds.
+     * Connect timeout is applied to connect and bind operations.
+     * If not specified, global timeout setting is applied.
+     *
+     * @return the timeout in milliseconds
+     */
+    public Long getConnectTimeout()
+    {
+        return connectTimeout;
+    }
+
+    /**
+     * Sets connect timeout in milliseconds.
+     * Connect timeout is applied to connect and bind operations.
+     * If not specified, global timeout setting is applied.
+     *
+     * @param timeout the timeout in milliseconds to set
+     */
+    public void setConnectTimeout( Long timeout )
+    {
+        this.connectTimeout = timeout;
+    }
+
+    /**
+     * Gets write operation timeout in milliseconds.
+     * Write operation timeout is applied to operations that write data, such as add, modify and delete.
+     * If not specified, global timeout setting is applied.
+     *
+     * @return the timeout in milliseconds
+     */
+    public Long getWriteOperationTimeout()
+    {
+        return writeOperationTimeout;
+    }
+
+    /**
+     * Sets write operation timeout in milliseconds.
+     * Write operation timeout is applied to operations that write data, such as add, modify and delete.
+     * If not specified, global timeout setting is applied.
+     *
+     * @param timeout the timeout in milliseconds to set
+     */
+    public void setWriteOperationTimeout( Long timeout )
+    {
+        this.writeOperationTimeout = timeout;
+    }
+
+    /**
+     * Gets read operation timeout in milliseconds.
+     * This timeout is applied to read operations, such as search and compare.
+     * If not specified, global timeout setting is applied.
+     *
+     * @return the timeout in milliseconds
+     */
+    public Long getReadOperationTimeout()
+    {
+        return readOperationTimeout;
+    }
+
+    /**
+     * Sets read operation timeout in milliseconds.
+     * This timeout is applied to read operations, such as search and compare.
+     * If not specified, global timeout setting is applied.
+     *
+     * @param timeout the timeout in milliseconds to set
+     */
+    public void setReadOperationTimeout( Long timeout )
+    {
+        this.readOperationTimeout = timeout;
+    }
+
+    /**
+     * Gets close timeout in milliseconds.
+     * Close timeout is applied to close and unbind operations.
+     * If not specified, global timeout setting is applied.
+     *
+     * @return the timeout in milliseconds
+     */
+    public Long getCloseTimeout()
+    {
+        return closeTimeout;
+    }
+
+
+    /**
+     * Sets close timeout in milliseconds.
+     * Close timeout is applied to close and unbind operations.
+     * If not specified, global timeout setting is applied.
+     *
+     * @param timeout the timeout in milliseconds to set
+     */
+    public void setCloseTimeout( Long timeout )
+    {
+        this.closeTimeout = timeout;
+    }
+
+    /**
+     * Gets send timeout in milliseconds.
+     * Send timeout is used for I/O (TCP) write operations.
+     * If not specified, global timeout setting is applied.
+     *
+     * @return the timeout in milliseconds
+     */
+    public Long getSendTimeout()
+    {
+        return sendTimeout;
+    }
+
+
+    /**
+     * Sets the send timeout in milliseconds.
+     * Send timeout is used for I/O (TCP) write operations.
+     * If not specified, global timeout setting is applied.
+     *
+     * @param timeout the timeout in milliseconds to set
+     */
+    public void setSendTimeout( Long timeout )
+    {
+        this.sendTimeout = timeout;
+    }
+
 
     /**
      * Gets the supported LDAP version.
diff --git a/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java b/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
index 43d3dab..842acad 100644
--- a/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
+++ b/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
@@ -205,6 +205,21 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
     /** The timeout used for response we are waiting for */
     private long timeout = LdapConnectionConfig.DEFAULT_TIMEOUT;
 
+    /** Timeout for connect and bind operations */
+    private long connectTimeout = LdapConnectionConfig.DEFAULT_TIMEOUT;
+
+    /** Timeout for write operations, such as add, modify and delete */
+    private long writeOperationTimeout = LdapConnectionConfig.DEFAULT_TIMEOUT;
+
+    /** Timeout for read operations, suc as search and compare */
+    private long readOperationTimeout = LdapConnectionConfig.DEFAULT_TIMEOUT;
+
+    /** Timeout for close and unbind operations */
+    private long closeTimeout = LdapConnectionConfig.DEFAULT_TIMEOUT;
+
+    /** Timeout for I/O (TCP) writes */
+    private long sendTimeout = LdapConnectionConfig.DEFAULT_TIMEOUT;
+
     /** configuration object for the connection */
     private LdapConnectionConfig config;
     
@@ -309,8 +324,17 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         }
         
         this.timeout = config.getTimeout();
+        this.connectTimeout = determineTimeoutConfiguration( config.getConnectTimeout() );
+        this.writeOperationTimeout = determineTimeoutConfiguration( config.getWriteOperationTimeout() );
+        this.readOperationTimeout = determineTimeoutConfiguration( config.getReadOperationTimeout() );
+        this.closeTimeout = determineTimeoutConfiguration( config.getCloseTimeout() );
+        this.sendTimeout = determineTimeoutConfiguration( config.getSendTimeout() );
     }
 
+    private long determineTimeoutConfiguration( Long localTimeout )
+    {
+        return localTimeout == null ? this.timeout : localTimeout;
+    }
 
     /**
      * Create a new instance of a LdapConnection on localhost,
@@ -645,15 +669,15 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
      * Get the largest timeout from the search time limit and the connection
      * timeout.
      * 
-     * @param connectionTimoutInMS Connection timeout
+     * @param configuredTimeout Timeout configured in LdapNetworkConnection
      * @param searchTimeLimitInSeconds Search timeout
      * @return The largest timeout
      */
-    public long getTimeout( long connectionTimoutInMS, int searchTimeLimitInSeconds )
+    public long getTimeout( long configuredTimeout, int searchTimeLimitInSeconds )
     {
         if ( searchTimeLimitInSeconds < 0 )
         {
-            return connectionTimoutInMS;
+            return configuredTimeout;
         }
         else if ( searchTimeLimitInSeconds == 0 )
         {
@@ -663,13 +687,13 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
             }
             else
             {
-                return config.getTimeout();
+                return configuredTimeout;
             }
         }
         else
         {
             long searchTimeLimitInMS = searchTimeLimitInSeconds * 1000L;
-            return Math.max( searchTimeLimitInMS, connectionTimoutInMS );
+            return Math.max( searchTimeLimitInMS, configuredTimeout );
         }
     }
 
@@ -690,7 +714,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         // Wait until it's established
         try
         {
-            result = connectionFuture.await( timeout );
+            result = connectionFuture.await( connectTimeout );
         }
         catch ( InterruptedException e )
         {
@@ -722,7 +746,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
             if ( connectionException == null )
             {
                 // This was a timeout
-                String message = I18n.msg( I18n.MSG_04177_CONNECTION_TIMEOUT, timeout );
+                String message = I18n.msg( I18n.MSG_04177_CONNECTION_TIMEOUT, connectTimeout );
                 
                 if ( LOG.isDebugEnabled() )
                 {
@@ -800,7 +824,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
     {
         try
         {
-            boolean isSecured = handshakeFuture.get( timeout, TimeUnit.MILLISECONDS );
+            boolean isSecured = handshakeFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( !isSecured )
             {
@@ -1036,7 +1060,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             if ( ( ioSession != null ) && ioSession.isConnected() )
             { 
-                connectionCloseFuture.get( timeout, TimeUnit.MILLISECONDS );
+                connectionCloseFuture.get( closeTimeout, TimeUnit.MILLISECONDS );
             }
         }
         catch ( TimeoutException | ExecutionException | InterruptedException e )
@@ -1141,7 +1165,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            AddResponse addResponse = addFuture.get( timeout, TimeUnit.MILLISECONDS );
+            AddResponse addResponse = addFuture.get( writeOperationTimeout, TimeUnit.MILLISECONDS );
 
             if ( addResponse == null )
             {
@@ -1536,7 +1560,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            BindResponse bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+            BindResponse bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( bindResponse == null )
             {
@@ -1697,7 +1721,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            BindResponse bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+            BindResponse bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( bindResponse == null )
             {
@@ -1769,7 +1793,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            BindResponse bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+            BindResponse bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( bindResponse == null )
             {
@@ -1877,7 +1901,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            BindResponse bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+            BindResponse bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( bindResponse == null )
             {
@@ -1963,7 +1987,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            BindResponse bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+            BindResponse bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( bindResponse == null )
             {
@@ -2035,7 +2059,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            BindResponse bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+            BindResponse bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( bindResponse == null )
             {
@@ -2107,7 +2131,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            BindResponse bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+            BindResponse bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
             if ( bindResponse == null )
             {
@@ -2387,9 +2411,9 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
 
         SearchFuture searchFuture = searchAsync( searchRequest );
 
-        long searchTimeout = getTimeout( timeout, searchRequest.getTimeLimit() );
+        long localSearchTimeout = getTimeout( readOperationTimeout, searchRequest.getTimeLimit() );
 
-        return new SearchCursorImpl( searchFuture, searchTimeout, TimeUnit.MILLISECONDS );
+        return new SearchCursorImpl( searchFuture, localSearchTimeout, TimeUnit.MILLISECONDS );
     }
 
 
@@ -2422,11 +2446,11 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         // Use this for logging instead: WriteFuture unbindFuture = ldapSession.write( unbindRequest )
         WriteFuture unbindFuture = ioSession.write( unbindRequest );
 
-        unbindFuture.awaitUninterruptibly( timeout );
+        unbindFuture.awaitUninterruptibly( sendTimeout );
 
         try
         {
-            connectionCloseFuture.get( timeout, TimeUnit.MILLISECONDS );
+            connectionCloseFuture.get( closeTimeout, TimeUnit.MILLISECONDS );
         }
         catch ( TimeoutException | ExecutionException | InterruptedException e )
         {
@@ -2464,14 +2488,26 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         if ( timeout <= 0 )
         {
             // Set a date in the far future : 100 years
-            this.timeout = 1000L * 60L * 60L * 24L * 365L * 100L;
+            setAllTimeOuts( 1000L * 60L * 60L * 24L * 365L * 100L );
         }
         else
         {
-            this.timeout = timeout;
+            setAllTimeOuts( timeout );
         }
     }
 
+    private void setAllTimeOuts( long timeout )
+    {
+        this.timeout = timeout;
+        // For compatibility.
+        // Set all timeouts to the same value to preserve previous behavior.
+        this.connectTimeout = timeout;
+        this.writeOperationTimeout = timeout;
+        this.readOperationTimeout = timeout;
+        this.closeTimeout = timeout;
+        this.sendTimeout = timeout;
+    }
+
 
     /**
      * Handle the exception we got.
@@ -3144,7 +3180,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            ModifyResponse modifyResponse = modifyFuture.get( timeout, TimeUnit.MILLISECONDS );
+            ModifyResponse modifyResponse = modifyFuture.get( writeOperationTimeout, TimeUnit.MILLISECONDS );
 
             if ( modifyResponse == null )
             {
@@ -3540,7 +3576,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            ModifyDnResponse modifyDnResponse = modifyDnFuture.get( timeout, TimeUnit.MILLISECONDS );
+            ModifyDnResponse modifyDnResponse = modifyDnFuture.get( writeOperationTimeout, TimeUnit.MILLISECONDS );
 
             if ( modifyDnResponse == null )
             {
@@ -3762,7 +3798,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            DeleteResponse delResponse = deleteFuture.get( timeout, TimeUnit.MILLISECONDS );
+            DeleteResponse delResponse = deleteFuture.get( writeOperationTimeout, TimeUnit.MILLISECONDS );
 
             if ( delResponse == null )
             {
@@ -3974,7 +4010,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         {
             // Read the response, waiting for it if not available immediately
             // Get the response, blocking
-            CompareResponse compareResponse = compareFuture.get( timeout, TimeUnit.MILLISECONDS );
+            CompareResponse compareResponse = compareFuture.get( readOperationTimeout, TimeUnit.MILLISECONDS );
 
             if ( compareResponse == null )
             {
@@ -4958,7 +4994,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
             {
                 ioSession.getFilterChain().addFirst( SSL_FILTER_KEY, sslFilter );
                 
-                boolean isSecured = handshakeFuture.get( timeout, TimeUnit.MILLISECONDS );
+                boolean isSecured = handshakeFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
                 
                 if ( !isSecured )
                 {
@@ -5076,7 +5112,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
                 writeRequest( bindRequest );
 
                 // Get the server's response, blocking
-                bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+                bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
                 if ( bindResponse == null )
                 {
@@ -5105,7 +5141,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
 
                 writeRequest( bindRequestCopy );
 
-                bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+                bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
                 if ( bindResponse == null )
                 {
@@ -5143,7 +5179,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
 
                     writeRequest( bindRequest );
 
-                    bindResponse = bindFuture.get( timeout, TimeUnit.MILLISECONDS );
+                    bindResponse = bindFuture.get( connectTimeout, TimeUnit.MILLISECONDS );
 
                     if ( bindResponse == null )
                     {
@@ -5196,7 +5232,7 @@ public class LdapNetworkConnection extends AbstractLdapConnection implements Lda
         // Send the request to the server
         WriteFuture writeFuture = ioSession.write( request );
 
-        long localTimeout = timeout;
+        long localTimeout = sendTimeout;
 
         while ( localTimeout > 0 )
         {
diff --git a/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/LdapNetworkConnectionTest.java b/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/LdapNetworkConnectionTest.java
index cddec62..9fe4710 100644
--- a/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/LdapNetworkConnectionTest.java
+++ b/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/LdapNetworkConnectionTest.java
@@ -45,7 +45,7 @@ public class LdapNetworkConnectionTest
         // index 2: expected timeout in ms
         return Stream.of(
             Arguments.of( 2000, -1, 2000, "Invalid search time limit, use connection timeout" ),
-            Arguments.of( 2000, 0, 30000, "Search time limit is 0, use config default value" ),
+            Arguments.of( 2000, 0, 2000, "Search time limit is 0, use config default value" ),
             Arguments.of( 2000, 1, 2000, "search time limit < connection timeout, use connection timeout" ),
             Arguments.of( 2000, 5, 5000, "search time limit > connection timeout, use search time limit" ),
             Arguments.of( 2000, Integer.MAX_VALUE, 2147483647000L, "Integer overflow" ),
@@ -56,7 +56,7 @@ public class LdapNetworkConnectionTest
             Arguments.of( 30000, 31, 31000, "search time limit > connection timeout, use search time limit" ),
             Arguments.of( 30000, 60, 60000, "search time limit > connection timeout, use search time limit" ),
             Arguments.of( Long.MAX_VALUE, -1, Long.MAX_VALUE, "Invalid search time limit, use connection timeout" ),
-            Arguments.of( Long.MAX_VALUE, 0, 30000, "Search time limit is 0, use config default value" ),
+            Arguments.of( Long.MAX_VALUE, 0, Long.MAX_VALUE, "Search time limit is 0, use config default value" ),
             Arguments.of( Long.MAX_VALUE, 1, Long.MAX_VALUE,
                 "search time limit < connection timeout, use connection timeout" ) );
     }