Martchus
/
lmdb-safe
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Re-work the class hierarchy of cursors and transactions 

MDBRWTransaction now inherits from MDBROTransaction. Both
transaction types will free their cursors before aborting or
committing. In addition, transactions are now virtual classes.

The reason for this is that RW and RO transactions are handled
very differently in LMDB. Nevertheless, it is very useful to be
able to write read-only code in a way which also can use an RW
transaction. This saves code duplication or unnecessary templates.

Since the only methods which are reqiured to be virtual on
transactions for this to work are the destructor and the
abort/commit methods, the overhead should be neglegible.

This also comes in very handy when going forward to implement
nested transactions, because it is not possible to nest RO
transactions below RW transactions, exacerbating the issue
described above.

Fixes #7 en passant.
This commit is contained in:
Jonas Schäfer 2019-10-24 17:56:45 +02:00
parent c0cc016f3a
commit d2b0ee057a
7 changed files with 358 additions and 195 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

158
lmdb-safe.cc
View File

@ -152,40 +152,88 @@ MDBDbi MDBEnv::openDB(const string_view dbname, int flags)
return ret;
}
MDBRWTransaction::MDBRWTransaction(MDBEnv* parent, int flags) : d_parent(parent)
MDB_txn *MDBRWTransaction::openRWTransaction(MDBEnv *env, MDB_txn *parent, int flags)
{
if(d_parent->getROTX() || d_parent->getRWTX())
MDB_txn *result;
if(env->getROTX() || env->getRWTX())
throw std::runtime_error("Duplicate RW transaction");
for(int tries =0 ; tries < 3; ++tries) { // it might happen twice, who knows
if(int rc=mdb_txn_begin(d_parent->d_env, 0, flags, &d_txn)) {
if(int rc=mdb_txn_begin(env->d_env, parent, flags, &result)) {
if(rc == MDB_MAP_RESIZED && tries < 2) {
// "If the mapsize is increased by another process (..) mdb_txn_begin() will return MDB_MAP_RESIZED.
// call mdb_env_set_mapsize with a size of zero to adopt the new size."
mdb_env_set_mapsize(d_parent->d_env, 0);
mdb_env_set_mapsize(env->d_env, 0);
continue;
}
throw std::runtime_error("Unable to start RW transaction: "+std::string(mdb_strerror(rc)));
}
break;
}
d_parent->incRWTX();
env->incRWTX();
return result;
}
MDBROTransaction::MDBROTransaction(MDBEnv* parent, int flags) : d_parent(parent)
MDBRWTransaction::MDBRWTransaction(MDBEnv* parent, int flags):
MDBROTransaction(parent, openRWTransaction(parent, nullptr, flags)),
d_rw_cursors(new decltype(d_rw_cursors)::element_type())
{
if(d_parent->getRWTX())
}
MDBRWTransaction::~MDBRWTransaction()
{
abort();
}
void MDBRWTransaction::commit()
{
closeRORWCursors();
if (!d_txn) {
return;
}
if(int rc = mdb_txn_commit(d_txn)) {
throw std::runtime_error("committing: " + std::string(mdb_strerror(rc)));
}
environment().decRWTX();
d_txn = nullptr;
}
void MDBRWTransaction::abort()
{
closeRORWCursors();
if (!d_txn) {
return;
}
mdb_txn_abort(d_txn);
// prevent the RO destructor from cleaning up the transaction itself
environment().decRWTX();
d_txn = nullptr;
}
MDBROTransaction::MDBROTransaction(MDBEnv *parent, MDB_txn *txn):
d_parent(parent),
d_cursors(new decltype(d_cursors)::element_type()),
d_txn(txn)
{
}
MDB_txn *MDBROTransaction::openROTransaction(MDBEnv *env, MDB_txn *parent, int flags)
{
if(env->getRWTX())
throw std::runtime_error("Duplicate RO transaction");
/*
A transaction and its cursors must only be used by a single thread, and a thread may only have a single transaction at a time. If MDB_NOTLS is in use, this does not apply to read-only transactions. */
MDB_txn *result = nullptr;
for(int tries =0 ; tries < 3; ++tries) { // it might happen twice, who knows
if(int rc=mdb_txn_begin(d_parent->d_env, 0, MDB_RDONLY | flags, &d_txn)) {
if(int rc=mdb_txn_begin(env->d_env, parent, MDB_RDONLY | flags, &result)) {
if(rc == MDB_MAP_RESIZED && tries < 2) {
// "If the mapsize is increased by another process (..) mdb_txn_begin() will return MDB_MAP_RESIZED.
// call mdb_env_set_mapsize with a size of zero to adopt the new size."
mdb_env_set_mapsize(d_parent->d_env, 0);
mdb_env_set_mapsize(env->d_env, 0);
continue;
}
@ -193,7 +241,56 @@ MDBROTransaction::MDBROTransaction(MDBEnv* parent, int flags) : d_parent(parent)
}
break;
}
d_parent->incROTX();
env->incROTX();
return result;
}
void MDBROTransaction::closeROCursors()
{
if (!d_cursors) {
return;
}
// we need to move the vector away to ensure that the cursors don’t mess with our iteration.
std::vector<MDBROCursor*> buf;
std::swap(*d_cursors, buf);
for (auto &cursor: buf) {
cursor->close();
}
}
MDBROTransaction::MDBROTransaction(MDBEnv *parent, int flags):
MDBROTransaction(parent, openROTransaction(parent, nullptr, flags))
{
}
MDBROTransaction::~MDBROTransaction()
{
// this is safe because C++ will not call overrides of virtual methods in destructors.
commit();
}
void MDBROTransaction::abort()
{
closeROCursors();
// if d_txn is non-nullptr here, either the transaction object was invalidated earlier (e.g. by moving from it), or it is an RW transaction which has already cleaned up the d_txn pointer (with an abort).
if (d_txn) {
d_parent->decROTX();
mdb_txn_abort(d_txn); // this appears to work better than abort for r/o database opening
d_txn = nullptr;
}
}
void MDBROTransaction::commit()
{
closeROCursors();
// if d_txn is non-nullptr here, either the transaction object was invalidated earlier (e.g. by moving from it), or it is an RW transaction which has already cleaned up the d_txn pointer (with an abort).
if (d_txn) {
d_parent->decROTX();
mdb_txn_commit(d_txn); // this appears to work better than abort for r/o database opening
d_txn = nullptr;
}
}
@ -205,9 +302,19 @@ void MDBRWTransaction::clear(MDB_dbi dbi)
}
}
MDBRWCursor MDBRWTransaction::getCursor(const MDBDbi& dbi)
MDBRWCursor MDBRWTransaction::getRWCursor(const MDBDbi& dbi)
{
return MDBRWCursor(this, dbi);
MDB_cursor *cursor;
int rc= mdb_cursor_open(d_txn, dbi, &cursor);
if(rc) {
throw std::runtime_error("Error creating RO cursor: "+std::string(mdb_strerror(rc)));
}
return MDBRWCursor(*d_rw_cursors, cursor);
}
MDBRWCursor MDBRWTransaction::getCursor(const MDBDbi &dbi)
{
return getRWCursor(dbi);
}
MDBROTransaction MDBEnv::getROTransaction()
@ -220,16 +327,31 @@ MDBRWTransaction MDBEnv::getRWTransaction()
}
void MDBRWTransaction::closeCursors()
void MDBRWTransaction::closeRWCursors()
{
for(auto& c : d_cursors)
c->close();
d_cursors.clear();
if (!d_rw_cursors) {
return;
}
decltype(d_rw_cursors)::element_type buf;
std::swap(*d_rw_cursors, buf);
for (auto &cursor: buf) {
cursor->close();
}
}
MDBROCursor MDBROTransaction::getCursor(const MDBDbi& dbi)
{
return MDBROCursor(this, dbi);
return getROCursor(dbi);
}
MDBROCursor MDBROTransaction::getROCursor(const MDBDbi &dbi)
{
MDB_cursor *cursor;
int rc= mdb_cursor_open(d_txn, dbi, &cursor);
if(rc) {
throw std::runtime_error("Error creating RO cursor: "+std::string(mdb_strerror(rc)));
}
return MDBROCursor(*d_cursors, cursor);
}

336
lmdb-safe.hh
View File

@ -9,6 +9,8 @@
#include <string>
#include <string.h>
#include <mutex>
#include <vector>
#include <algorithm>
// apple compiler somehow has string_view even in c++11!
#if __cplusplus < 201703L && !defined(__APPLE__)
@ -222,33 +224,57 @@ class MDBROCursor;
class MDBROTransaction
{
protected:
MDBROTransaction(MDBEnv *parent, MDB_txn *txn);
private:
static MDB_txn *openROTransaction(MDBEnv *env, MDB_txn *parent, int flags=0);
MDBEnv* d_parent;
std::unique_ptr<std::vector<MDBROCursor*>> d_cursors;
protected:
MDB_txn* d_txn;
void closeROCursors();
public:
explicit MDBROTransaction(MDBEnv* parent, int flags=0);
MDBROTransaction(MDBROTransaction&& rhs)
MDBROTransaction(const MDBROTransaction& src) = delete;
MDBROTransaction &operator=(const MDBROTransaction& src) = delete;
MDBROTransaction(MDBROTransaction&& rhs) noexcept:
d_parent(rhs.d_parent),
d_cursors(std::move(rhs.d_cursors)),
d_txn(rhs.d_txn)
{
rhs.d_parent = nullptr;
rhs.d_txn = nullptr;
}
MDBROTransaction &operator=(MDBROTransaction &&rhs) noexcept
{
if (d_txn) {
abort();
}
d_parent = rhs.d_parent;
d_txn = rhs.d_txn;
rhs.d_parent = 0;
rhs.d_txn = 0;
d_cursors = std::move(d_cursors);
rhs.d_txn = nullptr;
rhs.d_parent = nullptr;
return *this;
}
void reset()
{
// this does not free cursors
mdb_txn_reset(d_txn);
d_parent->decROTX();
}
/* ensure that we cannot move from subclasses, because that would be massively
* unsafe. */
template<typename T, typename _ = typename std::enable_if<std::is_base_of<MDBROTransaction, T>::value>::type>
MDBROTransaction(T&& rhs) = delete;
void renew()
{
if(d_parent->getROTX())
throw std::runtime_error("Duplicate RO transaction");
if(int rc = mdb_txn_renew(d_txn))
throw std::runtime_error("Renewing RO transaction: "+std::string(mdb_strerror(rc)));
d_parent->incROTX();
}
virtual ~MDBROTransaction();
virtual void abort();
virtual void commit();
int get(MDB_dbi dbi, const MDBInVal& key, MDBOutVal& val)
{
@ -280,22 +306,21 @@ public:
}
MDBROCursor getCursor(const MDBDbi&);
MDBROCursor getROCursor(const MDBDbi&);
~MDBROTransaction()
{
if(d_txn) {
d_parent->decROTX();
mdb_txn_commit(d_txn); // this appears to work better than abort for r/o database opening
}
}
operator MDB_txn*&()
operator MDB_txn*()
{
return d_txn;
}
MDBEnv* d_parent;
MDB_txn* d_txn;
inline operator bool() const {
return d_txn;
}
inline MDBEnv &environment()
{
return *d_parent;
}
};
/*
@ -304,12 +329,64 @@ public:
"If the parent transaction commits, the cursor must not be used again."
*/
template<class Transaction>
template<class Transaction, class T>
class MDBGenCursor
{
private:
std::vector<T*> *d_registry;
MDB_cursor* d_cursor;
public:
MDBGenCursor(std::vector<T*> &registry, MDB_cursor *cursor):
d_registry(&registry),
d_cursor(cursor)
{
registry.emplace_back(static_cast<T*>(this));
}
private:
void move_from(MDBGenCursor *src)
{
auto iter = std::find(d_registry->begin(),
d_registry->end(),
src);
if (iter != d_registry->end()) {
*iter = static_cast<T*>(this);
} else {
d_registry->emplace_back(static_cast<T*>(this));
}
}
public:
MDBGenCursor(const MDBGenCursor &src) = delete;
MDBGenCursor(MDBGenCursor &&src) noexcept:
d_registry(src.d_registry),
d_cursor(src.d_cursor)
{
move_from(&src);
src.d_registry = nullptr;
src.d_cursor = nullptr;
}
MDBGenCursor &operator=(const MDBGenCursor &src) = delete;
MDBGenCursor &operator=(MDBGenCursor &&src) noexcept
{
d_registry = src.d_registry;
d_cursor = src.d_cursor;
move_from(&src);
src.d_registry = nullptr;
src.d_cursor = nullptr;
return *this;
}
~MDBGenCursor()
{
close();
}
public:
MDBGenCursor(Transaction *t) : d_parent(t)
{}
int get(MDBOutVal& key, MDBOutVal& data, MDB_cursor_op op)
{
int rc = mdb_cursor_get(d_cursor, &key.d_mdbval, &data.d_mdbval, op);
@ -377,101 +454,83 @@ public:
return currentlast(key, data, MDB_FIRST);
}
operator MDB_cursor*&()
operator MDB_cursor*()
{
return d_cursor;
}
MDB_cursor* d_cursor;
Transaction* d_parent;
};
class MDBROCursor : public MDBGenCursor<MDBROTransaction>
{
public:
MDBROCursor(MDBROTransaction* parent, const MDB_dbi& dbi) : MDBGenCursor<MDBROTransaction>(parent)
operator bool() const
{
int rc= mdb_cursor_open(d_parent->d_txn, dbi, &d_cursor);
if(rc) {
throw std::runtime_error("Error creating RO cursor: "+std::string(mdb_strerror(rc)));
}
}
MDBROCursor(MDBROCursor&& rhs) : MDBGenCursor<MDBROTransaction>(rhs.d_parent)
{
d_cursor = rhs.d_cursor;
rhs.d_cursor=0;
return d_cursor;
}
void close()
{
mdb_cursor_close(d_cursor);
d_cursor=0;
}
~MDBROCursor()
{
if(d_cursor)
if (d_registry) {
auto iter = std::find(d_registry->begin(),
d_registry->end(),
static_cast<T*>(this));
if (iter != d_registry->end()) {
d_registry->erase(iter);
}
d_registry = nullptr;
}
if (d_cursor) {
mdb_cursor_close(d_cursor);
d_cursor = nullptr;
}
}
};
class MDBROCursor : public MDBGenCursor<MDBROTransaction, MDBROCursor>
{
public:
using MDBGenCursor<MDBROTransaction, MDBROCursor>::MDBGenCursor;
MDBROCursor(const MDBROCursor &src) = delete;
MDBROCursor(MDBROCursor &&src) = default;
MDBROCursor &operator=(const MDBROCursor &src) = delete;
MDBROCursor &operator=(MDBROCursor &&src) = default;
~MDBROCursor() = default;
};
class MDBRWCursor;
class MDBRWTransaction
class MDBRWTransaction: public MDBROTransaction
{
private:
static MDB_txn *openRWTransaction(MDBEnv* env, MDB_txn *parent, int flags);
private:
std::unique_ptr<std::vector<MDBRWCursor*>> d_rw_cursors;
void closeRWCursors();
inline void closeRORWCursors() {
closeROCursors();
closeRWCursors();
}
public:
explicit MDBRWTransaction(MDBEnv* parent, int flags=0);
MDBRWTransaction(MDBRWTransaction&& rhs)
MDBRWTransaction(MDBRWTransaction&& rhs) noexcept:
MDBROTransaction(std::move(static_cast<MDBROTransaction&>(rhs))),
d_rw_cursors(std::move(rhs.d_rw_cursors))
{
d_parent = rhs.d_parent;
d_txn = rhs.d_txn;
rhs.d_parent = 0;
rhs.d_txn = 0;
}
MDBRWTransaction& operator=(MDBRWTransaction&& rhs)
MDBRWTransaction &operator=(MDBRWTransaction&& rhs) noexcept
{
if(d_txn)
abort();
d_parent = rhs.d_parent;
d_txn = rhs.d_txn;
rhs.d_parent = 0;
rhs.d_txn = 0;
MDBROTransaction::operator=(std::move(static_cast<MDBROTransaction&>(rhs)));
d_rw_cursors = std::move(rhs.d_rw_cursors);
return *this;
}
~MDBRWTransaction()
{
if(d_txn) {
d_parent->decRWTX();
closeCursors();
mdb_txn_abort(d_txn); // XXX check response?
}
}
void closeCursors();
~MDBRWTransaction() override;
void commit()
{
closeCursors();
if(int rc = mdb_txn_commit(d_txn)) {
throw std::runtime_error("committing: " + std::string(mdb_strerror(rc)));
}
d_parent->decRWTX();
d_txn=0;
}
void abort()
{
closeCursors();
mdb_txn_abort(d_txn); // XXX check error?
d_txn = 0;
d_parent->decRWTX();
}
void commit() override;
void abort() override;
void clear(MDB_dbi dbi);
@ -529,79 +588,31 @@ public:
MDBDbi openDB(string_view dbname, int flags)
{
return MDBDbi(d_parent->d_env, d_txn, dbname, flags);
return MDBDbi(environment().d_env, d_txn, dbname, flags);
}
MDBRWCursor getRWCursor(const MDBDbi&);
MDBRWCursor getCursor(const MDBDbi&);
void reportCursor(MDBRWCursor* child)
{
d_cursors.insert(child);
}
void unreportCursor(MDBRWCursor* child)
{
d_cursors.erase(child);
}
void reportCursorMove(MDBRWCursor* from, MDBRWCursor* to)
{
d_cursors.erase(from);
d_cursors.insert(to);
}
operator MDB_txn*&()
{
return d_txn;
}
std::set<MDBRWCursor*> d_cursors;
MDBEnv* d_parent;
MDB_txn* d_txn;
};
/* "A cursor in a write-transaction can be closed before its transaction ends, and will otherwise be closed when its transaction ends"
This is a problem for us since it may means we are closing the cursor twice, which is bad
*/
class MDBRWCursor : public MDBGenCursor<MDBRWTransaction>
class MDBRWCursor : public MDBGenCursor<MDBRWTransaction, MDBRWCursor>
{
public:
MDBRWCursor(MDBRWTransaction* parent, const MDB_dbi& dbi) : MDBGenCursor<MDBRWTransaction>(parent)
{
int rc= mdb_cursor_open(d_parent->d_txn, dbi, &d_cursor);
if(rc) {
throw std::runtime_error("Error creating RW cursor: "+std::string(mdb_strerror(rc)));
}
d_parent->reportCursor(this);
}
MDBRWCursor(MDBRWCursor&& rhs) : MDBGenCursor<MDBRWTransaction>(rhs.d_parent)
{
d_cursor = rhs.d_cursor;
rhs.d_cursor=0;
d_parent->reportCursorMove(&rhs, this);
}
void close()
{
if(d_cursor)
mdb_cursor_close(d_cursor);
d_cursor=0;
}
~MDBRWCursor()
{
if(d_cursor)
mdb_cursor_close(d_cursor);
d_parent->unreportCursor(this);
}
using MDBGenCursor<MDBRWTransaction, MDBRWCursor>::MDBGenCursor;
MDBRWCursor(const MDBRWCursor &src) = default;
MDBRWCursor(MDBRWCursor &&src) = default;
MDBRWCursor &operator=(const MDBRWCursor &src) = default;
MDBRWCursor &operator=(MDBRWCursor &&src) = default;
~MDBRWCursor() = default;
void put(const MDBOutVal& key, const MDBInVal& data)
{
int rc = mdb_cursor_put(d_cursor,
const_cast<MDB_val*>(&key.d_mdbval),
const_cast<MDB_val*>(&data.d_mdbval), MDB_CURRENT);
int rc = mdb_cursor_put(*this,
const_cast<MDB_val*>(&key.d_mdbval),
const_cast<MDB_val*>(&data.d_mdbval), MDB_CURRENT);
if(rc)
throw std::runtime_error("mdb_cursor_put: " + std::string(mdb_strerror(rc)));
}
@ -610,14 +621,15 @@ public:
int put(const MDBOutVal& key, const MDBOutVal& data, int flags=0)
{
// XXX check errors
return mdb_cursor_put(d_cursor,
return mdb_cursor_put(*this,
const_cast<MDB_val*>(&key.d_mdbval),
const_cast<MDB_val*>(&data.d_mdbval), flags);
}
int del(int flags=0)
{
return mdb_cursor_del(d_cursor, flags);
return mdb_cursor_del(*this, flags);
}
};

2
lmdb-typed.cc
View File

@ -2,7 +2,7 @@
unsigned int MDBGetMaxID(MDBRWTransaction& txn, MDBDbi& dbi)
{
auto cursor = txn.getCursor(dbi);
auto cursor = txn.getRWCursor(dbi);
MDBOutVal maxidval, maxcontent;
unsigned int maxid{0};
if(!cursor.get(maxidval, maxcontent, MDB_LAST)) {

2
lmdb-typed.hh
View File

@ -633,7 +633,7 @@ public:
//! clear database & indexes (by hand!)
void clear()
{
auto cursor = d_txn->getCursor(d_parent->d_main);
auto cursor = d_txn->getRWCursor(d_parent->d_main);
bool first = true;
MDBOutVal key, data;
while(!cursor.get(key, data, first ? MDB_FIRST : MDB_NEXT)) {

4
multi-example.cc
View File

@ -1,6 +1,8 @@
#include "lmdb-safe.hh"
using namespace std;
#include <unistd.h>
int main()
{
unlink("./multi");
@ -27,7 +29,7 @@ int main()
txn.commit();
txn = env->getRWTransaction();
auto cursor = txn.getCursor(dbi);
auto cursor = txn.getRWCursor(dbi);
MDBOutVal key, data;

2
rel-example.cc
View File

@ -27,7 +27,7 @@ struct Record
static unsigned int getMaxID(MDBRWTransaction& txn, MDBDbi& dbi)
{
auto cursor = txn.getCursor(dbi);
auto cursor = txn.getRWCursor(dbi);
MDBOutVal maxidval, maxcontent;
unsigned int maxid{0};
if(!cursor.get(maxidval, maxcontent, MDB_LAST)) {

49
test-basic.cc
View File

@ -13,12 +13,12 @@ TEST_CASE("Most basic tests", "[mostbasic]") {
REQUIRE(1);
MDBDbi main = env.openDB("", MDB_CREATE);
auto txn = env.getRWTransaction();
MDBOutVal out;
REQUIRE(txn.get(main, "lmdb", out) == MDB_NOTFOUND);
txn.put(main, "lmdb", "hot");
REQUIRE(txn.get(main, "lmdb", out) == 0);
@ -36,18 +36,18 @@ TEST_CASE("Range tests", "[range]") {
REQUIRE(1);
MDBDbi main = env.openDB("", MDB_CREATE);
auto txn = env.getRWTransaction();
MDBOutVal out;
REQUIRE(txn.get(main, "lmdb", out) == MDB_NOTFOUND);
txn.put(main, "bert", "hubert");
txn.put(main, "bertt", "1975");
txn.put(main, "bertt", "1975");
txn.put(main, "berthubert", "lmdb");
txn.put(main, "bert1", "one");
txn.put(main, "beru", "not");
txn.put(main, "beru", "not");
{
auto cursor = txn.getCursor(main);
MDBInVal bert("bert");
@ -64,11 +64,11 @@ TEST_CASE("Range tests", "[range]") {
REQUIRE(key.get<string>() == "berthubert");
REQUIRE(val.get<string>() == "lmdb");
REQUIRE(cursor.lower_bound("kees", key, val) == MDB_NOTFOUND);
REQUIRE(cursor.lower_bound("kees", key, val) == MDB_NOTFOUND);
txn.commit();
}
auto rotxn = env.getROTransaction();
{
auto cursor = rotxn.getCursor(main);
@ -86,7 +86,34 @@ TEST_CASE("Range tests", "[range]") {
REQUIRE(key.get<string>() == "berthubert");
REQUIRE(val.get<string>() == "lmdb");
REQUIRE(cursor.lower_bound("kees", key, val) == MDB_NOTFOUND);
REQUIRE(cursor.lower_bound("kees", key, val) == MDB_NOTFOUND);
}
}
TEST_CASE("moving transactions")
{
unlink("./tests");
MDBEnv env("./tests", MDB_NOSUBDIR, 0600);
REQUIRE(1);
MDBDbi main = env.openDB("", MDB_CREATE);
auto txn = env.getRWTransaction();
MDBOutVal out;
REQUIRE(txn.get(main, "lmdb", out) == MDB_NOTFOUND);
txn.put(main, "bert", "hubert");
txn.put(main, "bertt", "1975");
txn.put(main, "berthubert", "lmdb");
txn.put(main, "bert1", "one");
txn.put(main, "beru", "not");
auto cursor = txn.getCursor(main);
auto txn2 = std::move(txn);
{
auto cursor2 = std::move(cursor);
}
}