Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve LMDBBackend reliability #15175

Merged
merged 1 commit into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions modules/lmdbbackend/lmdbbackend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,23 @@ LMDBBackend::LMDBBackend(const std::string& suffix)
d_dolog = ::arg().mustDo("query-logging");
}

LMDBBackend::~LMDBBackend()
{
// LMDB internals require that, if we have multiple transactions active,
// we destroy them in the reverse order of their creation, thus we can't
// let the default destructor take care of d_rotxn and d_rwtxn.
if (d_txnorder) {
// RO transaction more recent than RW transaction
d_rotxn.reset();
d_rwtxn.reset();
}
else {
// RW transaction more recent than RO transaction
d_rwtxn.reset();
d_rotxn.reset();
}
}

namespace boost
{
namespace serialization
Expand Down Expand Up @@ -1073,6 +1090,7 @@ bool LMDBBackend::startTransaction(const DNSName& domain, int domain_id)
throw DBException("Attempt to start a transaction while one was open already");
}
d_rwtxn = getRecordsRWTransaction(real_id);
d_txnorder = false;

d_transactiondomain = domain;
d_transactiondomainid = real_id;
Expand Down Expand Up @@ -1443,6 +1461,7 @@ bool LMDBBackend::list(const DNSName& target, int /* id */, bool include_disable
}

d_rotxn = getRecordsROTransaction(di.id, d_rwtxn);
d_txnorder = true;
d_getcursor = std::make_shared<MDBROCursor>(d_rotxn->txn->getCursor(d_rotxn->db->dbi));

compoundOrdername co;
Expand Down Expand Up @@ -1503,6 +1522,7 @@ void LMDBBackend::lookup(const QType& type, const DNSName& qdomain, int zoneId,
}
// cout<<"get will look for "<<relqname<< " in zone "<<hunt<<" with id "<<zoneId<<" and type "<<type.toString()<<endl;
d_rotxn = getRecordsROTransaction(zoneId, d_rwtxn);
d_txnorder = true;

compoundOrdername co;
d_getcursor = std::make_shared<MDBROCursor>(d_rotxn->txn->getCursor(d_rotxn->db->dbi));
Expand Down
2 changes: 2 additions & 0 deletions modules/lmdbbackend/lmdbbackend.hh
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class LMDBBackend : public DNSBackend
{
public:
explicit LMDBBackend(const string& suffix = "");
~LMDBBackend();

bool list(const DNSName& target, int id, bool include_disabled) override;

Expand Down Expand Up @@ -305,6 +306,7 @@ private:

shared_ptr<RecordsROTransaction> d_rotxn; // for lookup and list
shared_ptr<RecordsRWTransaction> d_rwtxn; // for feedrecord within begin/aborttransaction
bool d_txnorder{false}; // whether d_rotxn is more recent than d_rwtxn
std::shared_ptr<RecordsRWTransaction> getRecordsRWTransaction(uint32_t id);
std::shared_ptr<RecordsROTransaction> getRecordsROTransaction(uint32_t id, const std::shared_ptr<LMDBBackend::RecordsRWTransaction>& rwtxn = nullptr);
int genChangeDomain(const DNSName& domain, const std::function<void(DomainInfo&)>& func);
Expand Down