From: Miod Vallat Date: Fri, 31 Jan 2025 08:58:03 +0000 (+0100) Subject: Add infrastructure for backends to report whether file creation occurred. X-Git-Tag: dnsdist-2.0.0-alpha1~136^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d0e14056034a3add3fcb1b09b5cf84517c0a84a7;p=thirdparty%2Fpdns.git Add infrastructure for backends to report whether file creation occurred. This only concerns the LMDB backend, as no other backend is able to create local storage. To be used shortly in a pdnsutil near you. --- diff --git a/ext/lmdb-safe/lmdb-safe.cc b/ext/lmdb-safe/lmdb-safe.cc index f0db35fd81..dc081d25f6 100644 --- a/ext/lmdb-safe/lmdb-safe.cc +++ b/ext/lmdb-safe/lmdb-safe.cc @@ -80,17 +80,39 @@ namespace LMDBLS { #endif /* #ifndef DNSDIST */ +std::atomic MDBDbi::d_creationCount{0}; + MDBDbi::MDBDbi(MDB_env* /* env */, MDB_txn* txn, const string_view dbname, int flags) : d_dbi(-1) { // A transaction that uses this function must finish (either commit or abort) before any other transaction in the process may use this function. - int rc = mdb_dbi_open(txn, dbname.empty() ? 0 : &dbname[0], flags, &d_dbi); - if(rc) - throw std::runtime_error("Unable to open named database: " + MDBError(rc)); + int ret = MDBDbi::mdb_dbi_open(txn, dbname.empty() ? nullptr : dbname.data(), flags, &d_dbi); + if (ret != 0) { + throw std::runtime_error("Unable to open named database: " + MDBError(ret)); + } // Database names are keys in the unnamed database, and may be read but not written. } +// This is a wrapper around the real mdb_dbi_open(), in order to track creation +// of new files. +int MDBDbi::mdb_dbi_open(MDB_txn* txn, const char* name, unsigned int flags, MDB_dbi* dbi) +{ + if ((flags & MDB_CREATE) != 0) { + flags &= ~MDB_CREATE; + int retval = ::mdb_dbi_open(txn, name, flags, dbi); + if (retval == MDB_NOTFOUND) { + flags |= MDB_CREATE; + retval = ::mdb_dbi_open(txn, name, flags, dbi); + if (retval == 0) { + d_creationCount++; + } + } + return retval; + } + return ::mdb_dbi_open(txn, name, flags, dbi); +} + MDBEnv::MDBEnv(const char* fname, int flags, int mode, uint64_t mapsizeMB) { mdb_env_create(&d_env); diff --git a/ext/lmdb-safe/lmdb-safe.hh b/ext/lmdb-safe/lmdb-safe.hh index 07e96cffc1..54850d954a 100644 --- a/ext/lmdb-safe/lmdb-safe.hh +++ b/ext/lmdb-safe/lmdb-safe.hh @@ -13,6 +13,7 @@ #include #include #include +#include #include #ifndef DNSDIST @@ -56,6 +57,9 @@ public: } MDB_dbi d_dbi; + + static int mdb_dbi_open(MDB_txn *, const char *, unsigned int, MDB_dbi *); + static std::atomic d_creationCount; }; class MDBRWTransactionImpl; diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc index 988639b147..d75ec27687 100644 --- a/modules/lmdbbackend/lmdbbackend.cc +++ b/modules/lmdbbackend/lmdbbackend.cc @@ -104,7 +104,7 @@ std::pair LMDBBackend::getSchemaVersionAndShards(std::string MDB_dbi dbi; { - int retCode = mdb_dbi_open(txn, "pdns", 0, &dbi); + int retCode = MDBDbi::mdb_dbi_open(txn, "pdns", 0, &dbi); if (retCode != 0) { if (retCode == MDB_NOTFOUND) { // this means nothing has been inited yet @@ -432,7 +432,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) MDB_dbi shdbi = 0; - const auto dbiOpenRc = mdb_dbi_open(shtxn, "records", 0, &shdbi); + const auto dbiOpenRc = MDBDbi::mdb_dbi_open(shtxn, "records", 0, &shdbi); if (dbiOpenRc != 0) { if (dbiOpenRc == MDB_NOTFOUND) { mdb_txn_abort(shtxn); @@ -446,7 +446,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) MDB_dbi shdbi2 = 0; - if (mdb_dbi_open(shtxn, "records_v5", MDB_CREATE, &shdbi2) != 0) { + if (MDBDbi::mdb_dbi_open(shtxn, "records_v5", MDB_CREATE, &shdbi2) != 0) { mdb_dbi_close(shenv, shdbi); mdb_txn_abort(shtxn); mdb_env_close(shenv); @@ -480,14 +480,14 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) std::string tdbname = dbname + "_v5"; // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) - if (mdb_dbi_open(txn, dbname.c_str(), 0, &fromtypeddbi[index]) != 0) { + if (MDBDbi::mdb_dbi_open(txn, dbname.c_str(), 0, &fromtypeddbi[index]) != 0) { mdb_txn_abort(txn); mdb_env_close(env); - throw std::runtime_error("mdb_dbi_open typeddbi failed"); + throw std::runtime_error("MDBDbi::mdb_dbi_open typeddbi failed"); } // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) - if (mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &totypeddbi[index]) != 0) { + if (MDBDbi::mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &totypeddbi[index]) != 0) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, fromtypeddbi[index]); mdb_txn_abort(txn); @@ -527,14 +527,14 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) std::string tdbname = dbname + "_v5_0"; // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) - if (mdb_dbi_open(txn, fdbname.c_str(), 0, &fromindexdbi[index]) != 0) { + if (MDBDbi::mdb_dbi_open(txn, fdbname.c_str(), 0, &fromindexdbi[index]) != 0) { mdb_txn_abort(txn); mdb_env_close(env); throw std::runtime_error("mdb_dbi_open indexdbi failed"); } // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) - if (mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &toindexdbi[index]) != 0) { + if (MDBDbi::mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &toindexdbi[index]) != 0) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, fromindexdbi[index]); mdb_txn_abort(txn); @@ -566,7 +566,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) MDB_dbi dbi = 0; // finally, migrate the pdns db - if (mdb_dbi_open(txn, "pdns", 0, &dbi) != 0) { + if (MDBDbi::mdb_dbi_open(txn, "pdns", 0, &dbi) != 0) { mdb_txn_abort(txn); mdb_env_close(env); throw std::runtime_error("mdb_dbi_open pdns failed"); @@ -2831,6 +2831,16 @@ string LMDBBackend::directBackendCmd(const string& query) return "unknown lmdbbackend command\n"; } +bool LMDBBackend::hasCreatedLocalFiles() const +{ + // Since the lmdb file creation counter is global, if multiple LMDB backends + // are used, they may end up all reporting having created files even if + // not all of them did. + // But since this information is for the sake of pdnsutil, this is not + // really a problem. + return MDBDbi::d_creationCount != 0; +} + class LMDBFactory : public BackendFactory { public: diff --git a/modules/lmdbbackend/lmdbbackend.hh b/modules/lmdbbackend/lmdbbackend.hh index 70a712bf78..0b40b5284b 100644 --- a/modules/lmdbbackend/lmdbbackend.hh +++ b/modules/lmdbbackend/lmdbbackend.hh @@ -150,6 +150,8 @@ public: // other string directBackendCmd(const string& query) override; + bool hasCreatedLocalFiles() const override; + // functions to use without constructing a backend object static std::pair getSchemaVersionAndShards(std::string& filename); static bool upgradeToSchemav5(std::string& filename); diff --git a/pdns/dnsbackend.hh b/pdns/dnsbackend.hh index 7b7e4c2117..221881ad85 100644 --- a/pdns/dnsbackend.hh +++ b/pdns/dnsbackend.hh @@ -465,6 +465,12 @@ public: return false; } + //! Returns whether backend operations have caused files to be created. + virtual bool hasCreatedLocalFiles() const + { + return false; + } + const string& getPrefix() { return d_prefix; }; protected: diff --git a/pdns/ueberbackend.cc b/pdns/ueberbackend.cc index a0e986115a..3d8b41e592 100644 --- a/pdns/ueberbackend.cc +++ b/pdns/ueberbackend.cc @@ -886,6 +886,11 @@ bool UeberBackend::searchComments(const string& pattern, size_t maxResults, vect return ret; } +bool UeberBackend::hasCreatedLocalFiles() +{ + return std::any_of(backends.begin(), backends.end(), [](std::unique_ptr& backend) { return backend->hasCreatedLocalFiles(); }); +} + AtomicCounter UeberBackend::handle::instances(0); UeberBackend::handle::handle() diff --git a/pdns/ueberbackend.hh b/pdns/ueberbackend.hh index 0565abfaea..4705868028 100644 --- a/pdns/ueberbackend.hh +++ b/pdns/ueberbackend.hh @@ -141,6 +141,8 @@ public: bool inTransaction(); + bool hasCreatedLocalFiles(); + private: handle d_handle; vector d_answers;