Mercurial > moin > 2.0

changeset 208:321898d8273f

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
pycdb: add context manager. fs/hg storage: only use pycdb, not cdb. When testing moin2 with pypy 1.5, I had crashes due to "too many files open". I found out that cdb.init() opens the cdb file, but it is never explicitly closed. Pypy seems to be a bit more lazy with garbage collecting than CPython, so it quickly runs out of file handles. I added the context manager, so it can be used with "with" statement, so the cdb file gets closed automatically, when it exits the context. I didn't find how to do this with non-pure-python cdb code, thus I removed it, moin2 will only use the bundled pycdb code for now. I also added a context manager for cdbmake, so it calls finish() automatically.
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Sat, 30 Apr 2011 23:18:40 +0200
parents 2652fa715b44
children 76a6dd2fae57
files MoinMoin/storage/backends/fs.py MoinMoin/storage/backends/hg.py MoinMoin/util/pycdb.py
diffstat 3 files changed, 111 insertions(+), 88 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/storage/backends/fs.py	Sat Apr 30 21:28:25 2011 +0200
+++ b/MoinMoin/storage/backends/fs.py	Sat Apr 30 23:18:40 2011 +0200
@@ -16,10 +16,7 @@
 from MoinMoin import log
 logging = log.getLogger(__name__)
 
-try:
-    import cdb
-except ImportError:
-    from MoinMoin.util import pycdb as cdb
+from MoinMoin.util import pycdb as cdb
 
 from MoinMoin.util.lock import ExclusiveLock
 from MoinMoin.util import filesys
@@ -86,8 +83,8 @@
         call this under the name-mapping.lock.
         """
         if not os.path.exists(self._name_db):
-            maker = cdb.cdbmake(self._name_db, self._name_db + '.tmp')
-            maker.finish()
+            with cdb.cdbmake(self._name_db, self._name_db + '.tmp') as maker:
+                pass
 
     def _get_item_id(self, itemname):
         """
