Commit 9e12b6c7 authored by Jim Fulton's avatar Jim Fulton

Fixed bug:

  Savepoint blob data wasn't cleaned up after a transaction abort.
  https://bugs.launchpad.net/zodb/+bug/323067
parent b08a6d01
...@@ -31,7 +31,7 @@ from persistent.interfaces import IPersistentDataManager ...@@ -31,7 +31,7 @@ from persistent.interfaces import IPersistentDataManager
from ZODB.interfaces import IConnection from ZODB.interfaces import IConnection
from ZODB.interfaces import IBlobStorage from ZODB.interfaces import IBlobStorage
from ZODB.interfaces import IMVCCStorage from ZODB.interfaces import IMVCCStorage
from ZODB.blob import Blob, rename_or_copy_blob from ZODB.blob import Blob, rename_or_copy_blob, remove_committed_dir
from transaction.interfaces import ISavepointDataManager from transaction.interfaces import ISavepointDataManager
from transaction.interfaces import IDataManagerSavepoint from transaction.interfaces import IDataManagerSavepoint
from transaction.interfaces import ISynchronizer from transaction.interfaces import ISynchronizer
...@@ -1239,11 +1239,16 @@ class TmpStore: ...@@ -1239,11 +1239,16 @@ class TmpStore:
self.index = {} self.index = {}
self.creating = {} self.creating = {}
self._blob_dir = None
def __len__(self): def __len__(self):
return len(self.index) return len(self.index)
def close(self): def close(self):
self._file.close() self._file.close()
if self._blob_dir is not None:
remove_committed_dir(self._blob_dir)
self._blob_dir = None
def load(self, oid, version): def load(self, oid, version):
pos = self.index.get(oid) pos = self.index.get(oid)
...@@ -1307,11 +1312,18 @@ class TmpStore: ...@@ -1307,11 +1312,18 @@ class TmpStore:
return ZODB.blob.BlobFile(blob_filename, 'r', blob) return ZODB.blob.BlobFile(blob_filename, 'r', blob)
def _getBlobPath(self): def _getBlobPath(self):
return os.path.join(self.temporaryDirectory(), 'savepoints') blob_dir = self._blob_dir
if blob_dir is None:
blob_dir = tempfile.mkdtemp(dir=self.temporaryDirectory(),
prefix='savepoints')
self._blob_dir = blob_dir
return blob_dir
def _getCleanFilename(self, oid, tid): def _getCleanFilename(self, oid, tid):
return os.path.join(self._getBlobPath(), return os.path.join(
"%s-%s%s" % (utils.oid_repr(oid), utils.tid_repr(tid), SAVEPOINT_SUFFIX,) self._getBlobPath(),
"%s-%s%s" % (utils.oid_repr(oid), utils.tid_repr(tid),
SAVEPOINT_SUFFIX,)
) )
def temporaryDirectory(self): def temporaryDirectory(self):
......
...@@ -231,18 +231,22 @@ We do support optimistic savepoints: ...@@ -231,18 +231,22 @@ We do support optimistic savepoints:
>>> root5['blob'].open("r").read() >>> root5['blob'].open("r").read()
"I'm a happy blob. And I'm singing." "I'm a happy blob. And I'm singing."
Savepoints store the blobs in the `savepoints` directory in the temporary Savepoints store the blobs in temporary directories in the temporary
directory of the blob storage: directory of the blob storage:
>>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints')) >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
['0x...-0x....spb'] ... if name.startswith('savepoint')])
>>> transaction.commit() 1
>>> os.listdir(os.path.join(blob_dir, 'tmp'))[0].startswith('savepoint')
True
After committing the transaction, the temporary savepoint files are moved to After committing the transaction, the temporary savepoint files are moved to
the committed location again: the committed location again:
>>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints')) >>> transaction.commit()
[] >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
... if name.startswith('savepoint')])
0
We support non-optimistic savepoints too: We support non-optimistic savepoints too:
...@@ -251,28 +255,24 @@ We support non-optimistic savepoints too: ...@@ -251,28 +255,24 @@ We support non-optimistic savepoints too:
"I'm a happy blob. And I'm singing. And I'm dancing." "I'm a happy blob. And I'm singing. And I'm dancing."
>>> savepoint = transaction.savepoint() >>> savepoint = transaction.savepoint()
Again, the savepoint creates a new file for the blob state in the savepoints Again, the savepoint creates a new savepoints directory:
directory:
>>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints')) >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
['0x...-0x....spb'] ... if name.startswith('savepoint')])
1
>>> root5['blob'].open("w").write(" And the weather is beautiful.") >>> root5['blob'].open("w").write(" And the weather is beautiful.")
>>> savepoint.rollback() >>> savepoint.rollback()
XXX Currently, savepoint state of blobs remains after a rollback:
>>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints'))
['0x...-0x....spb']
>>> root5['blob'].open("r").read() >>> root5['blob'].open("r").read()
"I'm a happy blob. And I'm singing. And I'm dancing." "I'm a happy blob. And I'm singing. And I'm dancing."
>>> transaction.abort() >>> transaction.abort()
XXX Currently, savepoint state of blobs remains even after an abort: The savepoint blob directory gets cleaned up on an abort:
>>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints')) >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
['0x...-0x....spb'] ... if name.startswith('savepoint')])
0
Reading Blobs outside of a transaction Reading Blobs outside of a transaction
-------------------------------------- --------------------------------------
......
...@@ -15,23 +15,17 @@ ...@@ -15,23 +15,17 @@
from pickle import Pickler from pickle import Pickler
from pickle import Unpickler from pickle import Unpickler
from StringIO import StringIO from StringIO import StringIO
from ZConfig import ConfigurationSyntaxError from ZODB.blob import Blob
from ZODB.blob import Blob, BlobStorage
from ZODB.DB import DB from ZODB.DB import DB
from ZODB.FileStorage import FileStorage from ZODB.FileStorage import FileStorage
from ZODB import utils
from ZODB.tests.testConfig import ConfigTestBase from ZODB.tests.testConfig import ConfigTestBase
from zope.testing import doctest from zope.testing import doctest
import base64
import os import os
import random import random
import re import re
import shutil
import stat
import struct import struct
import sys import sys
import sys
import time import time
import transaction import transaction
import unittest import unittest
...@@ -538,6 +532,59 @@ def do_not_depend_on_cwd(): ...@@ -538,6 +532,59 @@ def do_not_depend_on_cwd():
>>> bs.close() >>> bs.close()
""" """
def savepoint_isolation():
"""Make sure savepoint data is distinct accross transactions
>>> bs = create_storage()
>>> db = DB(bs)
>>> conn = db.open()
>>> conn.root.b = ZODB.blob.Blob('initial')
>>> transaction.commit()
>>> conn.root.b.open('w').write('1')
>>> _ = transaction.savepoint()
>>> tm = transaction.TransactionManager()
>>> conn2 = db.open(transaction_manager=tm)
>>> conn2.root.b.open('w').write('2')
>>> _ = tm.savepoint()
>>> conn.root.b.open().read()
'1'
>>> conn2.root.b.open().read()
'2'
>>> transaction.abort()
>>> tm.commit()
>>> conn.sync()
>>> conn.root.b.open().read()
'2'
>>> db.close()
"""
def savepoint_cleanup():
"""Make sure savepoint data gets cleaned up.
>>> bs = create_storage()
>>> tdir = bs.temporaryDirectory()
>>> os.listdir(tdir)
[]
>>> db = DB(bs)
>>> conn = db.open()
>>> conn.root.b = ZODB.blob.Blob('initial')
>>> _ = transaction.savepoint()
>>> len(os.listdir(tdir))
1
>>> transaction.abort()
>>> os.listdir(tdir)
[]
>>> conn.root.b = ZODB.blob.Blob('initial')
>>> transaction.commit()
>>> conn.root.b.open('w').write('1')
>>> _ = transaction.savepoint()
>>> transaction.abort()
>>> os.listdir(tdir)
[]
"""
def setUp(test): def setUp(test):
ZODB.tests.util.setUp(test) ZODB.tests.util.setUp(test)
test.globs['rmtree'] = zope.testing.setupstack.rmtree test.globs['rmtree'] = zope.testing.setupstack.rmtree
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment