Implement change_message_uid

Previously, assigning a new UID to a mapped IMAP or Maildir repository
was done by loading the "local" item, saving it under a new UID and
deleting the old one. This involved lots of disk activity for nothing
more than an effective file rename in Maildirs, and lots of network
usage in the MappedUID cases.

We do this on every upload from a local to a remote item, so that can
potentially be quite expensive. This patch lets backends that support it
(Maildir, MappedUID) efficiently rename the file rather than having to
read the mail content, write it out as a new file and delete the old
file. This speeds up uploads from Maildir and the MappedUID server.

Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This commit is contained in:
Sebastian Spaeth 2011-08-30 10:52:11 +02:00
parent 09ce56c594
commit de537dc09c
5 changed files with 93 additions and 37 deletions

View File

@ -16,6 +16,10 @@ New Features
Changes Changes
------- -------
* Uploading of Messages from Maildir and IMAP<->IMAP has been made more
efficient by renaming files/mapping entries, rather than actually
loading and saving the message under a new UID.
Bug Fixes Bug Fixes
--------- ---------

View File

@ -234,6 +234,15 @@ class BaseFolder(object):
for uid in uidlist: for uid in uidlist:
self.deletemessageflags(uid, flags) self.deletemessageflags(uid, flags)
def change_message_uid(self, uid, new_uid):
"""Change the message from existing uid to new_uid
If the backend supports it (IMAP does not).
:param new_uid: (optional) If given, the old UID will be changed
to a new UID. This allows backends efficient renaming of
messages if the UID has changed."""
raise NotImplementedException
def deletemessage(self, uid): def deletemessage(self, uid):
raise NotImplementedException raise NotImplementedException
@ -275,20 +284,15 @@ class BaseFolder(object):
#remained negative, no server was willing to assign us an #remained negative, no server was willing to assign us an
#UID. If newid is 0, saving succeeded, but we could not #UID. If newid is 0, saving succeeded, but we could not
#retrieve the new UID. Ignore message in this case. #retrieve the new UID. Ignore message in this case.
newuid = dstfolder.savemessage(uid, message, flags, rtime) new_uid = dstfolder.savemessage(uid, message, flags, rtime)
if new_uid > 0:
if newuid > 0: if new_uid != uid:
if newuid != uid: # Got new UID, change the local uid to match the new one.
self.change_message_uid(uid, new_uid)
statusfolder.deletemessage(uid)
# Got new UID, change the local uid. # Got new UID, change the local uid.
#TODO: Maildir could do this with a rename rather than
#load/save/del operation, IMPLEMENT a changeuid()
#function or so.
self.savemessage(newuid, message, flags, rtime)
self.deletemessage(uid)
uid = newuid
# Save uploaded status in the statusfolder # Save uploaded status in the statusfolder
statusfolder.savemessage(uid, message, flags, rtime) statusfolder.savemessage(new_uid, message, flags, rtime)
elif newuid == 0: elif newuid == 0:
# Message was stored to dstfolder, but we can't find it's UID # Message was stored to dstfolder, but we can't find it's UID
# This means we can't link current message to the one created # This means we can't link current message to the one created
@ -299,11 +303,11 @@ class BaseFolder(object):
self.deletemessage(uid) self.deletemessage(uid)
else: else:
raise OfflineImapError("Trying to save msg (uid %d) on folder " raise OfflineImapError("Trying to save msg (uid %d) on folder "
"%s returned invalid uid %d" % \ "%s returned invalid uid %d" % (uid,
(uid, dstfolder.getvisiblename(), new_uid),
dstfolder.getvisiblename(),
newuid),
OfflineImapError.ERROR.MESSAGE) OfflineImapError.ERROR.MESSAGE)
except (KeyboardInterrupt): # bubble up CTRL-C
raise
except OfflineImapError, e: except OfflineImapError, e:
if e.severity > OfflineImapError.ERROR.MESSAGE: if e.severity > OfflineImapError.ERROR.MESSAGE:
raise # buble severe errors up raise # buble severe errors up
@ -311,10 +315,9 @@ class BaseFolder(object):
except Exception, e: except Exception, e:
self.ui.error(e, "Copying message %s [acc: %s]:\n %s" %\ self.ui.error(e, "Copying message %s [acc: %s]:\n %s" %\
(uid, self.accountname, (uid, self.accountname,
traceback.format_exc())) exc_info()[2]))
raise #raise on unknown errors, so we can fix those raise #raise on unknown errors, so we can fix those
def syncmessagesto_copy(self, dstfolder, statusfolder): def syncmessagesto_copy(self, dstfolder, statusfolder):
"""Pass1: Copy locally existing messages not on the other side """Pass1: Copy locally existing messages not on the other side

View File

@ -597,8 +597,8 @@ class IMAPFolder(BaseFolder):
self.ui.debug('imap', 'savemessage: returning new UID %d' % uid) self.ui.debug('imap', 'savemessage: returning new UID %d' % uid)
return uid return uid
def savemessageflags(self, uid, flags): def savemessageflags(self, uid, flags):
"""Change a message's flags to `flags`."""
imapobj = self.imapserver.acquireconnection() imapobj = self.imapserver.acquireconnection()
try: try:
try: try:
@ -684,6 +684,14 @@ class IMAPFolder(BaseFolder):
elif operation == '-': elif operation == '-':
self.messagelist[uid]['flags'] -= flags self.messagelist[uid]['flags'] -= flags
def change_message_uid(self, uid, new_uid):
"""Change the message from existing uid to new_uid
If the backend supports it. IMAP does not and will throw errors."""
raise OfflineImapError('IMAP backend cannot change a messages UID from '
'%d to %d' % (uid, new_uid),
OfflineImapError.ERROR.MESSAGE)
def deletemessage(self, uid): def deletemessage(self, uid):
self.deletemessages_noconvert([uid]) self.deletemessages_noconvert([uid])

