From 06d3d7355d1b0ec05e61d2e7fe67f8d7153c1ff9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Jul 2003 16:05:35 +0000 Subject: [PATCH] * Enclose most operations that update the database in transactions. * Open all database tables (Db objects) at initialisation time, not every time they are used. This is necessary because tables have to outlive all transactions that refer to them. --- src/db.cc | 131 +++++++++++++++++++++++++++-------------------- src/db.hh | 25 +++++---- src/globals.cc | 16 +++--- src/globals.hh | 8 +-- src/normalise.cc | 4 +- src/store.cc | 57 ++++++++++++++------- 6 files changed, 145 insertions(+), 96 deletions(-) diff --git a/src/db.cc b/src/db.cc index 77ad93a86..61ecb203a 100644 --- a/src/db.cc +++ b/src/db.cc @@ -5,15 +5,6 @@ /* Wrapper class to ensure proper destruction. */ -class DestroyDb -{ - Db * db; -public: - DestroyDb(Db * _db) : db(_db) { } - ~DestroyDb() { db->close(0); delete db; } -}; - - class DestroyDbc { Dbc * dbc; @@ -38,24 +29,39 @@ Transaction::Transaction() Transaction::Transaction(Database & db) { db.requireEnv(); - db.env->txn_begin(0, &txn, 0); + try { + db.env->txn_begin(0, &txn, 0); + } catch (DbException e) { rethrow(e); } } Transaction::~Transaction() { - if (txn) { - txn->abort(); - txn = 0; - } + if (txn) abort(); } void Transaction::commit() { if (!txn) throw Error("commit called on null transaction"); - txn->commit(0); + debug(format("committing transaction %1%") % (void *) txn); + DbTxn * txn2 = txn; txn = 0; + try { + txn2->commit(0); + } catch (DbException e) { rethrow(e); } +} + + +void Transaction::abort() +{ + if (!txn) throw Error("abort called on null transaction"); + debug(format("aborting transaction %1%") % (void *) txn); + DbTxn * txn2 = txn; + txn = 0; + try { + txn2->abort(); + } catch (DbException e) { rethrow(e); } } @@ -65,28 +71,18 @@ void Database::requireEnv() } -Db * Database::openDB(const Transaction & txn, - const string & table, bool create) +Db * Database::getDb(TableId table) { - requireEnv(); - - Db * db = new Db(env, 0); - - try { - // !!! fixme when switching to BDB 4.1: use txn. - db->open(table.c_str(), 0, - DB_HASH, create ? DB_CREATE : 0, 0666); - } catch (...) { - delete db; - throw; - } - - return db; + map::iterator i = tables.find(table); + if (i == tables.end()) + throw Error("unknown table id"); + return i->second; } Database::Database() : env(0) + , nextId(1) { } @@ -95,8 +91,23 @@ Database::~Database() { if (env) { debug(format("closing database environment")); - env->txn_checkpoint(0, 0, 0); - env->close(0); + + try { + + for (map::iterator i = tables.begin(); + i != tables.end(); i++) + { + debug(format("closing table %1%") % i->first); + Db * db = i->second; + db->close(0); + delete db; + } + + env->txn_checkpoint(0, 0, 0); + env->close(0); + + } catch (DbException e) { rethrow(e); } + delete env; } } @@ -112,8 +123,9 @@ void Database::open(const string & path) env->set_lg_bsize(32 * 1024); /* default */ env->set_lg_max(256 * 1024); /* must be > 4 * lg_bsize */ + env->set_lk_detect(DB_LOCK_DEFAULT); - env->open(path.c_str(), + env->open(path.c_str(), DB_INIT_LOCK | DB_INIT_LOG | DB_INIT_MPOOL | DB_INIT_TXN | DB_CREATE, 0666); @@ -122,22 +134,36 @@ void Database::open(const string & path) } -void Database::createTable(const string & table) +TableId Database::openTable(const string & tableName) { + requireEnv(); + TableId table = nextId++; + try { - Db * db = openDB(noTxn, table, true); - DestroyDb destroyDb(db); + + Db * db = new Db(env, 0); + + try { + // !!! fixme when switching to BDB 4.1: use txn. + db->open(tableName.c_str(), 0, DB_HASH, DB_CREATE, 0666); + } catch (...) { + delete db; + throw; + } + + tables[table] = db; + } catch (DbException e) { rethrow(e); } + + return table; } -bool Database::queryString(const Transaction & txn, const string & table, +bool Database::queryString(const Transaction & txn, TableId table, const string & key, string & data) { try { - - Db * db = openDB(txn, table, false); - DestroyDb destroyDb(db); + Db * db = getDb(table); Dbt kt((void *) key.c_str(), key.length()); Dbt dt; @@ -156,7 +182,7 @@ bool Database::queryString(const Transaction & txn, const string & table, } -bool Database::queryStrings(const Transaction & txn, const string & table, +bool Database::queryStrings(const Transaction & txn, TableId table, const string & key, Strings & data) { string d; @@ -190,13 +216,11 @@ bool Database::queryStrings(const Transaction & txn, const string & table, } -void Database::setString(const Transaction & txn, const string & table, +void Database::setString(const Transaction & txn, TableId table, const string & key, const string & data) { try { - Db * db = openDB(txn, table, false); - DestroyDb destroyDb(db); - + Db * db = getDb(table); Dbt kt((void *) key.c_str(), key.length()); Dbt dt((void *) data.c_str(), data.length()); db->put(txn.txn, &kt, &dt, 0); @@ -204,7 +228,7 @@ void Database::setString(const Transaction & txn, const string & table, } -void Database::setStrings(const Transaction & txn, const string & table, +void Database::setStrings(const Transaction & txn, TableId table, const string & key, const Strings & data) { string d; @@ -227,28 +251,25 @@ void Database::setStrings(const Transaction & txn, const string & table, } -void Database::delPair(const Transaction & txn, const string & table, +void Database::delPair(const Transaction & txn, TableId table, const string & key) { try { - Db * db = openDB(txn, table, false); - DestroyDb destroyDb(db); + Db * db = getDb(table); Dbt kt((void *) key.c_str(), key.length()); db->del(txn.txn, &kt, 0); } catch (DbException e) { rethrow(e); } } -void Database::enumTable(const Transaction & txn, const string & table, +void Database::enumTable(const Transaction & txn, TableId table, Strings & keys) { try { - - Db * db = openDB(txn, table, false); - DestroyDb destroyDb(db); + Db * db = getDb(table); Dbc * dbc; - db->cursor(0, &dbc, 0); + db->cursor(txn.txn, &dbc, 0); DestroyDbc destroyDbc(dbc); Dbt kt, dt; diff --git a/src/db.hh b/src/db.hh index 57b6e4d8e..4bac943e5 100644 --- a/src/db.hh +++ b/src/db.hh @@ -3,6 +3,7 @@ #include #include +#include #include @@ -26,6 +27,7 @@ public: Transaction(Database & _db); ~Transaction(); + void abort(); void commit(); }; @@ -33,6 +35,9 @@ public: #define noTxn Transaction() +typedef unsigned int TableId; /* table handles */ + + class Database { friend class Transaction; @@ -40,10 +45,12 @@ class Database private: DbEnv * env; + TableId nextId; + map tables; + void requireEnv(); - Db * openDB(const Transaction & txn, - const string & table, bool create); + Db * getDb(TableId table); public: Database(); @@ -51,24 +58,24 @@ public: void open(const string & path); - void createTable(const string & table); + TableId openTable(const string & table); - bool queryString(const Transaction & txn, const string & table, + bool queryString(const Transaction & txn, TableId table, const string & key, string & data); - bool queryStrings(const Transaction & txn, const string & table, + bool queryStrings(const Transaction & txn, TableId table, const string & key, Strings & data); - void setString(const Transaction & txn, const string & table, + void setString(const Transaction & txn, TableId table, const string & key, const string & data); - void setStrings(const Transaction & txn, const string & table, + void setStrings(const Transaction & txn, TableId table, const string & key, const Strings & data); - void delPair(const Transaction & txn, const string & table, + void delPair(const Transaction & txn, TableId table, const string & key); - void enumTable(const Transaction & txn, const string & table, + void enumTable(const Transaction & txn, TableId table, Strings & keys); }; diff --git a/src/globals.cc b/src/globals.cc index 8c3ec3828..1ec0c4f9b 100644 --- a/src/globals.cc +++ b/src/globals.cc @@ -5,10 +5,10 @@ Database nixDB; -string dbPath2Id = "path2id"; -string dbId2Paths = "id2paths"; -string dbSuccessors = "successors"; -string dbSubstitutes = "substitutes"; +TableId dbPath2Id; +TableId dbId2Paths; +TableId dbSuccessors; +TableId dbSubstitutes; string nixStore = "/UNINIT"; @@ -20,13 +20,13 @@ string nixDBPath = "/UNINIT"; void openDB() { nixDB.open(nixDBPath); + dbPath2Id = nixDB.openTable("path2id"); + dbId2Paths = nixDB.openTable("id2paths"); + dbSuccessors = nixDB.openTable("successors"); + dbSubstitutes = nixDB.openTable("substitutes"); } void initDB() { - nixDB.createTable(dbPath2Id); - nixDB.createTable(dbId2Paths); - nixDB.createTable(dbSuccessors); - nixDB.createTable(dbSubstitutes); } diff --git a/src/globals.hh b/src/globals.hh index 9df827622..2c4d33920 100644 --- a/src/globals.hh +++ b/src/globals.hh @@ -17,13 +17,13 @@ extern Database nixDB; Each pair (p, id) records that path $p$ contains an expansion of $id$. */ -extern string dbPath2Id; +extern TableId dbPath2Id; /* dbId2Paths :: FSId -> [Path] A mapping from ids to lists of paths. */ -extern string dbId2Paths; +extern TableId dbId2Paths; /* dbSuccessors :: FSId -> FSId @@ -35,7 +35,7 @@ extern string dbId2Paths; Note that a term $y$ is successor of $x$ iff there exists a sequence of rewrite steps that rewrites $x$ into $y$. */ -extern string dbSuccessors; +extern TableId dbSuccessors; /* dbSubstitutes :: FSId -> [FSId] @@ -51,7 +51,7 @@ extern string dbSuccessors; this case might be an fstate expression that fetches the Nix archive. */ -extern string dbSubstitutes; +extern TableId dbSubstitutes; /* Path names. */ diff --git a/src/normalise.cc b/src/normalise.cc index 5a8cb9a0d..e8fc6fc55 100644 --- a/src/normalise.cc +++ b/src/normalise.cc @@ -9,7 +9,9 @@ void registerSuccessor(const FSId & id1, const FSId & id2) { - nixDB.setString(noTxn, dbSuccessors, id1, id2); + Transaction txn(nixDB); + nixDB.setString(txn, dbSuccessors, id1, id2); + txn.commit(); } diff --git a/src/store.cc b/src/store.cc index 3dc625a7b..6d7861d0b 100644 --- a/src/store.cc +++ b/src/store.cc @@ -32,6 +32,8 @@ struct CopySource : RestoreSource void copyPath(string src, string dst) { + debug(format("copying `%1%' to `%2%'") % src % dst); + /* Unfortunately C++ doesn't support coprocedures, so we have no nice way to chain CopySink and CopySource together. Instead we fork off a child to run the sink. (Fork-less platforms should @@ -96,54 +98,69 @@ void registerSubstitute(const FSId & srcId, const FSId & subId) /* For now, accept only one substitute per id. */ Strings subs; subs.push_back(subId); - nixDB.setStrings(noTxn, dbSubstitutes, srcId, subs); + + Transaction txn(nixDB); + nixDB.setStrings(txn, dbSubstitutes, srcId, subs); + txn.commit(); } void registerPath(const string & _path, const FSId & id) { string path(canonPath(_path)); + Transaction txn(nixDB); - nixDB.setString(noTxn, dbPath2Id, path, id); + debug(format("registering path `%1%' with id %2%") + % path % (string) id); + + string oldId; + if (nixDB.queryString(txn, dbPath2Id, path, oldId)) { + txn.abort(); + if (id != parseHash(oldId)) + throw Error(format("path `%1%' already contains id %2%") + % path % oldId); + return; + } + + nixDB.setString(txn, dbPath2Id, path, id); Strings paths; - nixDB.queryStrings(noTxn, dbId2Paths, id, paths); /* non-existence = ok */ - - for (Strings::iterator it = paths.begin(); - it != paths.end(); it++) - if (*it == path) return; + nixDB.queryStrings(txn, dbId2Paths, id, paths); /* non-existence = ok */ paths.push_back(path); - nixDB.setStrings(noTxn, dbId2Paths, id, paths); + nixDB.setStrings(txn, dbId2Paths, id, paths); + + txn.commit(); } void unregisterPath(const string & _path) { string path(canonPath(_path)); + Transaction txn(nixDB); + + debug(format("unregistering path `%1%'") % path); string _id; - if (!nixDB.queryString(noTxn, dbPath2Id, path, _id)) return; + if (!nixDB.queryString(txn, dbPath2Id, path, _id)) { + txn.abort(); + return; + } FSId id(parseHash(_id)); - nixDB.delPair(noTxn, dbPath2Id, path); + nixDB.delPair(txn, dbPath2Id, path); - /* begin transaction */ - Strings paths, paths2; - nixDB.queryStrings(noTxn, dbId2Paths, id, paths); /* non-existence = ok */ + nixDB.queryStrings(txn, dbId2Paths, id, paths); /* non-existence = ok */ - bool changed = false; for (Strings::iterator it = paths.begin(); it != paths.end(); it++) - if (*it != path) paths2.push_back(*it); else changed = true; + if (*it != path) paths2.push_back(*it); - if (changed) - nixDB.setStrings(noTxn, dbId2Paths, id, paths2); - - /* end transaction */ + nixDB.setStrings(txn, dbId2Paths, id, paths2); + txn.commit(); } @@ -230,6 +247,8 @@ string expandId(const FSId & id, const string & target, void addToStore(string srcPath, string & dstPath, FSId & id, bool deterministicName) { + debug(format("adding `%1%' to the store") % srcPath); + srcPath = absPath(srcPath); id = hashPath(srcPath);