nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Smarter keepalives and a small Conversations fix.

Browse source 
The NOOP keepalives were being every n seconds no matter what the
traffic was like.  Now only being sent if no traffic has been
received after n seconds, meaning less traffic overall.  Also, there
are now two different timers, one for selected mailbox and one for
authorized state, since receiving unsolicited responses only happens
when a mailbox is selected.

Also includes a small fix in Conversations where a nullable ref was
being treated as un-nullable.
This commit is contained in:
Jim Nelson 2012-01-06 11:32:20 -08:00
parent 755d5cfa39
commit eea2b31a73
6 changed files with 99 additions and 122 deletions
Download patch file Download diff file Expand all files Collapse all files

3
src/engine/api/geary-conversations.vala
View file

@ -644,7 +644,8 @@ public class Geary.Conversations : Object {
if (conversation.get_usable_count() == 0) {
// prune all Nodes in the conversation tree
prune_nodes((Node) conversation.get_origin());
if (conversation.get_origin() != null)
prune_nodes((Node) conversation.get_origin());
// prune all orphan Nodes
Gee.Collection<ConversationNode>? orphans = conversation.get_orphans();

70
src/engine/imap/decoders/imap-noop-results.vala
View file

@ -1,70 +0,0 @@
/* Copyright 2011 Yorba Foundation
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
public class Geary.Imap.NoopResults : Geary.Imap.CommandResults {
public Gee.List<MessageNumber>? expunged { get; private set; }
/**
* -1 if "exists" result not returned by server.
*/
public int exists { get; private set; }
public MailboxAttributes? flags { get; private set; }
/**
* -1 if "recent" result not returned by server.
*/
public int recent { get; private set; }
public NoopResults(StatusResponse status_response, Gee.List<MessageNumber>? expunged, int exists,
MailboxAttributes? flags, int recent) {
base (status_response);
this.expunged = expunged;
this.exists = exists;
this.flags = flags;
this.recent = recent;
}
public static NoopResults decode(CommandResponse response) {
assert(response.is_sealed());
Gee.List<MessageNumber> expunged = new Gee.ArrayList<MessageNumber>();
MailboxAttributes? flags = null;
int exists = -1;
int recent = -1;
foreach (ServerData data in response.server_data) {
UnsolicitedServerData? unsolicited = UnsolicitedServerData.from_server_data(data);
if (unsolicited == null) {
message("NOOP server data \"%s\" unrecognized", data.to_string());
continue;
}
if (unsolicited.exists >= 0)
exists = unsolicited.exists;
if (unsolicited.recent >= 0)
recent = unsolicited.recent;
if (unsolicited.flags != null)
flags = unsolicited.flags;
if (unsolicited.expunge != null)
expunged.add(unsolicited.expunge);
}
return new NoopResults(response.status_response, (expunged.size > 0) ? expunged : null,
exists, (flags != null && flags.size > 0) ? flags : null, recent);
}
public bool has_exists() {
return exists >= 0;
}
public bool has_recent() {
return recent >= 0;
}
}

20
src/engine/imap/transport/imap-client-session-manager.vala
View file

@ -6,7 +6,6 @@
public class Geary.Imap.ClientSessionManager {
private const int MIN_POOL_SIZE = 2;
private const int SELECTED_KEEPALIVE_SEC = 30;
private Endpoint endpoint;
private Credentials credentials;
@ -14,8 +13,8 @@ public class Geary.Imap.ClientSessionManager {
private Geary.NonblockingMutex sessions_mutex = new Geary.NonblockingMutex();
private Gee.HashSet<SelectedContext> examined_contexts = new Gee.HashSet<SelectedContext>();
private Gee.HashSet<SelectedContext> selected_contexts = new Gee.HashSet<SelectedContext>();
private int keepalive_sec = ClientSession.DEFAULT_KEEPALIVE_SEC;
private int selected_keepalive_sec = SELECTED_KEEPALIVE_SEC;
private int unselected_keepalive_sec = ClientSession.DEFAULT_UNSELECTED_KEEPALIVE_SEC;
private int selected_keepalive_sec = ClientSession.DEFAULT_SELECTED_KEEPALIVE_SEC;
public signal void login_failed();
@ -39,14 +38,15 @@ public class Geary.Imap.ClientSessionManager {
}
/**
* Set to zero or negative value if keepalives should be disabled. (This is not recommended.)
* Set to zero or negative value if keepalives should be disabled when a connection has not
* selected a mailbox. (This is not recommended.)
*
* This only affects newly created sessions or sessions leaving the selected/examined state
* and returning to an authorized state.
*/
public void set_keepalive(int keepalive_sec) {
public void set_unselected_keepalive(int unselected_keepalive_sec) {
// set for future connections
this.keepalive_sec = keepalive_sec;
this.unselected_keepalive_sec = unselected_keepalive_sec;
}
/**
@ -179,10 +179,8 @@ public class Geary.Imap.ClientSessionManager {
bool removed = contexts.remove(context);
assert(removed);
if (context.session != null) {
if (context.session != null)
context.session.close_mailbox_async.begin();
context.session.enable_keepalives(keepalive_sec);
}
}
// This should only be called when sessions_mutex is locked.
@ -195,7 +193,7 @@ public class Geary.Imap.ClientSessionManager {
yield new_session.login_async(credentials, cancellable);
// do this after logging in
new_session.enable_keepalives(keepalive_sec);
new_session.enable_keepalives(selected_keepalive_sec, unselected_keepalive_sec);
// since "disconnected" is used to remove the ClientSession from the sessions list, want
// to only connect to the signal once the object has been added to the list; otherwise it's
@ -241,8 +239,6 @@ public class Geary.Imap.ClientSessionManager {
ClientSession authd = yield get_authorized_session(cancellable);
authd.enable_keepalives(selected_keepalive_sec);
results = yield authd.select_examine_async(folder, is_select, cancellable);
return authd;

108
src/engine/imap/transport/imap-client-session.vala
View file

@ -5,9 +5,13 @@
*/
public class Geary.Imap.ClientSession {
// 30 min keepalive required to maintain session; back off by 5 min for breathing room
public const int MIN_KEEPALIVE_SEC = 25 * 60;
public const int DEFAULT_KEEPALIVE_SEC = 3 * 60;
// 30 min keepalive required to maintain session; back off by 1 min for breathing room
public const int MIN_KEEPALIVE_SEC = 29 * 60;
// NOOP is only sent after this amount of time has passed since the last received
// message on the connection dependant on connection state (selected/examined vs. authorized)
public const int DEFAULT_SELECTED_KEEPALIVE_SEC = 45;
public const int DEFAULT_UNSELECTED_KEEPALIVE_SEC = MIN_KEEPALIVE_SEC;
public enum Context {
UNCONNECTED,
@ -172,6 +176,8 @@ public class Geary.Imap.ClientSession {
Hashable.hash_func, Equalable.equal_func);
private CommandResponse current_cmd_response = new CommandResponse();
private uint keepalive_id = 0;
private int selected_keepalive_secs = 0;
private int unselected_keepalive_secs = 0;
// state used only during connect and disconnect
private bool awaiting_connect_response = false;
@ -528,17 +534,13 @@ public class Geary.Imap.ClientSession {
* Although keepalives can be enabled at any time, if they're enabled and trigger sending
* a command prior to connection, error signals may be fired.
*/
public void enable_keepalives(int seconds = DEFAULT_KEEPALIVE_SEC) {
if (seconds <= 0) {
disable_keepalives();
return;
}
public void enable_keepalives(int seconds_while_selected = DEFAULT_SELECTED_KEEPALIVE_SEC,
int seconds_while_unselected = DEFAULT_UNSELECTED_KEEPALIVE_SEC) {
selected_keepalive_secs = seconds_while_selected;
unselected_keepalive_secs = seconds_while_unselected;
if (keepalive_id != 0)
Source.remove(keepalive_id);
keepalive_id = Timeout.add_seconds(seconds, on_keepalive);
// schedule one now, although will be rescheduled if traffic is received before it fires
schedule_keepalive();
}
/**
@ -554,41 +556,59 @@ public class Geary.Imap.ClientSession {
return true;
}
private void schedule_keepalive() {
// if old one was scheduled, unschedule and schedule anew
if (keepalive_id != 0)
Source.remove(keepalive_id);
int seconds;
switch (get_context(null)) {
case Context.UNCONNECTED:
return;
case Context.IN_PROGRESS:
case Context.EXAMINED:
case Context.SELECTED:
seconds = selected_keepalive_secs;
break;
case Context.UNAUTHORIZED:
case Context.AUTHORIZED:
default:
seconds = unselected_keepalive_secs;
break;
}
// Possible to not have keepalives in one state but in another, or for neither
//
// Yes, we allow keepalive to be set to 1 second. It's their dime.
if (seconds > 0)
keepalive_id = Timeout.add_seconds(seconds, on_keepalive);
}
private bool on_keepalive() {
send_command_async.begin(new NoopCommand(), null, on_keepalive_completed);
return true;
// Reschedule to reflect current connection state, although will be rescheduled again if
// traffic is received
keepalive_id = 0;
schedule_keepalive();
return false;
}
private void on_keepalive_completed(Object? source, AsyncResult result) {
NoopResults results;
CommandResponse response;
try {
results = NoopResults.decode(send_command_async.end(result));
response = send_command_async.end(result);
} catch (Error err) {
message("Keepalive error: %s", err.message);
debug("Keepalive error: %s", err.message);
return;
}
if (results.status_response.status != Status.OK) {
debug("Keepalive failed: %s", results.status_response.to_string());
return;
}
if (results.expunged != null) {
foreach (MessageNumber msg in results.expunged)
unsolicited_expunged(msg);
}
if (results.has_exists())
unsolicited_exists(results.exists);
if (results.has_recent())
unsolicited_recent(results.recent);
if (results.flags != null)
unsolicited_flags(results.flags);
if (response.status_response.status != Status.OK)
debug("Keepalive failed: %s", response.status_response.to_string());
}
//
@ -616,9 +636,12 @@ public class Geary.Imap.ClientSession {
if (params.err != null)
throw params.err;
// look for unsolicited server data and signal all that are found ... since SELECT/EXAMINE
// Look for unsolicited server data and signal all that are found ... since SELECT/EXAMINE
// aren't allowed here, don't need to check for them (because their fields aren't considered
// unsolicited)
// unsolicited). Note that this also captures NOOP's responses; although not exactly
// unsolicited in one sense, the NOOP command is merely the transport vehicle to allow the
// server to deliver unsolicited messages when no other response is available to piggyback
// upon.
//
// Note that EXPUNGE returns *EXPUNGED* results, not *EXPUNGE*, which is what the unsolicited
// version is.
@ -1093,6 +1116,9 @@ public class Geary.Imap.ClientSession {
}
private void on_received_status_response(StatusResponse status_response) {
// reschedule keepalive, now that traffic has been seen
schedule_keepalive();
assert(!current_cmd_response.is_sealed());
current_cmd_response.seal(status_response);
assert(current_cmd_response.is_sealed());
@ -1114,6 +1140,9 @@ public class Geary.Imap.ClientSession {
}
private void on_received_server_data(ServerData server_data) {
// reschedule keepalive, now that traffic has been seen
schedule_keepalive();
// The first response from the server is an untagged status response, which is considered
// ServerData in our model. This captures that and treats it as such.
if (awaiting_connect_response) {
@ -1129,6 +1158,9 @@ public class Geary.Imap.ClientSession {
}
private void on_received_bad_response(RootParameters root, ImapError err) {
// reschedule keepalive, now that traffic has been seen
schedule_keepalive();
debug("Received bad response %s: %s", root.to_string(), err.message);
}

18
src/engine/util/util-numeric.vala Executable file
View file

@ -0,0 +1,18 @@
/* Copyright 2012 Yorba Foundation
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
namespace Geary.Numeric {
public inline int int_floor(int value, int floor) {
return (value >= floor) ? value : floor;
}
public inline int int_ceiling(int value, int ceiling) {
return (value <= ceiling) ? value : ceiling;
}
}

2
src/wscript
View file

@ -53,7 +53,6 @@ def build(bld):
'../engine/imap/decoders/imap-fetch-data-decoder.vala',
'../engine/imap/decoders/imap-fetch-results.vala',
'../engine/imap/decoders/imap-list-results.vala',
'../engine/imap/decoders/imap-noop-results.vala',
'../engine/imap/decoders/imap-select-examine-results.vala',
'../engine/imap/decoders/imap-status-results.vala',
'../engine/imap/imap-error.vala',
@ -146,6 +145,7 @@ def build(bld):
'../engine/util/util-inet.vala',
'../engine/util/util-interfaces.vala',
'../engine/util/util-memory.vala',
'../engine/util/util-numeric.vala',
'../engine/util/util-reference-semantics.vala',
'../engine/util/util-scheduler.vala',
'../engine/util/util-stream.vala',