bmatsuo / lmdb-go

Bindings for the LMDB C library
BSD 3-Clause "New" or "Revised" License
158 stars 59 forks source link

Keep finalizer on Txn.Reset and address incompatibilities of Txn and sync.Pool #105

Closed bmatsuo closed 7 years ago

bmatsuo commented 7 years ago

Fixes #104

In addition to removing the undocumented finalizer behavior in Txn.Reset and Txn.Renew which, as #104 points out, was never a good idea or a safe one. I have gone on to try and ensure that lmdb.Txn and sync.Pool can be used together effectively (to whatever extent that is truly possible).

There are some questions about how sync.Pool and lmdb.Txn truly interact, mostly because of how poorly defined finalizers are. In testing it has been seen that naive pooling fails terribly if the application has been compiled with -race. The interaction with -race was so bad that it spurred the creation of a new experimental package lmdbpool, which contains a workaround for race detection using build tags.

At this point I'm not completely certain if there are unforeseen problems in interactions between sync.Pool, finalizers, the Go runtime, and cgo. But as far as I have been able to tell the only potential downside is an excess of pages being used. But my understanding of LMDB makes me believe it is possible to benefit from pooling while tracking which transactions would cause LMDB to retain stale pages and keeping such pages to a minimum (not significantly more stale pages than an application without pooling would generate).

So, I am willing to provide the lmdbpool package as an 'experimental' package included in the lmdb-go project until its use is shown to be truly and unavoidably unsafe. If the lmdbpool package proves to be successful with enough benefits then it can be rolled into the core lmdb package for more direct/transparent integration.