[Checkins] SVN: Sandbox/tseaver/no_pdata_in_cache_dammit/ Avoid
thrashing ZODB Connection cache when serving large
OFS.Image.File objects.
Tres Seaver
tseaver at palladion.com
Thu Oct 25 17:57:12 EDT 2007
Log message for revision 81116:
Avoid thrashing ZODB Connection cache when serving large OFS.Image.File objects.
(Launchpad #157245)
Changed:
U Sandbox/tseaver/no_pdata_in_cache_dammit/doc/CHANGES.txt
U Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/Image.py
U Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/testFileAndImage.py
A Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/test_FileIterator.py
-=-
Modified: Sandbox/tseaver/no_pdata_in_cache_dammit/doc/CHANGES.txt
===================================================================
--- Sandbox/tseaver/no_pdata_in_cache_dammit/doc/CHANGES.txt 2007-10-25 19:27:14 UTC (rev 81115)
+++ Sandbox/tseaver/no_pdata_in_cache_dammit/doc/CHANGES.txt 2007-10-25 21:57:12 UTC (rev 81116)
@@ -8,6 +8,9 @@
Bugs fixed
+ - Launchpad #157245: avoid thrashing ZODB Connection cache when
+ serving large OFS.Image.File objects.
+
- Launchpad #147201: treat container-class in zope.conf as a string,
making it possible to use types from extra products directories.
Modified: Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/Image.py
===================================================================
--- Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/Image.py 2007-10-25 19:27:14 UTC (rev 81115)
+++ Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/Image.py 2007-10-25 21:57:12 UTC (rev 81116)
@@ -15,6 +15,7 @@
$Id$
"""
import struct
+from UserDict import UserDict
from zope.contenttype import guess_content_type
from Globals import DTMLFile
from Globals import InitializeClass
@@ -38,7 +39,7 @@
from mimetools import choose_boundary
from ZPublisher import HTTPRangeSupport
from ZPublisher.HTTPRequest import FileUpload
-from ZPublisher.Iterators import filestream_iterator
+from ZPublisher.Iterators import IStreamIterator
from zExceptions import Redirect
from cgi import escape
import transaction
@@ -402,12 +403,26 @@
RESPONSE.setBase(None)
return data
- while data is not None:
- RESPONSE.write(data.data)
- data=data.next
+ # Fetch our clone from a new, throwaway jar, so that we
+ # can return an iterator, rather than trashing the pickle cache
+ # of the current jar by ripping through all our Pdata instances.
+ jar = self._getTransientJar()
+ clone = jar.get(self._p_oid)
+ return FileIterator(clone)
- return ''
+ security.declarePrivate('_getTransientJar')
+ def _getTransientJar(self):
+ jar = self._p_jar
+ if jar is None:
+ raise ValueError('No existing jar!')
+
+ new_jar = jar.__class__(jar._db, jar._version, 0)
+ new_jar._cache = EmptyPickleCache()
+ new_jar.open(jar.transaction_manager, jar._mvcc, False, False)
+
+ return new_jar
+
security.declareProtected(View, 'view_image_or_file')
def view_image_or_file(self, URL1):
"""
@@ -903,3 +918,53 @@
next=self.next
return ''.join(r)
+
+
+class EmptyPickleCache(UserDict):
+ # Don't cache *anything*, period.
+ def __setitem__(self, key, value):
+ pass
+
+ def invalidate(self, invalidated):
+ pass
+
+ def incrgc(self):
+ pass
+
+class FileIterator(object):
+
+ __implements__ = (IStreamIterator,)
+
+ def __init__(self, fileobj):
+
+ self._to_close = fileobj._p_jar
+
+ next = fileobj.data
+
+ if next == '':
+ next = None
+ elif not isinstance(next, Pdata): #normalize
+ next = Pdata(next)
+
+ self._next = next
+
+ def _cleanup(self):
+ if self._to_close is not None:
+ self._to_close.close(primary=False)
+ self._to_close = None
+
+ def __del__(self):
+ self._cleanup()
+
+ def __len__(self):
+ raise NotImplementedError #WTF is this for?
+
+ def next(self):
+
+ if self._next is None:
+ self._cleanup()
+ raise StopIteration
+
+ current, self._next = self._next, self._next.next
+
+ return current.data
Modified: Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/testFileAndImage.py
===================================================================
--- Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/testFileAndImage.py 2007-10-25 19:27:14 UTC (rev 81115)
+++ Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/testFileAndImage.py 2007-10-25 21:57:12 UTC (rev 81116)
@@ -212,14 +212,21 @@
self.assertEqual(str(self.file.data), s)
def testIndexHtmlWithPdata(self):
+ from ZPublisher.Iterators import IStreamIterator
self.file.manage_upload('a' * (2 << 16)) # 128K
- self.file.index_html(self.app.REQUEST, self.app.REQUEST.RESPONSE)
- self.assert_(self.app.REQUEST.RESPONSE._wrote)
+ result = self.file.index_html(self.app.REQUEST,
+ self.app.REQUEST.RESPONSE)
+ self.failUnless(IStreamIterator.isImplementedBy(result))
+ self.failIf(self.app.REQUEST.RESPONSE._wrote)
+
def testIndexHtmlWithString(self):
- self.file.manage_upload('a' * 100) # 100 bytes
- self.file.index_html(self.app.REQUEST, self.app.REQUEST.RESPONSE)
- self.assert_(not self.app.REQUEST.RESPONSE._wrote)
+ A100 = 'a' * 100 # 100 bytes
+ self.file.manage_upload(A100)
+ result = self.file.index_html(self.app.REQUEST,
+ self.app.REQUEST.RESPONSE)
+ self.assertEqual(result, A100)
+ self.failIf(self.app.REQUEST.RESPONSE._wrote)
def testStr(self):
self.assertEqual(str(self.file), self.data)
Added: Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/test_FileIterator.py
===================================================================
--- Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/test_FileIterator.py (rev 0)
+++ Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/test_FileIterator.py 2007-10-25 21:57:12 UTC (rev 81116)
@@ -0,0 +1,111 @@
+import unittest
+
+class FileIteratorTests(unittest.TestCase):
+
+ def _getTargetClass(self):
+ from OFS.Image import FileIterator
+ return FileIterator
+
+ def _makeOne(self, fileobj, *args, **kw):
+ return self._getTargetClass()(fileobj, *args, **kw)
+
+ def _makeFile(self, id='test', title='', file='', chunks=()):
+ from OFS.Image import File
+ from OFS.Image import Pdata
+ fileobj = File(id, title, file)
+ jar = fileobj._p_jar = DummyConnection()
+ if chunks:
+ chunks = list(chunks)
+ chunks.reverse()
+ head = None
+ for chunk in chunks: # build Pdata chain
+ pdata = Pdata(chunk)
+ pdata.next = head
+ head = pdata
+ fileobj.data = head
+ return fileobj
+
+ def test_class_conforms_to_Z2_IStreamIterator(self):
+ from Interface.Verify import verifyClass
+ from ZPublisher.Iterators import IStreamIterator
+ verifyClass(IStreamIterator, self._getTargetClass())
+
+ def test_instance_conforms_to_Z2_IStreamIterator(self):
+ from Interface.Verify import verifyObject
+ from ZPublisher.Iterators import IStreamIterator
+ verifyObject(IStreamIterator, self._makeOne(self._makeFile()))
+
+ def test___len___raises_NotImplementedErrlr(self):
+ fileobj = self._makeFile()
+ iterator = self._makeOne(fileobj)
+ self.assertRaises(NotImplementedError, lambda: len(iterator))
+
+ def test_iteration_over_empty_file(self):
+ fileobj = self._makeFile()
+ iterator = self._makeOne(fileobj)
+
+ self.assertEqual(fileobj._p_jar._close_called, None)
+ self.assertRaises(StopIteration, iterator.next)
+ self.assertEqual(fileobj._p_jar._close_called, (False,))
+
+ def test_iteration_over_file_with_string(self):
+ ABC = 'ABC'
+ fileobj = self._makeFile(file=ABC)
+ iterator = self._makeOne(fileobj)
+
+ chunk = iterator.next()
+ self.assertEqual(chunk, ABC)
+
+ self.assertEqual(fileobj._p_jar._close_called, None)
+ self.assertRaises(StopIteration, iterator.next)
+ self.assertEqual(fileobj._p_jar._close_called, (False,))
+
+ def test_iteration_over_file_with_one_chunk(self):
+ ABC = 'ABC'
+ fileobj = self._makeFile(chunks=(ABC,))
+ iterator = self._makeOne(fileobj)
+
+ chunk = iterator.next()
+ self.assertEqual(chunk, ABC)
+
+ self.assertEqual(fileobj._p_jar._close_called, None)
+ self.assertRaises(StopIteration, iterator.next)
+ self.assertEqual(fileobj._p_jar._close_called, (False,))
+
+ def test_iteration_over_file_with_one_chunk(self):
+ CHUNKS = ('ABC', 'DEF', 'GHI', 'JKL')
+ fileobj = self._makeFile(chunks=CHUNKS)
+ iterator = self._makeOne(fileobj)
+
+ for expected in CHUNKS:
+ found = iterator.next()
+ self.assertEqual(found, expected)
+
+ self.assertEqual(fileobj._p_jar._close_called, None)
+ self.assertRaises(StopIteration, iterator.next)
+ self.assertEqual(fileobj._p_jar._close_called, (False,))
+
+ def test___del___closes_targets_jar_not_as_primary(self):
+ fileobj = self._makeFile()
+ iterator = self._makeOne(fileobj)
+ self.assertEqual(fileobj._p_jar._close_called, None)
+ del iterator
+ self.assertEqual(fileobj._p_jar._close_called, (False,))
+
+class DummyConnection:
+
+ _close_called = None
+
+ def register(self, obj):
+ pass
+
+ def close(self, primary=True):
+ self._close_called = (primary,)
+
+def test_suite():
+ return unittest.TestSuite((
+ unittest.makeSuite(FileIteratorTests),
+ ))
+
+if __name__ == '__main__':
+ unittest.main(defaultTest='test_suite')
Property changes on: Sandbox/tseaver/no_pdata_in_cache_dammit/lib/python/OFS/tests/test_FileIterator.py
___________________________________________________________________
Name: svn:keywords
+ Id
Name: svn:eol-style
+ native
More information about the Checkins
mailing list