wbolster / plyvel

Plyvel, a fast and feature-rich Python interface to LevelDB
https://plyvel.readthedocs.io/
Other
530 stars 75 forks source link

del db does actually not release the LOCK #3

Closed oleiade closed 11 years ago

oleiade commented 11 years ago

While trying to port plyvel into Elevator, I noticed that DB.del method does not actually release the LOCK.

>>> db = plyvel.DB('default')
>>> del db
>>> new_db = plyvel.DB('default')
---------------------------------------------------------------------------
IOError                                   Traceback (most recent call last)
<ipython-input-10-7174fcd34511> in <module>()
----> 1 new_db = plyvel.DB('default')

/Users/oleiade/.python-eggs/plyvel-0.1dev-py2.7-macosx-10.8-intel.egg-tmp/plyvel/_plyvel.so in plyvel._plyvel.DB.__init__ (plyvel/_plyvel.cpp:2264)()

/Users/oleiade/.python-eggs/plyvel-0.1dev-py2.7-macosx-10.8-intel.egg-tmp/plyvel/_plyvel.so in plyvel._plyvel.raise_for_status (plyvel/_plyvel.cpp:1061)()

IOError: IO error: lock default/LOCK: already held by process
oleiade commented 11 years ago

My bad, bug seems to be inherent to my macbook dev environment rather than plyvel.

wbolster commented 11 years ago

I'm seeing similar issues, actually. Any clue on what might be causing this?

wbolster commented 11 years ago

Is the example you gave above a complete example? Did you execute any operations in between that may have resulted in references to iterators or write batches still lying around (note that write batches in a "with" block will not be deallocated automatically)?

oleiade commented 11 years ago

Hey, it kind of reassure me at least that you get the same problem too, 'cause I started seing it on another computer running freebsd today.

Yes this is a complete sample.

I've noticed that through unit tests, and went down to the minimal operation of creating a db connector -> releasing it -> creating a new connector to that db; which raises the problem.

oleiade commented 11 years ago

Tested on Linux mint, this passes:

>>> import plyvel
>>> db = plyvel.DB('default')
>>> new_db = plyvel.DB('default')
>>> db.put('1', 'a')
>>> new_db.get('1')

So I guess that in some way, python/cython, makes it possible for two DB objects working on the same db, in the same process to share the DB* pointer. (Don't ask me how, this is just an hypothesis).

On what os, python version etc did you experienced that bug?

wbolster commented 11 years ago

The double open support is a LevelDB bug I think. I guess it can lead to corruption even.

wbolster commented 11 years ago

I'm working on DB.close() btw, but it's a bit tricky due to the way things like iterator destructors work. And it requires a check in each public API method. :-(

wbolster commented 11 years ago

Btw, the double open issue does not reuse the pointer I think.

wbolster commented 11 years ago

It took some effort, but here we are: plyvel.DB has a .close () method since commit 755e000498463b70afd7423f9d3aa1b2850b6f07. Please review and test, since the code is a bit tricky. (Don't forget to check the API docs.)

wbolster commented 11 years ago

Did you have any chance to test whether the .close() API solves the problems reported earlier?

wbolster commented 11 years ago

Btw, the double open issue has been fixed a while back in LevelDB itself: http://code.google.com/p/leveldb/issues/detail?id=120

oleiade commented 11 years ago

Problem seems to be solved now. With plyvel head at fccb583488a81 and leveldb 1.7.1

wbolster commented 11 years ago

Ok, same here. Thanks for the report.