@@ -95,8 +92,8 @@
 
         :param itemname: name of item (unicode)
         """
-        c = cdb.init(self._name_db)
-        return c.get(itemname.encode('utf-8'))
+        with cdb.init(self._name_db) as c:
+            return c.get(itemname.encode('utf-8'))
 
     def get_item(self, itemname):
         item_id = self._get_item_id(itemname)
@@ -126,13 +123,13 @@
         return item
 
     def iter_items_noindex(self):
-        c = cdb.init(self._name_db)
-        r = c.each()
-        while r:
-            item = Item(self, r[0].decode('utf-8'))
-            item._fs_item_id = r[1]
-            yield item
+        with cdb.init(self._name_db) as c:
             r = c.each()
+            while r:
+                item = Item(self, r[0].decode('utf-8'))
+                item._fs_item_id = r[1]
+                yield item
+                r = c.each()
 
     iteritems = iter_items_noindex
 
@@ -213,19 +210,18 @@
         nn = newname.encode('utf-8')
         npath = os.path.join(self._path, item._fs_item_id, 'name')
 
-        c = cdb.init(self._name_db)
-        maker = cdb.cdbmake(self._name_db + '.ndb', self._name_db + '.tmp')
-        r = c.each()
-        while r:
-            i, v = r
-            if i == nn:
-                raise ItemAlreadyExistsError("Target item '%r' already exists!" % newname)
-            elif v == item._fs_item_id:
-                maker.add(nn, v)
-            else:
-                maker.add(i, v)
-            r = c.each()
-        maker.finish()
+        with cdb.init(self._name_db) as c:
+            with cdb.cdbmake(self._name_db + '.ndb', self._name_db + '.tmp') as maker:
+                r = c.each()
+                while r:
+                    i, v = r
+                    if i == nn:
+                        raise ItemAlreadyExistsError("Target item '%r' already exists!" % newname)
+                    elif v == item._fs_item_id:
+                        maker.add(nn, v)
+                    else:
+                        maker.add(i, v)
+                    r = c.each()
 
         filesys.rename(self._name_db + '.ndb', self._name_db)
         nf = open(npath, mode='wb')
@@ -262,24 +258,28 @@
 
         nn = item.name.encode('utf-8')
 
-        c = cdb.init(self._name_db)
-        maker = cdb.cdbmake(self._name_db + '.ndb', self._name_db + '.tmp')
-        r = c.each()
-        while r:
-            i, v = r
-            if i == nn:
-                # Oops. This item already exists! Clean up and error out.
-                maker.finish()
-                os.unlink(self._name_db + '.ndb')
-                os.rmdir(ipath)
-                if newrev is not None:
-                    os.unlink(newrev)
-                raise ItemAlreadyExistsError("Item '%r' already exists!" % item.name)
-            else:
-                maker.add(i, v)
-            r = c.each()
-        maker.add(nn, itemid)
-        maker.finish()
+        class DuplicateError(Exception):
+            """ raise if we have a duplicate name """
+
+        try:
+            with cdb.init(self._name_db) as c:
+                with cdb.cdbmake(self._name_db + '.ndb', self._name_db + '.tmp') as maker:
+                    r = c.each()
+                    while r:
+                        i, v = r
+                        if i == nn:
+                            raise DuplicateError
+                        else:
+                            maker.add(i, v)
+                        r = c.each()
+                    maker.add(nn, itemid)
+        except DuplicateError:
+            # Oops. This item already exists! Clean up and error out.
+            os.unlink(self._name_db + '.ndb')
+            os.rmdir(ipath)
+            if newrev is not None:
+                os.unlink(newrev)
+            raise ItemAlreadyExistsError("Item '%r' already exists!" % item.name)
 
         if newrev is not None:
             rp = os.path.join(self._path, itemid, 'rev.0')
@@ -366,15 +366,14 @@
         os.unlink(rev._fs_revpath)
 
     def _destroy_item_locked(self, item):
-        c = cdb.init(self._name_db)
-        maker = cdb.cdbmake(self._name_db + '.ndb', self._name_db + '.tmp')
-        r = c.each()
-        while r:
-            i, v = r
-            if v != item._fs_item_id:
-                maker.add(i, v)
-            r = c.each()
-        maker.finish()
+        with cdb.init(self._name_db) as c:
+            with cdb.cdbmake(self._name_db + '.ndb', self._name_db + '.tmp') as maker:
+                r = c.each()
+                while r:
+                    i, v = r
+                    if v != item._fs_item_id:
+                        maker.add(i, v)
+                    r = c.each()
 
         filesys.rename(self._name_db + '.ndb', self._name_db)
         path = os.path.join(self._path, item._fs_item_id)
--- a/MoinMoin/storage/backends/hg.py	Sat Apr 30 21:28:25 2011 +0200
+++ b/MoinMoin/storage/backends/hg.py	Sat Apr 30 23:18:40 2011 +0200
@@ -52,10 +52,7 @@
 except ImportError:
     pass
 
-try:
-    import cdb
-except ImportError:
-    from MoinMoin.util import pycdb as cdb
+from MoinMoin.util import pycdb as cdb
 
 from MoinMoin.config import USERID, COMMENT, MTIME
 from MoinMoin.storage import Backend, Item, StoredRevision, NewRevision
@@ -149,13 +146,13 @@
             item = Item(self, self._name(id))
             item._id = id
             yield item
-        c = cdb.init(self._meta_db)
-        record = c.each()
-        while record:
-            item = Item(self, record[1])
-            item._id = record[0]
-            yield item
+        with cdb.init(self._meta_db) as c:
             record = c.each()
+            while record:
+                item = Item(self, record[1])
+                item._id = record[0]
+                yield item
+                record = c.each()
 
     iteritems = iter_items_noindex
 
@@ -457,8 +454,8 @@
             meta = fctx.changectx().extra()
             return self._decode_metadata(meta, BACKEND_METADATA_PREFIX)['name']
         except LookupError:
-            c = cdb.init(self._meta_db)
-            return c.get(itemid)
+            with cdb.init(self._meta_db) as c:
+                return c.get(itemid)
 
     def _iter_changelog(self, reverse=True, filter_id=None, start_rev=None, filter_meta=None):
         """
@@ -584,34 +581,38 @@
 
     def _has_meta(self, itemid):
         """Return True if Item with given ID has Metadata. Otherwise return None."""
-        c = cdb.init(self._meta_db)
-        return c.get(itemid)
+        with cdb.init(self._meta_db) as c:
+            return c.get(itemid)
 
     def _add_to_cdb(self, itemid, itemname, replace=None):
         """Add Item Metadata file to name-mapping."""
-        c = cdb.init(self._meta_db)
-        maker = cdb.cdbmake("%s.ndb" % self._meta_db, "%s.tmp" % self._meta_db)
-        record = c.each()
-        while record:
-            id, name = record
-            if id == itemid:
-                maker.finish()
-                os.unlink(self._meta_db + '.ndb')
-                raise ItemAlreadyExistsError("Destination item already exists: %s" % itemname)
-            elif id == replace:
-                pass
-            else:
-                maker.add(id, name)
-            record = c.each()
-        maker.add(itemid, itemname.encode('utf-8'))
-        maker.finish()
-        util.rename("%s.ndb" % self._meta_db, self._meta_db)
+        class DuplicateError(Exception):
+            """ raise for duplicate item names """
+
+        try:
+            with cdb.init(self._meta_db) as c:
+                with cdb.cdbmake("%s.ndb" % self._meta_db, "%s.tmp" % self._meta_db) as maker:
+                    record = c.each()
+                    while record:
+                        id, name = record
+                        if id == itemid:
+                            raise DuplicateError
+                        elif id == replace:
+                            pass
+                        else:
+                            maker.add(id, name)
+                        record = c.each()
+                    maker.add(itemid, itemname.encode('utf-8'))
+                util.rename("%s.ndb" % self._meta_db, self._meta_db)
+        except DuplicateError:
+            os.unlink(self._meta_db + '.ndb')
+            raise ItemAlreadyExistsError("Destination item already exists: %s" % itemname)
 
     def _create_cdb(self):
         """Create name-mapping file for storing Item Metadata files mappings."""
         if not os.path.exists(self._meta_db):
-            maker = cdb.cdbmake(self._meta_db, "%s.tmp" % self._meta_db)
-            maker.finish()
+            with cdb.cdbmake(self._meta_db, "%s.tmp" % self._meta_db) as maker:
+                pass
 
     def _destroyed_index(self, item, create=False):
         return Index(os.path.join(self._rev_path, "%s.rip" % item._id), create)
--- a/MoinMoin/util/pycdb.py	Sat Apr 30 21:28:25 2011 +0200
+++ b/MoinMoin/util/pycdb.py	Sat Apr 30 23:18:40 2011 +0200
@@ -57,6 +57,17 @@
         self._keyiter = None
         self._eachiter = None
 
+    def close(self):
+        if self._fp:
+            self._fp.close()
+            self._fp = None
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, type, value, traceback):
+        self.close()
+
     def __getstate__(self):
         raise TypeError
 
@@ -149,6 +160,18 @@
         self._pos = 2048
         self._bucket = [array('I') for _ in xrange(256)]
 
+    def close(self):
+        # you don't need to call this if you called finish()
+        if self._fp:
+            self._fp.close()
+            self._fp = None
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, type, value, traceback):
+        self.finish()
+
     def __len__(self):
         return self.numentries
 
@@ -198,7 +221,7 @@
             a.append(len(b1))
             pos_hash += len(b1) * 8
         self._fp.write(encode(a))
-        self._fp.close()
+        self.close()
         os.rename(self.fntmp, self.fn)
 
 cdbmake = CDBMaker