Explicit iterator closing

[wbolster]

The iterator API should allow explicit closing. Ideas:

• Add an `Iterator.close() method. Currently iterators keep a reference to the DB, an DB's a weak reference to the iterators. All public API should check for validity before calling any LevelDB API on the underlying structures.
• Maybe add an .iterator(autoclose=True) or similar flag to automatically cleanup an iterator whenever the first StopIteration is raised.
• Maybe allow iterators to be used as context managers (though @contextlib.closing would be the same if .close() was implemented).

This issue was inspired by @jtolds in #17:

also, we need to support closing iterators, so that we don't have to wait for garbage collection to kick in to free the leveldb snapshot backing the iterator

[wbolster]

Actually, the current implementation already has an internal Iterator.close(), but it is not accessible from the outside.

[wbolster]

@jtolds, I'd appreciate your take on this.

[jtolio]

whoops, i suck at the internet, sorry for the delay.

honestly, my preference is option 1.

iterator closing isn't necessary in low-load or not-very-memory-constrained environments, so it's not that big of a deal to forget to call it or something. my expectation is the only people who will close iterators are people who are already aware of the snapshot/gc leak problem. so i'm okay with having a sort of explicit interface.

i don't think you want to close the iterator on stopiteration. it feels a little funny. if someone iterates and then wants to call seek again, what happens?

the context manager thing is fine, but yeah as you pointed out that's the same as @contextlib.closing

[wbolster]

I've just implemented option 1 and 3 from the original report in commit 53e65ad. Updated docs describe how it works:

• https://plyvel.readthedocs.org/en/latest/user.html#closing-iterators
• https://plyvel.readthedocs.org/en/latest/api.html#Iterator.close

@jtolds Please review the docs, and feel free to come up with better text suggestions.

[wbolster]

i don't think you want to close the iterator on stopiteration. it feels a little funny. if someone iterates and then wants to call seek again, what happens?

I agree this is funny. Specifying an autoclose=True flag is just as much typing as it.close(), but in the latter case it is much more obvious what happens. Explicit is better than implicit. :)

[jtolio]

this all looks great, thanks a lot wouter

[wbolster]

Ok, great. I've just released Plyvel 0.6: https://plyvel.readthedocs.org/en/0.6/news.html#plyvel-0-6