Commit 435b3b75 authored by Christian Theune's avatar Christian Theune

- updated the blob todo list

 - some cleanups
 - re-introduced a way to ask for a rename() operation but fall back to copy
   if rename doesn't work
 - using the rename_or_copy for blobs where reasonable
 - fixed a test that was trying to test a failing consumeBlob but where the
   wrong exception was tested for
parent 763291b1
...@@ -920,14 +920,14 @@ class ClientStorage(object): ...@@ -920,14 +920,14 @@ class ClientStorage(object):
i = 0 i = 0
while 1: while 1:
try: try:
os.rename(filename, target + str(i)) utils.rename_or_copy(filename, target + str(i))
except OSError: except OSError:
i += 1 i += 1
else: else:
break break
target += str(i) target += str(i)
else: else:
os.rename(filename, target) utils.rename_or_copy(filename, target)
# Now tell the server where we put it # Now tell the server where we put it
self._server.storeBlobShared( self._server.storeBlobShared(
oid, serial, data, oid, serial, data,
......
...@@ -1242,7 +1242,7 @@ class TmpStore: ...@@ -1242,7 +1242,7 @@ class TmpStore:
def storeBlob(self, oid, serial, data, blobfilename, version, def storeBlob(self, oid, serial, data, blobfilename, version,
transaction): transaction):
serial = self.store(oid, serial, data, version, transaction) serial = self.store(oid, serial, data, version, transaction)
assert isinstance(serial, str) # XXX in theory serials could be assert isinstance(serial, str) # XXX in theory serials could be
# something else # something else
targetpath = self._getBlobPath(oid) targetpath = self._getBlobPath(oid)
...@@ -1250,7 +1250,7 @@ class TmpStore: ...@@ -1250,7 +1250,7 @@ class TmpStore:
os.makedirs(targetpath, 0700) os.makedirs(targetpath, 0700)
targetname = self._getCleanFilename(oid, serial) targetname = self._getCleanFilename(oid, serial)
os.rename(blobfilename, targetname) utils.rename_or_copy(blobfilename, targetname)
def loadBlob(self, oid, serial): def loadBlob(self, oid, serial):
"""Return the filename where the blob file can be found. """Return the filename where the blob file can be found.
......
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
import base64 import base64
import logging import logging
import logging
import os import os
import shutil import shutil
import sys import sys
...@@ -195,7 +194,7 @@ class Blob(persistent.Persistent): ...@@ -195,7 +194,7 @@ class Blob(persistent.Persistent):
os.unlink(target) os.unlink(target)
try: try:
os.rename(filename, target) utils.rename_or_copy(filename, target)
except: except:
# Recover from the failed consumption: First remove the file, it # Recover from the failed consumption: First remove the file, it
# might exist and mark the pointer to the uncommitted file. # might exist and mark the pointer to the uncommitted file.
...@@ -254,7 +253,7 @@ class BlobFile(file): ...@@ -254,7 +253,7 @@ class BlobFile(file):
def __init__(self, name, mode, blob): def __init__(self, name, mode, blob):
super(BlobFile, self).__init__(name, mode+'b') super(BlobFile, self).__init__(name, mode+'b')
self.blob = blob self.blob = blob
def close(self): def close(self):
self.blob.closed(self) self.blob.closed(self)
file.close(self) file.close(self)
...@@ -411,7 +410,7 @@ class BlobStorage(SpecificationDecoratorBase): ...@@ -411,7 +410,7 @@ class BlobStorage(SpecificationDecoratorBase):
os.makedirs(targetpath, 0700) os.makedirs(targetpath, 0700)
targetname = self.fshelper.getBlobFilename(oid, serial) targetname = self.fshelper.getBlobFilename(oid, serial)
os.rename(blobfilename, targetname) utils.rename_or_copy(blobfilename, targetname)
# XXX if oid already in there, something is really hosed. # XXX if oid already in there, something is really hosed.
# The underlying storage should have complained anyway # The underlying storage should have complained anyway
...@@ -611,9 +610,6 @@ class BlobStorage(SpecificationDecoratorBase): ...@@ -611,9 +610,6 @@ class BlobStorage(SpecificationDecoratorBase):
# cache. (Idea: LRU principle via mstat access time and a # cache. (Idea: LRU principle via mstat access time and a
# size-based threshold) currently). # size-based threshold) currently).
# #
# Make blobs able to efficiently consume existing files from the
# filesystem
#
# Savepoint support # Savepoint support
# ================= # =================
# #
......
...@@ -64,29 +64,53 @@ Edge cases ...@@ -64,29 +64,53 @@ Edge cases
There are some edge cases what happens when the link() operation There are some edge cases what happens when the link() operation
fails. We simulate this in different states: fails. We simulate this in different states:
Case 1: We don't have uncommitted data, but the link operation fails. The Case 1: We don't have uncommitted data, but the link operation fails. We fall
exception will be re-raised and the target file will not exist:: back to try a copy/remove operation that is successfull::
>>> open('to_import', 'wb').write('Some data.') >>> open('to_import', 'wb').write('Some data.')
>>> def failing_rename(self, filename): >>> def failing_rename(f1, f2):
... raise Exception("I can't link.") ... import exceptions
... if f1 == 'to_import':
... raise exceptions.OSError("I can't link.")
... os_rename(f1, f2)
>>> blob = Blob() >>> blob = Blob()
>>> os_rename = os.rename >>> os_rename = os.rename
>>> os.rename = failing_rename >>> os.rename = failing_rename
>>> blob.consumeFile('to_import') >>> blob.consumeFile('to_import')
The blob did not have data before, so it shouldn't have data now::
>>> blob.open('r').read()
'Some data.'
Case 2: We don't have uncommitted data and both the link operation and the
copy fail. The exception will be re-raised and the target file will not
exist::
>>> blob = Blob()
>>> import ZODB.utils
>>> utils_cp = ZODB.utils.cp
>>> def failing_copy(f1, f2):
... import exceptions
... raise exceptions.OSError("I can't copy.")
>>> ZODB.utils.cp = failing_copy
>>> open('to_import', 'wb').write('Some data.')
>>> blob.consumeFile('to_import')
Traceback (most recent call last): Traceback (most recent call last):
Exception: I can't link. OSError: I can't copy.
The blob did not have data before, so it shouldn't have data now:: The blob did not have data before, so it shouldn't have data now::
>>> blob.open('r').read() >>> blob.open('r').read()
'' ''
Case 2: We thave uncommitted data, but the link operation fails. The Case 3: We have uncommitted data, but the link and the copy operations fail.
exception will be re-raised and the target file will exist with the previous The exception will be re-raised and the target file will exist with the
uncomitted data:: previous uncomitted data::
>>> blob = Blob() >>> blob = Blob()
>>> blob_writing = blob.open('w') >>> blob_writing = blob.open('w')
...@@ -95,7 +119,7 @@ uncomitted data:: ...@@ -95,7 +119,7 @@ uncomitted data::
>>> blob.consumeFile('to_import') >>> blob.consumeFile('to_import')
Traceback (most recent call last): Traceback (most recent call last):
Exception: I can't link. OSError: I can't copy.
The blob did existed before and had uncommitted data, this shouldn't have The blob did existed before and had uncommitted data, this shouldn't have
changed:: changed::
...@@ -104,3 +128,4 @@ changed:: ...@@ -104,3 +128,4 @@ changed::
'Uncommitted data' 'Uncommitted data'
>>> os.rename = os_rename >>> os.rename = os_rename
>>> ZODB.utils.cp = utils_cp
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
# #
############################################################################## ##############################################################################
import exceptions
import sys import sys
import time import time
import struct import struct
...@@ -84,6 +85,7 @@ def u64(v): ...@@ -84,6 +85,7 @@ def u64(v):
U64 = u64 U64 = u64
def cp(f1, f2, length=None): def cp(f1, f2, length=None):
"""Copy all data from one file to another. """Copy all data from one file to another.
...@@ -112,6 +114,22 @@ def cp(f1, f2, length=None): ...@@ -112,6 +114,22 @@ def cp(f1, f2, length=None):
write(data) write(data)
length -= len(data) length -= len(data)
def rename_or_copy(f1, f2):
"""Try to rename f1 to f2, fallback to copy.
Under certain conditions a rename might not work, e.g. because the target
directory is on a different partition. In this case we try to copy the
data and remove the old file afterwards.
"""
try:
os.rename(f1, f2)
except exceptions.OSError:
cp(open(f1, 'rb'), open(f2, 'wb'))
os.unlink(f1)
def newTimeStamp(old=None, def newTimeStamp(old=None,
TimeStamp=TimeStamp, TimeStamp=TimeStamp,
time=time.time, gmtime=time.gmtime): time=time.time, gmtime=time.gmtime):
......
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