View File

@ -273,7 +273,7 @@ class MaildirFolder(BaseFolder):
if rtime != None: if rtime != None:
os.utime(os.path.join(tmpdir, messagename), (rtime, rtime)) os.utime(os.path.join(tmpdir, messagename), (rtime, rtime))
self.messagelist[uid] = {'flags': set(), self.messagelist[uid] = {'flags': flags,
'filename': os.path.join('tmp', messagename)} 'filename': os.path.join('tmp', messagename)}
# savemessageflags moves msg to 'cur' or 'new' as appropriate # savemessageflags moves msg to 'cur' or 'new' as appropriate
self.savemessageflags(uid, flags) self.savemessageflags(uid, flags)
@ -284,25 +284,25 @@ class MaildirFolder(BaseFolder):
return self.messagelist[uid]['flags'] return self.messagelist[uid]['flags']
def savemessageflags(self, uid, flags): def savemessageflags(self, uid, flags):
"""Sets the specified message's flags to the given set.
This function moves the message to the cur or new subdir,
depending on the Seen flag."""
# TODO: This function could be improved to only parse the
# filenames if the flags actually changed.
oldfilename = self.messagelist[uid]['filename'] oldfilename = self.messagelist[uid]['filename']
dir_prefix, newname = os.path.split(oldfilename) dir_prefix, filename = os.path.split(oldfilename)
tmpdir = os.path.join(self.getfullname(), 'tmp') # If a message has been seen, it goes into 'cur'
if 'S' in flags: dir_prefix = 'cur' if 'S' in flags else 'new'
# If a message has been seen, it goes into the cur # Strip off existing infostring (preserving small letter flags that
# directory. CR debian#152482
dir_prefix = 'cur'
else:
dir_prefix = 'new'
# Strip off existing infostring (preserving small letter flags, that
# dovecot uses) # dovecot uses)
infomatch = self.flagmatchre.search(newname) infomatch = self.flagmatchre.search(filename)
if infomatch: if infomatch:
newname = newname[:-len(infomatch.group())] #strip off filename = filename[:-len(infomatch.group())] #strip off
infostr = '%s2,%s' % (self.infosep, ''.join(sorted(flags))) infostr = '%s2,%s' % (self.infosep, ''.join(sorted(flags)))
newname += infostr filename += infostr
newfilename = os.path.join(dir_prefix, filename)
newfilename = os.path.join(dir_prefix, newname)
if (newfilename != oldfilename): if (newfilename != oldfilename):
try: try:
os.rename(os.path.join(self.getfullname(), oldfilename), os.rename(os.path.join(self.getfullname(), oldfilename),
@ -315,10 +315,26 @@ class MaildirFolder(BaseFolder):
self.messagelist[uid]['flags'] = flags self.messagelist[uid]['flags'] = flags
self.messagelist[uid]['filename'] = newfilename self.messagelist[uid]['filename'] = newfilename
# By now, the message had better not be in tmp/ land! def change_message_uid(self, uid, new_uid):
final_dir, final_name = os.path.split(self.messagelist[uid]['filename']) """Change the message from existing uid to new_uid
assert final_dir != 'tmp'
This will not update the statusfolder UID, you need to do that yourself.
:param new_uid: (optional) If given, the old UID will be changed
to a new UID. The Maildir backend can implement this as an efficient
rename."""
if not uid in self.messagelist:
raise OfflineImapError("Cannot change unknown Maildir UID %s" % uid)
if uid == new_uid: return
oldfilename = self.messagelist[uid]['filename']
dir_prefix, filename = os.path.split(oldfilename)
flags = self.getmessageflags(uid)
filename = self.new_message_filename(new_uid, flags)
os.rename(os.path.join(self.getfullname(), oldfilename),
os.path.join(self.getfullname(), dir_prefix, filename))
self.messagelist[new_uid] = self.messagelist[uid]
del self.messagelist[uid]
def deletemessage(self, uid): def deletemessage(self, uid):
"""Unlinks a message file from the Maildir. """Unlinks a message file from the Maildir.

View File

@ -221,6 +221,31 @@ class MappedIMAPFolder(IMAPFolder):
self._mb.addmessagesflags(self._uidlist(self.r2l, uidlist), self._mb.addmessagesflags(self._uidlist(self.r2l, uidlist),
flags) flags)
def change_message_uid(self, ruid, new_ruid):
"""Change the message from existing ruid to new_ruid
:param new_uid: The old remote UID will be changed to a new
UID. The UIDMaps case handles this efficiently by simply
changing the mappings file."""
if ruid not in self.r2l:
raise OfflineImapError("Cannot change unknown Maildir UID %s" % ruid,
OfflineImapError.ERROR.MESSAGE)
if ruid == new_ruid: return # sanity check shortcut
self.maplock.acquire()
try:
luid = self.r2l[ruid]
self.l2r[luid] = new_ruid
del self.r2l[ruid]
self.r2l[new_ruid] = luid
#TODO: diskl2r|r2l are a pain to sync and should be done away with
#diskl2r only contains positive UIDs, so wrap in ifs
if luid>0: self.diskl2r[luid] = new_ruid
if ruid>0: del self.diskr2l[ruid]
if new_ruid > 0: self.diskr2l[new_ruid] = luid
self._savemaps(dolock = 0)
finally:
self.maplock.release()
def _mapped_delete(self, uidlist): def _mapped_delete(self, uidlist):
self.maplock.acquire() self.maplock.acquire()
try: try: