From 860e407ba0a86be30865a77ec29c6ecacf7899a4 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 6 Feb 2014 01:39:47 -0400 Subject: defer copy and soledad writes --- src/leap/mail/imap/mailbox.py | 68 ++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 27 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index d8af0a5..84bfa54 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -447,7 +447,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): return exists = self.getMessageCount() recent = self.getRecentCount() - logger.debug("NOTIFY: there are %s messages, %s recent" % ( + logger.debug("NOTIFY (%r): there are %s messages, %s recent" % ( + self.mbox, exists, recent)) @@ -528,7 +529,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): return seq_messg @deferred_to_thread - #@profile def fetch(self, messages_asked, uid): """ Retrieve one or more messages in this mailbox. @@ -809,6 +809,44 @@ class SoledadMailbox(WithMsgFields, MBoxParser): UID of the message :type observer: Deferred """ + memstore = self._memstore + + def createCopy(result): + exist, new_fdoc, hdoc = result + if exist: + # Should we signal error on the callback? + logger.warning("Destination message already exists!") + + # XXX I'm still not clear if we should raise the + # errback. This actually rases an ugly warning + # in some muas like thunderbird. I guess the user does + # not deserve that. + observer.callback(True) + else: + mbox = self.mbox + uid_next = memstore.increment_last_soledad_uid(mbox) + new_fdoc[self.UID_KEY] = uid_next + new_fdoc[self.MBOX_KEY] = mbox + + # FIXME set recent! + + self._memstore.create_message( + self.mbox, uid_next, + MessageWrapper( + new_fdoc, hdoc.content), + observer=observer, + notify_on_disk=False) + + d = self._get_msg_copy(message) + d.addCallback(createCopy) + d.addErrback(lambda f: log.msg(f.getTraceback())) + + @deferred_to_thread + def _get_msg_copy(self, message): + """ + Get a copy of the fdoc for this message, and check whether + it already exists. + """ # XXX for clarity, this could be delegated to a # MessageCollection mixin that implements copy too, and # moved out of here. @@ -822,7 +860,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): logger.warning("Tried to copy a MSG with no fdoc") return new_fdoc = copy.deepcopy(fdoc.content) - fdoc_chash = new_fdoc[fields.CONTENT_HASH_KEY] # XXX is this hitting the db??? --- probably. @@ -830,30 +867,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): dest_fdoc = memstore.get_fdoc_from_chash( fdoc_chash, self.mbox) exist = dest_fdoc and not empty(dest_fdoc.content) - - if exist: - # Should we signal error on the callback? - logger.warning("Destination message already exists!") - - # XXX I'm still not clear if we should raise the - # errback. This actually rases an ugly warning - # in some muas like thunderbird. I guess the user does - # not deserve that. - observer.callback(True) - else: - mbox = self.mbox - uid_next = memstore.increment_last_soledad_uid(mbox) - new_fdoc[self.UID_KEY] = uid_next - new_fdoc[self.MBOX_KEY] = mbox - - # FIXME set recent! - - self._memstore.create_message( - self.mbox, uid_next, - MessageWrapper( - new_fdoc, hdoc.content), - observer=observer, - notify_on_disk=False) + return exist, new_fdoc, hdoc # convenience fun -- cgit v1.2.3 From bd83f834920709db3350c58dedd3cd2181c1b2cc Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 6 Feb 2014 02:28:54 -0400 Subject: prefetch flag docs --- src/leap/mail/imap/mailbox.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 84bfa54..f319bf0 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -90,6 +90,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): next_uid_lock = threading.Lock() + _fdoc_primed = {} + def __init__(self, mbox, soledad, memstore, rw=1): """ SoledadMailbox constructor. Needs to get passed a name, plus a @@ -129,6 +131,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): if self._memstore: self.prime_known_uids_to_memstore() self.prime_last_uid_to_memstore() + self.prime_flag_docs_to_memstore() @property def listeners(self): @@ -279,6 +282,16 @@ class SoledadMailbox(WithMsgFields, MBoxParser): known_uids = self.messages.all_soledad_uid_iter() self._memstore.set_known_uids(self.mbox, known_uids) + def prime_flag_docs_to_memstore(self): + """ + Prime memstore with all the flags documents. + """ + primed = self._fdoc_primed.get(self.mbox, False) + if not primed: + all_flag_docs = self.messages.get_all_soledad_flag_docs() + self._memstore.load_flag_docs(self.mbox, all_flag_docs) + self._fdoc_primed[self.mbox] = True + def getUIDValidity(self): """ Return the unique validity identifier for this mailbox. @@ -606,7 +619,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): messages_asked = self._bound_seq(messages_asked) seq_messg = self._filter_msg_seq(messages_asked) - all_flags = self.messages.all_flags() + all_flags = self._memstore.all_flags(self.mbox) result = ((msgid, flagsPart( msgid, all_flags.get(msgid, tuple()))) for msgid in seq_messg) return result @@ -833,7 +846,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): self._memstore.create_message( self.mbox, uid_next, MessageWrapper( - new_fdoc, hdoc.content), + new_fdoc, hdoc), observer=observer, notify_on_disk=False) @@ -860,6 +873,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): logger.warning("Tried to copy a MSG with no fdoc") return new_fdoc = copy.deepcopy(fdoc.content) + copy_hdoc = copy.deepcopy(hdoc.content) fdoc_chash = new_fdoc[fields.CONTENT_HASH_KEY] # XXX is this hitting the db??? --- probably. @@ -867,7 +881,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): dest_fdoc = memstore.get_fdoc_from_chash( fdoc_chash, self.mbox) exist = dest_fdoc and not empty(dest_fdoc.content) - return exist, new_fdoc, hdoc + return exist, new_fdoc, copy_hdoc # convenience fun -- cgit v1.2.3 From 3b6ff2133e477441eb8f6956a17be2412fa1ac7c Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 6 Feb 2014 10:27:55 -0400 Subject: do not defer fetches to thread I think this is not a good idea now that all is done in the memstore, overhead from passing the data to thread and gathering the result seems to be much higher than just retreiving the data we need from the memstore. --- src/leap/mail/imap/mailbox.py | 3 --- 1 file changed, 3 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index f319bf0..1fa0554 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -541,7 +541,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): seq_messg = set_asked.intersection(set_exist) return seq_messg - @deferred_to_thread def fetch(self, messages_asked, uid): """ Retrieve one or more messages in this mailbox. @@ -580,7 +579,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): result = ((msgid, getmsg(msgid)) for msgid in seq_messg) return result - @deferred_to_thread def fetch_flags(self, messages_asked, uid): """ A fast method to fetch all flags, tricking just the @@ -624,7 +622,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): msgid, all_flags.get(msgid, tuple()))) for msgid in seq_messg) return result - @deferred_to_thread def fetch_headers(self, messages_asked, uid): """ A fast method to fetch all headers, tricking just the -- cgit v1.2.3 From ff3a6a640fdb345449a5f9cd3379bbaefa36111e Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 6 Feb 2014 15:46:17 -0400 Subject: take recent count from memstore --- src/leap/mail/imap/mailbox.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 1fa0554..c188f91 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -559,6 +559,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :rtype: A tuple of two-tuples of message sequence numbers and LeapMessage """ + from twisted.internet import reactor # For the moment our UID is sequential, so we # can treat them all the same. # Change this to the flag that twisted expects when we @@ -577,6 +578,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): raise NotImplementedError else: result = ((msgid, getmsg(msgid)) for msgid in seq_messg) + reactor.callLater(0, self.unset_recent_flags, seq_messg) return result def fetch_flags(self, messages_asked, uid): @@ -838,6 +840,10 @@ class SoledadMailbox(WithMsgFields, MBoxParser): new_fdoc[self.UID_KEY] = uid_next new_fdoc[self.MBOX_KEY] = mbox + flags = list(new_fdoc[self.FLAGS_KEY]) + flags.append(fields.RECENT_FLAG) + new_fdoc[self.FLAGS_KEY] = flags + # FIXME set recent! self._memstore.create_message( @@ -890,12 +896,11 @@ class SoledadMailbox(WithMsgFields, MBoxParser): for doc in docs: self.messages._soledad.delete_doc(doc) - def unset_recent_flags(self, uids): + def unset_recent_flags(self, uid_seq): """ Unset Recent flag for a sequence of UIDs. """ - seq_messg = self._bound_seq(uids) - self.messages.unset_recent_flags(seq_messg) + self.messages.unset_recent_flags(uid_seq) def __repr__(self): """ -- cgit v1.2.3 From 813db4a356141592337f39f9c801203367c63193 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Fri, 7 Feb 2014 02:54:52 -0400 Subject: remove hdoc copy since it's in its own structure now --- src/leap/mail/imap/mailbox.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index c188f91..6e472ee 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -824,12 +824,12 @@ class SoledadMailbox(WithMsgFields, MBoxParser): memstore = self._memstore def createCopy(result): - exist, new_fdoc, hdoc = result + exist, new_fdoc = result if exist: # Should we signal error on the callback? logger.warning("Destination message already exists!") - # XXX I'm still not clear if we should raise the + # XXX I'm not sure if we should raise the # errback. This actually rases an ugly warning # in some muas like thunderbird. I guess the user does # not deserve that. @@ -848,8 +848,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): self._memstore.create_message( self.mbox, uid_next, - MessageWrapper( - new_fdoc, hdoc), + MessageWrapper(new_fdoc), observer=observer, notify_on_disk=False) @@ -862,6 +861,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): """ Get a copy of the fdoc for this message, and check whether it already exists. + + :return: exist, new_fdoc + :rtype: tuple """ # XXX for clarity, this could be delegated to a # MessageCollection mixin that implements copy too, and @@ -869,22 +871,16 @@ class SoledadMailbox(WithMsgFields, MBoxParser): msg = message memstore = self._memstore - # XXX should use a public api instead - fdoc = msg._fdoc - hdoc = msg._hdoc - if not fdoc: + if empty(msg.fdoc): logger.warning("Tried to copy a MSG with no fdoc") return - new_fdoc = copy.deepcopy(fdoc.content) - copy_hdoc = copy.deepcopy(hdoc.content) + new_fdoc = copy.deepcopy(msg.fdoc.content) fdoc_chash = new_fdoc[fields.CONTENT_HASH_KEY] - # XXX is this hitting the db??? --- probably. - # We should profile after the pre-fetch. dest_fdoc = memstore.get_fdoc_from_chash( fdoc_chash, self.mbox) exist = dest_fdoc and not empty(dest_fdoc.content) - return exist, new_fdoc, copy_hdoc + return exist, new_fdoc # convenience fun -- cgit v1.2.3 From 0d61ed62ed1a2b3bead50b16324d50acfe71727d Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Tue, 11 Feb 2014 01:35:35 -0400 Subject: add profile-command utility --- src/leap/mail/imap/mailbox.py | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 6e472ee..122875b 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -50,6 +50,25 @@ If the environment variable `LEAP_SKIPNOTIFY` is set, we avoid notifying clients of new messages. Use during stress tests. """ NOTIFY_NEW = not os.environ.get('LEAP_SKIPNOTIFY', False) +PROFILE_CMD = os.environ.get('LEAP_PROFILE_IMAPCMD', False) + +if PROFILE_CMD: + import time + + def _debugProfiling(result, cmdname, start): + took = (time.time() - start) * 1000 + log.msg("CMD " + cmdname + " TOOK: " + str(took) + " msec") + return result + + def do_profile_cmd(d, name): + """ + Add the profiling debug to the passed callback. + :param d: deferred + :param name: name of the command + :type name: str + """ + d.addCallback(_debugProfiling, name, time.time()) + d.addErrback(lambda f: log.msg(f.getTraceback())) class SoledadMailbox(WithMsgFields, MBoxParser): @@ -133,6 +152,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): self.prime_last_uid_to_memstore() self.prime_flag_docs_to_memstore() + from twisted.internet import reactor + self.reactor = reactor + @property def listeners(self): """ @@ -711,14 +733,15 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :raise ReadOnlyMailbox: Raised if this mailbox is not open for read-write. """ - from twisted.internet import reactor if not self.isWriteable(): log.msg('read only mailbox!') raise imap4.ReadOnlyMailbox d = defer.Deferred() - deferLater(reactor, 0, self._do_store, messages_asked, flags, - mode, uid, d) + self.reactor.callLater(0, self._do_store, messages_asked, flags, + mode, uid, d) + if PROFILE_CMD: + do_profile_cmd(d, "STORE") return d def _do_store(self, messages_asked, flags, mode, uid, observer): @@ -797,15 +820,11 @@ class SoledadMailbox(WithMsgFields, MBoxParser): uid when the copy succeed. :rtype: Deferred """ - from twisted.internet import reactor - d = defer.Deferred() - # XXX this should not happen ... track it down, - # probably to FETCH... - if message is None: - log.msg("BUG: COPY found a None in passed message") - d.callback(None) - deferLater(reactor, 0, self._do_copy, message, d) + if PROFILE_CMD: + do_profile_cmd(d, "COPY") + d.addCallback(lambda r: self.reactor.callLater(0, self.notify_new)) + deferLater(self.reactor, 0, self._do_copy, message, d) return d def _do_copy(self, message, observer): -- cgit v1.2.3 From 912873a939214bc805fb398bc5a2fe1949fe34d6 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Tue, 11 Feb 2014 01:37:23 -0400 Subject: do not get last_uid from the set of soledad messages but always from the counter instead. once assigned, the uid must never be reused, unless the uidvalidity mailbox value changes. doing otherwise will cause messages not to be shown until next session. Also, renamed get_mbox method for clarity. --- src/leap/mail/imap/mailbox.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 122875b..018f88e 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -108,6 +108,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): _listeners = defaultdict(set) next_uid_lock = threading.Lock() + last_uid_lock = threading.Lock() _fdoc_primed = {} @@ -196,7 +197,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): self.listeners.remove(listener) # TODO move completely to soledadstore, under memstore reponsibility. - def _get_mbox(self): + def _get_mbox_doc(self): """ Return mailbox document. @@ -220,7 +221,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :returns: tuple of flags for this mailbox :rtype: tuple of str """ - mbox = self._get_mbox() + mbox = self._get_mbox_doc() if not mbox: return None flags = mbox.content.get(self.FLAGS_KEY, []) @@ -235,7 +236,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): """ leap_assert(isinstance(flags, tuple), "flags expected to be a tuple") - mbox = self._get_mbox() + mbox = self._get_mbox_doc() if not mbox: return None mbox.content[self.FLAGS_KEY] = map(str, flags) @@ -250,7 +251,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :return: True if the mailbox is closed :rtype: bool """ - mbox = self._get_mbox() + mbox = self._get_mbox_doc() return mbox.content.get(self.CLOSED_KEY, False) def _set_closed(self, closed): @@ -261,7 +262,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :type closed: bool """ leap_assert(isinstance(closed, bool), "closed needs to be boolean") - mbox = self._get_mbox() + mbox = self._get_mbox_doc() mbox.content[self.CLOSED_KEY] = closed self._soledad.put_doc(mbox) @@ -290,8 +291,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): """ Prime memstore with last_uid value """ - set_exist = set(self.messages.all_uid_iter()) - last = max(set_exist) if set_exist else 0 + mbox = self._get_mbox_doc() + last = mbox.content.get('lastuid', 0) logger.info("Priming Soledad last_uid to %s" % (last,)) self._memstore.set_last_soledad_uid(self.mbox, last) @@ -321,7 +322,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :return: unique validity identifier :rtype: int """ - mbox = self._get_mbox() + mbox = self._get_mbox_doc() return mbox.content.get(self.CREATED_KEY, 1) def getUID(self, message): @@ -483,12 +484,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): exists = self.getMessageCount() recent = self.getRecentCount() logger.debug("NOTIFY (%r): there are %s messages, %s recent" % ( - self.mbox, - exists, - recent)) + self.mbox, exists, recent)) for l in self.listeners: - logger.debug('notifying...') l.newMessages(exists, recent) # commands, do not rename methods @@ -507,7 +505,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): # we should postpone the removal # XXX move to memory store?? - self._soledad.delete_doc(self._get_mbox()) + self._soledad.delete_doc(self._get_mbox_doc()) def _close_cb(self, result): self.closed = True @@ -756,7 +754,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :type observer: deferred """ # XXX implement also sequence (uid = 0) - # XXX we should prevent cclient from setting Recent flag? + # XXX we should prevent client from setting Recent flag? leap_assert(not isinstance(flags, basestring), "flags cannot be a string") flags = tuple(flags) -- cgit v1.2.3 From 5e96a249ab541cedcc79e4e60f46cd4a187e47fb Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Tue, 11 Feb 2014 01:39:43 -0400 Subject: fix repeated recent flag --- src/leap/mail/imap/mailbox.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 018f88e..fa97512 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -854,12 +854,13 @@ class SoledadMailbox(WithMsgFields, MBoxParser): else: mbox = self.mbox uid_next = memstore.increment_last_soledad_uid(mbox) + new_fdoc[self.UID_KEY] = uid_next new_fdoc[self.MBOX_KEY] = mbox flags = list(new_fdoc[self.FLAGS_KEY]) flags.append(fields.RECENT_FLAG) - new_fdoc[self.FLAGS_KEY] = flags + new_fdoc[self.FLAGS_KEY] = tuple(set(flags)) # FIXME set recent! @@ -896,7 +897,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): dest_fdoc = memstore.get_fdoc_from_chash( fdoc_chash, self.mbox) - exist = dest_fdoc and not empty(dest_fdoc.content) + + exist = not empty(dest_fdoc) return exist, new_fdoc # convenience fun -- cgit v1.2.3 From fd9c8c2e3c88476b90805b689f6914fe5eac16df Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Tue, 11 Feb 2014 02:53:28 -0400 Subject: defer fetch to thread also, dispatch query for all headers to its own method. --- src/leap/mail/imap/mailbox.py | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index fa97512..21f0554 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -211,6 +211,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): fields.TYPE_MBOX_VAL, self.mbox) if query: return query.pop() + else: + logger.error("Could not find mbox document for %r" % + (self.mbox,)) except Exception as exc: logger.exception("Unhandled error %r" % exc) @@ -576,10 +579,30 @@ class SoledadMailbox(WithMsgFields, MBoxParser): otherwise. :type uid: bool + :rtype: deferred + """ + d = defer.Deferred() + self.reactor.callInThread(self._do_fetch, messages_asked, uid, d) + if PROFILE_CMD: + do_profile_cmd(d, "FETCH") + return d + + # called in thread + def _do_fetch(self, messages_asked, uid, d): + """ + :param messages_asked: IDs of the messages to retrieve information + about + :type messages_asked: MessageSet + + :param uid: If true, the IDs are UIDs. They are message sequence IDs + otherwise. + :type uid: bool + :param d: deferred whose callback will be called with result. + :type d: Deferred + :rtype: A tuple of two-tuples of message sequence numbers and LeapMessage """ - from twisted.internet import reactor # For the moment our UID is sequential, so we # can treat them all the same. # Change this to the flag that twisted expects when we @@ -597,9 +620,11 @@ class SoledadMailbox(WithMsgFields, MBoxParser): logger.debug("Getting msg by index: INEFFICIENT call!") raise NotImplementedError else: - result = ((msgid, getmsg(msgid)) for msgid in seq_messg) - reactor.callLater(0, self.unset_recent_flags, seq_messg) - return result + got_msg = [(msgid, getmsg(msgid)) for msgid in seq_messg] + result = ((msgid, msg) for msgid, msg in got_msg + if msg is not None) + self.reactor.callLater(0, self.unset_recent_flags, seq_messg) + self.reactor.callFromThread(d.callback, result) def fetch_flags(self, messages_asked, uid): """ @@ -668,6 +693,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): MessagePart. :rtype: tuple """ + # TODO how often is thunderbird doing this? + class headersPart(object): def __init__(self, uid, headers): self.uid = uid @@ -685,10 +712,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): messages_asked = self._bound_seq(messages_asked) seq_messg = self._filter_msg_seq(messages_asked) - all_chash = self.messages.all_flags_chash() all_headers = self.messages.all_headers() result = ((msgid, headersPart( - msgid, all_headers.get(all_chash.get(msgid, 'nil'), {}))) + msgid, all_headers.get(msgid, {}))) for msgid in seq_messg) return result -- cgit v1.2.3 From f6566fe83c93625b918664526e8858f7be667354 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Tue, 11 Feb 2014 16:20:26 -0400 Subject: defer appends too and cut some more time by firing the callback as soon as we've got an UID. --- src/leap/mail/imap/mailbox.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 21f0554..7083316 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -111,6 +111,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): last_uid_lock = threading.Lock() _fdoc_primed = {} + _last_uid_primed = {} def __init__(self, mbox, soledad, memstore, rw=1): """ @@ -294,10 +295,13 @@ class SoledadMailbox(WithMsgFields, MBoxParser): """ Prime memstore with last_uid value """ - mbox = self._get_mbox_doc() - last = mbox.content.get('lastuid', 0) - logger.info("Priming Soledad last_uid to %s" % (last,)) - self._memstore.set_last_soledad_uid(self.mbox, last) + primed = self._last_uid_primed.get(self.mbox, False) + if not primed: + mbox = self._get_mbox_doc() + last = mbox.content.get('lastuid', 0) + logger.info("Priming Soledad last_uid to %s" % (last,)) + self._memstore.set_last_soledad_uid(self.mbox, last) + self._last_uid_primed[self.mbox] = True def prime_known_uids_to_memstore(self): """ @@ -459,6 +463,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): flags = tuple(str(flag) for flag in flags) d = self._do_add_message(message, flags=flags, date=date) + if PROFILE_CMD: + do_profile_cmd(d, "APPEND") + # XXX should notify here probably return d def _do_add_message(self, message, flags, date): @@ -467,13 +474,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): Invoked from addMessage. """ d = self.messages.add_msg(message, flags=flags, date=date) - # XXX Removing notify temporarily. - # This is interfering with imaptest results. I'm not clear if it's - # because we clutter the logging or because the set of listeners is - # ever-growing. We should come up with some smart way of dealing with - # it, or maybe just disabling it using an environmental variable since - # we will only have just a few listeners in the regular desktop case. - #d.addCallback(self.notify_new) return d def notify_new(self, *args): -- cgit v1.2.3 From 54114126d0b8e16784b67ee972e549e5c152c9d0 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Wed, 12 Feb 2014 12:37:31 -0400 Subject: purge empty fdocs on select --- src/leap/mail/imap/mailbox.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 7083316..087780f 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -157,6 +157,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): from twisted.internet import reactor self.reactor = reactor + # purge memstore from empty fdocs. + self._memstore.purge_fdoc_store(mbox) + @property def listeners(self): """ -- cgit v1.2.3 From b520a60d0e48f36dcebe03d19b65839afc460fe9 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Wed, 12 Feb 2014 12:39:33 -0400 Subject: move mbox-doc handling to soledadstore, and lock it --- src/leap/mail/imap/mailbox.py | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 087780f..d18bc9a 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -200,7 +200,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): """ self.listeners.remove(listener) - # TODO move completely to soledadstore, under memstore reponsibility. def _get_mbox_doc(self): """ Return mailbox document. @@ -209,17 +208,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): the query failed. :rtype: SoledadDocument or None. """ - try: - query = self._soledad.get_from_index( - fields.TYPE_MBOX_IDX, - fields.TYPE_MBOX_VAL, self.mbox) - if query: - return query.pop() - else: - logger.error("Could not find mbox document for %r" % - (self.mbox,)) - except Exception as exc: - logger.exception("Unhandled error %r" % exc) + return self._memstore.get_mbox_doc(self.mbox) def getFlags(self): """ @@ -234,6 +223,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): flags = mbox.content.get(self.FLAGS_KEY, []) return map(str, flags) + # XXX move to memstore->soledadstore def setFlags(self, flags): """ Sets flags for this mailbox. @@ -258,8 +248,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :return: True if the mailbox is closed :rtype: bool """ - mbox = self._get_mbox_doc() - return mbox.content.get(self.CLOSED_KEY, False) + return self._memstore.get_mbox_closed(self.mbox) def _set_closed(self, closed): """ @@ -268,10 +257,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :param closed: the state to be set :type closed: bool """ - leap_assert(isinstance(closed, bool), "closed needs to be boolean") - mbox = self._get_mbox_doc() - mbox.content[self.CLOSED_KEY] = closed - self._soledad.put_doc(mbox) + self._memstore.set_mbox_closed(self.mbox, closed) closed = property( _get_closed, _set_closed, doc="Closed attribute.") -- cgit v1.2.3 From ac4c70f0be36c985e16e3f4ec0a38ef6f8d48166 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Wed, 12 Feb 2014 12:42:02 -0400 Subject: remove all refs during removal, and protect from empty docs --- src/leap/mail/imap/mailbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index d18bc9a..045de82 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -609,7 +609,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): logger.debug("Getting msg by index: INEFFICIENT call!") raise NotImplementedError else: - got_msg = [(msgid, getmsg(msgid)) for msgid in seq_messg] + got_msg = ((msgid, getmsg(msgid)) for msgid in seq_messg) result = ((msgid, msg) for msgid, msg in got_msg if msg is not None) self.reactor.callLater(0, self.unset_recent_flags, seq_messg) -- cgit v1.2.3 From 45733a231128cc06e123f352b4eb9886d6820878 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Fri, 14 Feb 2014 12:41:58 -0400 Subject: docstring fixes --- src/leap/mail/imap/mailbox.py | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 045de82..d55cae6 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -895,6 +895,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): Get a copy of the fdoc for this message, and check whether it already exists. + :param message: an IMessage implementor + :type message: LeapMessage :return: exist, new_fdoc :rtype: tuple """ -- cgit v1.2.3 From f72fd69e637aa252f64c0e0ec8f7e3ebeb2290bb Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 17 Feb 2014 10:50:31 -0400 Subject: speedup mailbox select --- src/leap/mail/imap/mailbox.py | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) (limited to 'src/leap/mail/imap/mailbox.py') diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index d55cae6..57505f0 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -110,8 +110,10 @@ class SoledadMailbox(WithMsgFields, MBoxParser): next_uid_lock = threading.Lock() last_uid_lock = threading.Lock() + # TODO unify all the `primed` dicts _fdoc_primed = {} _last_uid_primed = {} + _known_uids_primed = {} def __init__(self, mbox, soledad, memstore, rw=1): """ @@ -130,6 +132,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :param rw: read-and-write flag for this mailbox :type rw: int """ + logger.debug("Initializing mailbox %r" % (mbox,)) leap_assert(mbox, "Need a mailbox name to initialize") leap_assert(soledad, "Need a soledad instance to initialize") @@ -146,6 +149,10 @@ class SoledadMailbox(WithMsgFields, MBoxParser): self.messages = MessageCollection( mbox=mbox, soledad=self._soledad, memstore=self._memstore) + # XXX careful with this get/set (it would be + # hitting db unconditionally, move to memstore too) + # Now it's returning a fixed amount of flags from mem + # as a workaround. if not self.getFlags(): self.setFlags(self.INIT_FLAGS) @@ -159,6 +166,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): # purge memstore from empty fdocs. self._memstore.purge_fdoc_store(mbox) + logger.debug("DONE initializing mailbox %r" % (mbox,)) @property def listeners(self): @@ -217,10 +225,18 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :returns: tuple of flags for this mailbox :rtype: tuple of str """ - mbox = self._get_mbox_doc() - if not mbox: - return None - flags = mbox.content.get(self.FLAGS_KEY, []) + flags = self.INIT_FLAGS + + # XXX returning fixed flags always + # Since I have not found a case where the client + # wants to modify this, as a way of speeding up + # selects. To do it right, we probably should keep + # track of the set of all flags used by msgs + # in this mailbox. Does it matter? + #mbox = self._get_mbox_doc() + #if not mbox: + #return None + #flags = mbox.content.get(self.FLAGS_KEY, []) return map(str, flags) # XXX move to memstore->soledadstore @@ -237,6 +253,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): if not mbox: return None mbox.content[self.FLAGS_KEY] = map(str, flags) + logger.debug("Writing mbox document for %r to Soledad" + % (self.mbox,)) self._soledad.put_doc(mbox) # XXX SHOULD BETTER IMPLEMENT ADD_FLAG, REMOVE_FLAG. @@ -298,8 +316,11 @@ class SoledadMailbox(WithMsgFields, MBoxParser): We do this to be able to filter the requests efficiently. """ - known_uids = self.messages.all_soledad_uid_iter() - self._memstore.set_known_uids(self.mbox, known_uids) + primed = self._known_uids_primed.get(self.mbox, False) + if not primed: + known_uids = self.messages.all_soledad_uid_iter() + self._memstore.set_known_uids(self.mbox, known_uids) + self._known_uids_primed[self.mbox] = True def prime_flag_docs_to_memstore(self): """ @@ -465,6 +486,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): d = self.messages.add_msg(message, flags=flags, date=date) return d + @deferred_to_thread def notify_new(self, *args): """ Notify of new messages to all the listeners. @@ -836,7 +858,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): d = defer.Deferred() if PROFILE_CMD: do_profile_cmd(d, "COPY") - d.addCallback(lambda r: self.reactor.callLater(0, self.notify_new)) deferLater(self.reactor, 0, self._do_copy, message, d) return d @@ -863,9 +884,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): # XXX I'm not sure if we should raise the # errback. This actually rases an ugly warning - # in some muas like thunderbird. I guess the user does - # not deserve that. - observer.callback(True) + # in some muas like thunderbird. + # UID 0 seems a good convention for no uid. + observer.callback(0) else: mbox = self.mbox uid_next = memstore.increment_last_soledad_uid(mbox) -- cgit v1.2.3