Compatibility with PyPy #2
Conversation
|
Something in the test suite is leaking an open connection (that gets reclaimed by garbage collection), hence the failure. I'm trying to track it down, but it's proving elusive. |
|
ZODB.tests.TransactionalUndoStorage definitely leaks open databases and connections but I don't see how that could be related |
|
I'm pretty sure I've tracked the leaked objects that cause this failure down to relstorage.tests.RecoveryStorage:UndoableRecoveryStorage.checkRestoreAcrossPack (and possibly checkRestoreWithMultipleObjectsInUndoRedo). It takes some weird manipulations but I can reproduce the errors now. Running the GC here will be the simplest solution, I think, until there's an updated RelStorage release that closes things up properly. |
I notice that setup.py specifies both `RelStorage[postgresql]` and also `psycopg2`. The former will work correctly on PyPy, but the latter will not (you want `psycopg2cffi`).
There doesn't seem to be any actual usage of psycopg2 in the code itself, only in a test case:
```
newtdb$ ack psycopg2
setup.py
3:install_requires = ['setuptools', 'RelStorage[postgresql]', 'psycopg2', 'six']
src/newt/db/_search.py
6:import psycopg2
src/newt/db/tests/base.py
1:import psycopg2
13: self.base_conn = psycopg2.connect('')
```
If you need to connect to a database in a test case, you could try this:
```
import relstorage.adapters.postgresql.adapter as adapter
self.base_conn = adapter.select_driver().connect('')
```
Then in theory you should Just Work with any supported driver, assuming you stick to the DB-API. Or at least, psyc2pg2 and psycopg2cffi pretty much implement the same extensions.
Adding `- pypy-5.4.1` to `.travis.yml` should run your tests on a new-ish pypy.
|
Thoughts @jimfulton ? |
|
Hm, I wasn't notified of this PR. I wonder what I need to do to fix that... sigh |
|
Thanks for this! I knew I had some debt in this area. :) I'll look at this shortly. |
|
Oh, I needed to watch this repo. :) |
| @@ -173,6 +173,10 @@ def create_text_index(conn, fname, D, C=None, B=None, A=None): | |||
| conn.commit() | |||
| finally: | |||
| try: | |||
| cursor.close() | |||
There was a problem hiding this comment.
Really? Closing the connection doesn't close the cursor?
There was a problem hiding this comment.
In most DBAPI implementations I've looked at (far too many 😄 ), the cursor has a reference to the connection, but the connection does not have references to the cursors that are opened.
Now, closing the connection will close the socket, it's true. And that would probably be enough to solve this particular case. In general, however, it's best to be sure to close the cursors, I've found. A buffered cursor (most of them are by default) will have the last row set in memory and it won't get rid of it until its closed (or GCd), so closing it allows resources to be cleaned up sooner and deterministically. With unbuffered cursors, if you don't close the cursor then when you close the connection it will detect that it was in the middle of a result set and complain either by logging a warning (pymysql) or raising an error (Oracle's official driver). I haven't tried unbuffered cursors on postgres so I don't know what they do.
There was a problem hiding this comment.
So the leak check code that I use in RelStorage complains when both cursors and connections are left open to be GCd. That's how I found this.
|
Thanks. Sorry about the unpleasantness of needing to call gc.collect() due to the leaking test case in RelStorage. But since there's also a leaking test case in ZODB itself, I'm not too sorry 😉 I'll try to submit a PR against ZODB. |
|
Gaaaa, it looks like pypy and cpython eggs get named the same thing. This is a pain for buildlut because of it's shared eggs directory. whimper. |
|
There's no binary (native) code here, so why does it matter? Don't both interpreters ignore bad .pyc files if there's a .py file too? Which reminds me, there should probably be a So that creating wheels and uploading them to PyPI does the right thing. |
|
Oh, or is it the native code in the RelStorage/persistent/BTree eggs that causing the problems? That I've definitely run into before and had to resort to separate egg directories. |
|
idk what's going on. I have to remove RelStorage and cffi eggs from my egg cache when switching between cpython and pypy. For example, switching to pypy without removing those eggs: Also, RelStorage's egg name is: If I remove the cffi egg and rerun buildout, I get: |
|
I don't really understand how pypy handles extensions. When building with pypy, I get the same egg names as with cpython. I wonder if there's something else going on here. |
|
No, that's what's happening. I've seen the same thing with persistent and other things that use CFFI or optional native code. Wheels use a different wheel "ABI tag" for PyPy and CPython (pypy_41 and cp27-cp27m[u] respectively), but traditional setuptools doesn't know about ABI tags, just the architecture...you'd have the same problem, for example, switching between wide and narrow unicode builds of CPython 2. |
|
Yup. But I'd have no reason to switch back and forth between those. sigh |
|
This is a large part of why I now typically prefer to avoid buildout in favor of virtualenvs for individual projects (reserving buildout for integration of applications). I work on a lot of projects that use CFFI/native code, all of which support PyPy 😄 |
I notice that setup.py specifies both
RelStorage[postgresql]and alsopsycopg2. The former will work correctly on PyPy, but the latter will not (you wantpsycopg2cffi).There doesn't seem to be any actual usage of psycopg2 in the code itself, only in a test case:
If you need to connect to a database in a test case, you could try this:
Then in theory you should Just Work with any supported driver, assuming you stick to the DB-API. Or at least, psyc2pg2 and psycopg2cffi pretty much implement the same extensions.
Adding
- pypy-5.4.1to.travis.ymlshould run your tests on a new-ish pypy.