[geary/wip/cmd-timeout] Back-off algorithm for IMAP command timeout



commit 5a6fb499916b8fa2d088ecbca5af40ceb2efd34d
Author: Jim Nelson <jim yorba org>
Date:   Tue Apr 15 12:47:37 2014 -0700

    Back-off algorithm for IMAP command timeout

 src/console/main.vala                              |    3 +-
 src/engine/api/geary-account-information.vala      |    6 ++-
 src/engine/api/geary-endpoint.vala                 |   52 ++++++++++++++++++--
 .../gmail/imap-engine-gmail-account.vala           |    6 ++-
 .../outlook/imap-engine-outlook-account.vala       |    6 ++-
 .../yahoo/imap-engine-yahoo-account.vala           |    6 ++-
 .../imap/transport/imap-client-connection.vala     |   23 +++------
 src/engine/smtp/smtp-client-connection.vala        |    5 ++
 src/mailer/main.vala                               |    6 ++-
 9 files changed, 83 insertions(+), 30 deletions(-)
---
diff --git a/src/console/main.vala b/src/console/main.vala
index fdcfcae..1b2f3b1 100644
--- a/src/console/main.vala
+++ b/src/console/main.vala
@@ -312,7 +312,8 @@ class ImapConsole : Gtk.Window {
         
         cx = new Geary.Imap.ClientConnection(
             new Geary.Endpoint(args[0], Geary.Imap.ClientConnection.DEFAULT_PORT_SSL,
-                flags, Geary.Imap.ClientConnection.DEFAULT_TIMEOUT_SEC));
+                flags, Geary.Imap.ClientConnection.DEFAULT_TIMEOUT_SEC,
+                Geary.Imap.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC));
         
         cx.sent_command.connect(on_sent_command);
         cx.received_status_response.connect(on_received_status_response);
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index 0c0e4e9..8c26568 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -456,7 +456,8 @@ public class Geary.AccountInformation : BaseObject {
                     imap_flags |= Endpoint.Flags.STARTTLS;
                 
                 return new Endpoint(default_imap_server_host, default_imap_server_port,
-                    imap_flags, Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
+                    imap_flags, Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC,
+                    Imap.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
             
             default:
                 assert_not_reached();
@@ -482,7 +483,8 @@ public class Geary.AccountInformation : BaseObject {
                     smtp_flags |= Endpoint.Flags.STARTTLS;
                 
                 return new Endpoint(default_smtp_server_host, default_smtp_server_port,
-                    smtp_flags, Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+                    smtp_flags, Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC,
+                    Smtp.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
             
             default:
                 assert_not_reached();
diff --git a/src/engine/api/geary-endpoint.vala b/src/engine/api/geary-endpoint.vala
index bf0ab51..4830b83 100644
--- a/src/engine/api/geary-endpoint.vala
+++ b/src/engine/api/geary-endpoint.vala
@@ -10,6 +10,8 @@
  */
 
 public class Geary.Endpoint : BaseObject {
+    public const uint DEFAULT_COMMAND_TIMEOUT_SEC = 15;
+    
     [Flags]
     public enum Flags {
         NONE = 0,
@@ -34,9 +36,26 @@ public class Geary.Endpoint : BaseObject {
     
     public NetworkAddress remote_address { get; private set; }
     public Flags flags { get; private set; }
-    public uint timeout_sec { get; private set; }
+    public uint connection_timeout_sec { get; private set; }
     public TlsCertificateFlags tls_validation_flags { get; set; default = TlsCertificateFlags.VALIDATE_ALL; }
     public bool force_ssl3 { get; set; default = false; }
+    /**
+     * The baseline command timeout in seconds, to be determined by the connection layer or
+     * protocol specifications.
+     *
+     * @see command_timeout_sec
+     */
+    public uint initial_command_timeout_sec { get; set; default = DEFAULT_COMMAND_TIMEOUT_SEC; }
+    /**
+     * Not set or enforced by { link Endpoint}, this is persistent state that can be used by the
+     * connection layer to track transactional (versus connection-based) timeouts.
+     *
+     * This cannot be changed directly, use { link increment_command_timeout} and
+     * { link reset_command_timeout} to update.
+     *
+     * @see initial_command_timeout_sec
+     */
+    public uint command_timeout_sec { get; private set; }
     
     public bool is_ssl { get {
         return flags.is_all_set(Flags.SSL);
@@ -48,10 +67,13 @@ public class Geary.Endpoint : BaseObject {
     
     private SocketClient? socket_client = null;
     
-    public Endpoint(string host_specifier, uint16 default_port, Flags flags, uint timeout_sec) {
+    public Endpoint(string host_specifier, uint16 default_port, Flags flags, uint connection_timeout_sec,
+        uint command_timeout_sec) {
         this.remote_address = new NetworkAddress(host_specifier, default_port);
         this.flags = flags;
-        this.timeout_sec = timeout_sec;
+        this.connection_timeout_sec = connection_timeout_sec;
+        initial_command_timeout_sec = command_timeout_sec;
+        this.command_timeout_sec = initial_command_timeout_sec;
     }
     
     public SocketClient get_socket_client() {
@@ -66,7 +88,7 @@ public class Geary.Endpoint : BaseObject {
             socket_client.event.connect(on_socket_client_event);
         }
 
-        socket_client.set_timeout(timeout_sec);
+        socket_client.set_timeout(connection_timeout_sec);
 
         return socket_client;
     }
@@ -192,6 +214,28 @@ public class Geary.Endpoint : BaseObject {
         return AttemptStarttls.YES;
     }
     
+    /**
+     * Increment the { link command_timeout_sec}, generally due to a transaction or request taking
+     * too long, possibly due to line problems.
+     *
+     * @returns Updated command_timeout_sec
+     */
+    public uint increment_command_timeout_sec(uint increment_sec, uint max_sec) {
+        return command_timeout_sec = Numeric.uint_ceiling(command_timeout_sec + increment_sec, max_sec);
+    }
+    
+    /**
+     * Reset the { link command_timeout_sec}, generally if some indication is made that network
+     * congestion has subsided or network conditions have changed.
+     *
+     * This resets command_timeout_sec to { link initial_command_timeout_sec}.
+     *
+     * @returns Updated command_timeout_sec, i.e. initial_command_timeout_sec
+     */
+    public uint reset_command_timeout_sec() {
+        return command_timeout_sec = initial_command_timeout_sec;
+    }
+    
     public string to_string() {
         return "%s/default:%u".printf(remote_address.hostname, remote_address.port);
     }
diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala 
b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala
index 9ad9a29..439dce0 100644
--- a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala
+++ b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala
@@ -12,7 +12,8 @@ private class Geary.ImapEngine.GmailAccount : Geary.ImapEngine.GenericAccount {
                 "imap.gmail.com",
                 Imap.ClientConnection.DEFAULT_PORT_SSL,
                 Geary.Endpoint.Flags.SSL | Geary.Endpoint.Flags.GRACEFUL_DISCONNECT,
-                Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
+                Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC,
+                Imap.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
         }
         
         return _imap_endpoint;
@@ -25,7 +26,8 @@ private class Geary.ImapEngine.GmailAccount : Geary.ImapEngine.GenericAccount {
                 "smtp.gmail.com",
                 Smtp.ClientConnection.DEFAULT_PORT_SSL,
                 Geary.Endpoint.Flags.SSL | Geary.Endpoint.Flags.GRACEFUL_DISCONNECT,
-                Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+                Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC,
+                Smtp.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
         }
         
         return _smtp_endpoint;
diff --git a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala 
b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
index c40d0ea..7d8ffb4 100644
--- a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
+++ b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
@@ -12,7 +12,8 @@ private class Geary.ImapEngine.OutlookAccount : Geary.ImapEngine.GenericAccount
                 "imap-mail.outlook.com",
                 Imap.ClientConnection.DEFAULT_PORT_SSL,
                 Geary.Endpoint.Flags.SSL | Geary.Endpoint.Flags.GRACEFUL_DISCONNECT,
-                Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
+                Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC,
+                Imap.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
         }
         
         return _imap_endpoint;
@@ -25,7 +26,8 @@ private class Geary.ImapEngine.OutlookAccount : Geary.ImapEngine.GenericAccount
                 "smtp-mail.outlook.com",
                 Smtp.ClientConnection.DEFAULT_PORT_STARTTLS,
                 Geary.Endpoint.Flags.STARTTLS | Geary.Endpoint.Flags.GRACEFUL_DISCONNECT,
-                Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+                Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC,
+                Smtp.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
         }
         
         return _smtp_endpoint;
diff --git a/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala 
b/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala
index 68958e5..33ee65a 100644
--- a/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala
+++ b/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala
@@ -12,7 +12,8 @@ private class Geary.ImapEngine.YahooAccount : Geary.ImapEngine.GenericAccount {
                 "imap.mail.yahoo.com",
                 Imap.ClientConnection.DEFAULT_PORT_SSL,
                 Geary.Endpoint.Flags.SSL | Geary.Endpoint.Flags.GRACEFUL_DISCONNECT,
-                Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
+                Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC,
+                Imap.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
         }
         
         return _imap_endpoint;
@@ -25,7 +26,8 @@ private class Geary.ImapEngine.YahooAccount : Geary.ImapEngine.GenericAccount {
                 "smtp.mail.yahoo.com",
                 Smtp.ClientConnection.DEFAULT_PORT_SSL,
                 Geary.Endpoint.Flags.SSL | Geary.Endpoint.Flags.GRACEFUL_DISCONNECT,
-                Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+                Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC,
+                Smtp.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
         }
         
         return _smtp_endpoint;
diff --git a/src/engine/imap/transport/imap-client-connection.vala 
b/src/engine/imap/transport/imap-client-connection.vala
index e6dfce7..1cc00fc 100644
--- a/src/engine/imap/transport/imap-client-connection.vala
+++ b/src/engine/imap/transport/imap-client-connection.vala
@@ -25,10 +25,10 @@ public class Geary.Imap.ClientConnection : BaseObject {
     
     /**
      * The default timeout for an issued command to result in a response code from the server.
-     *
-     * @see command_timeout_sec
      */
     public const uint DEFAULT_COMMAND_TIMEOUT_SEC = 15;
+    public const uint MAX_COMMAND_TIMEOUT_SEC = 60;
+    public const uint INCREMENT_COMMAND_TIMEOUT_SEC = 5;
     
     private const int FLUSH_TIMEOUT_MSEC = 10;
     
@@ -85,16 +85,6 @@ public class Geary.Imap.ClientConnection : BaseObject {
     private static int next_cx_id = 0;
     
     /**
-     * The timeout in seconds before an uncompleted { link Command} is considered abandoned.
-     *
-     * ClientConnection does not time out the initial greeting from the server (as there's no
-     * command associated with it).  That's the responsibility of the caller.
-     *
-     * A timed-out command will result in the connection being forcibly closed.
-     */
-    public uint command_timeout_sec { get; set; default = DEFAULT_COMMAND_TIMEOUT_SEC; }
-    
-    /**
      * This identifier is used only for debugging, to differentiate connections from one another
      * in logs and debug output.
      */
@@ -755,7 +745,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
         timeout_cmd_count++;
         
         if (timeout_id == 0)
-            timeout_id = Timeout.add_seconds(command_timeout_sec, on_cmd_timeout);
+            timeout_id = Timeout.add_seconds(endpoint.command_timeout_sec, on_cmd_timeout);
     }
     
     private void cmd_completed_timeout() {
@@ -771,7 +761,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
     private void increase_timeout() {
         if (timeout_id != 0) {
             Source.remove(timeout_id);
-            timeout_id = Timeout.add_seconds(command_timeout_sec, on_cmd_timeout);
+            timeout_id = Timeout.add_seconds(endpoint.command_timeout_sec, on_cmd_timeout);
         }
     }
     
@@ -796,7 +786,10 @@ public class Geary.Imap.ClientConnection : BaseObject {
         timeout_cmd_count = 0;
         
         receive_failure(new ImapError.TIMED_OUT("No response to command(s) after %u seconds",
-            command_timeout_sec));
+            endpoint.command_timeout_sec));
+        
+        // increment after formatting error, above
+        endpoint.increment_command_timeout_sec(INCREMENT_COMMAND_TIMEOUT_SEC, MAX_COMMAND_TIMEOUT_SEC);
         
         return false;
     }
diff --git a/src/engine/smtp/smtp-client-connection.vala b/src/engine/smtp/smtp-client-connection.vala
index f629a64..347b770 100644
--- a/src/engine/smtp/smtp-client-connection.vala
+++ b/src/engine/smtp/smtp-client-connection.vala
@@ -11,6 +11,11 @@ public class Geary.Smtp.ClientConnection {
     
     public const uint DEFAULT_TIMEOUT_SEC = 60;
     
+    /**
+     * This is currently not respected by { link ClientConnection}, but added here for future use.
+     */
+    public const uint DEFAULT_COMMAND_TIMEOUT_SEC = 60;
+    
     public Geary.Smtp.Capabilities? capabilities { get; private set; default = null; }
     
     private Geary.Endpoint endpoint;
diff --git a/src/mailer/main.vala b/src/mailer/main.vala
index 88320ee..3568e8a 100644
--- a/src/mailer/main.vala
+++ b/src/mailer/main.vala
@@ -130,14 +130,16 @@ int main(string[] args) {
     if (arg_gmail) {
         endpoint = new Geary.Endpoint("smtp.gmail.com", Geary.Smtp.ClientConnection.DEFAULT_PORT_STARTTLS,
             Geary.Endpoint.Flags.STARTTLS | Geary.Endpoint.Flags.GRACEFUL_DISCONNECT,
-            Geary.Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+            Geary.Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC,
+            Geary.Smtp.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
     } else {
         Geary.Endpoint.Flags flags = Geary.Endpoint.Flags.GRACEFUL_DISCONNECT;
         if (!arg_no_tls)
             flags |= Geary.Endpoint.Flags.SSL;
         
         endpoint = new Geary.Endpoint(arg_hostname, (uint16) arg_port, flags,
-            Geary.Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+            Geary.Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC,
+            Geary.Smtp.ClientConnection.DEFAULT_COMMAND_TIMEOUT_SEC);
     }
 
     stdout.printf("Enabling debug: %s\n", arg_debug.to_string());


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]