From 05f93acb38f17f531596629462a15b178c01a278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn?= Date: Fri, 20 Apr 2012 10:23:51 +0200 Subject: [PATCH 1/2] Bug #1237: Disable the cache name index for the VM pool --- include/PoolSQL.h | 34 +++++++++++++----------- include/VMTemplatePool.h | 2 +- src/cluster/ClusterPool.cc | 2 +- src/datastore/DatastorePool.cc | 2 +- src/group/GroupPool.cc | 2 +- src/host/HostPool.cc | 2 +- src/image/Image.cc | 2 +- src/image/ImagePool.cc | 2 +- src/pool/PoolSQL.cc | 47 ++++++++++++++++++++++++---------- src/pool/test/TestPoolSQL.h | 2 +- src/um/UserPool.cc | 2 +- src/vm/VirtualMachinePool.cc | 2 +- src/vnm/VirtualNetworkPool.cc | 2 +- 13 files changed, 63 insertions(+), 40 deletions(-) diff --git a/include/PoolSQL.h b/include/PoolSQL.h index 86641daaa7..c127d075e6 100644 --- a/include/PoolSQL.h +++ b/include/PoolSQL.h @@ -37,17 +37,16 @@ using namespace std; class PoolSQL: public Callbackable, public Hookable { public: - /** * Initializes the oid counter. This function sets lastOID to * the last used Object identifier by querying the corresponding database * table. This function SHOULD be called before any pool related function. * @param _db a pointer to the database - * @param table the name of the table supporting the pool (to set the oid + * @param _table the name of the table supporting the pool (to set the oid * counter). If null the OID counter is not updated. - * @param with_uid the Pool objects have an owner id (uid) + * @param cache_by_name True if the objects can be retrieved by name */ - PoolSQL(SqlDB * _db, const char * _table); + PoolSQL(SqlDB * _db, const char * _table, bool cache_by_name); virtual ~PoolSQL(); @@ -71,17 +70,6 @@ public: */ PoolObjectSQL * get(int oid, bool lock); - /** - * Gets an object from the pool (if needed the object is loaded from the - * database). - * @param name of the object - * @param uid id of owner - * @param lock locks the object if true - * - * @return a pointer to the object, 0 in case of failure - */ - PoolObjectSQL * get(const string& name, int uid, bool lock); - /** * Updates the cache name index. Must be called when the owner of an object * is changed @@ -168,6 +156,17 @@ public: protected: + /** + * Gets an object from the pool (if needed the object is loaded from the + * database). + * @param name of the object + * @param uid id of owner + * @param lock locks the object if true + * + * @return a pointer to the object, 0 in case of failure + */ + PoolObjectSQL * get(const string& name, int uid, bool lock); + /** * Pointer to the database. */ @@ -238,6 +237,11 @@ private: */ map pool; + /** + * Whether or not this pool uses the name_pool index + */ + bool uses_name_pool; + /** * This is a name index for the pool map. The key is the name of the object * , that may be combained with the owner id. diff --git a/include/VMTemplatePool.h b/include/VMTemplatePool.h index f1bf5443c0..71c7d7bf73 100644 --- a/include/VMTemplatePool.h +++ b/include/VMTemplatePool.h @@ -27,7 +27,7 @@ class VMTemplatePool : public PoolSQL { public: - VMTemplatePool(SqlDB * db) : PoolSQL(db, VMTemplate::table){}; + VMTemplatePool(SqlDB * db) : PoolSQL(db, VMTemplate::table, true){}; ~VMTemplatePool(){}; diff --git a/src/cluster/ClusterPool.cc b/src/cluster/ClusterPool.cc index 32d549b1dc..61d48d2aea 100644 --- a/src/cluster/ClusterPool.cc +++ b/src/cluster/ClusterPool.cc @@ -32,7 +32,7 @@ const int ClusterPool::NONE_CLUSTER_ID = -1; /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ -ClusterPool::ClusterPool(SqlDB * db):PoolSQL(db, Cluster::table) +ClusterPool::ClusterPool(SqlDB * db):PoolSQL(db, Cluster::table, true) { ostringstream oss; string error_str; diff --git a/src/datastore/DatastorePool.cc b/src/datastore/DatastorePool.cc index f98396ca38..b38318fc04 100644 --- a/src/datastore/DatastorePool.cc +++ b/src/datastore/DatastorePool.cc @@ -36,7 +36,7 @@ const int DatastorePool::DEFAULT_DS_ID = 1; /* -------------------------------------------------------------------------- */ DatastorePool::DatastorePool(SqlDB * db): - PoolSQL(db, Datastore::table) + PoolSQL(db, Datastore::table, true) { ostringstream oss; string error_str; diff --git a/src/group/GroupPool.cc b/src/group/GroupPool.cc index b12cda200f..4a03790056 100644 --- a/src/group/GroupPool.cc +++ b/src/group/GroupPool.cc @@ -37,7 +37,7 @@ const int GroupPool::USERS_ID = 1; /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ -GroupPool::GroupPool(SqlDB * db):PoolSQL(db, Group::table) +GroupPool::GroupPool(SqlDB * db):PoolSQL(db, Group::table, true) { ostringstream oss; string error_str; diff --git a/src/host/HostPool.cc b/src/host/HostPool.cc index 3ead792f4c..9bb138a12b 100644 --- a/src/host/HostPool.cc +++ b/src/host/HostPool.cc @@ -33,7 +33,7 @@ HostPool::HostPool(SqlDB* db, vector hook_mads, const string& hook_location, const string& remotes_location) - : PoolSQL(db,Host::table) + : PoolSQL(db, Host::table, true) { // ------------------ Initialize Hooks for the pool ---------------------- diff --git a/src/image/Image.cc b/src/image/Image.cc index f6f9d7d04e..adcb40d64c 100644 --- a/src/image/Image.cc +++ b/src/image/Image.cc @@ -513,7 +513,7 @@ int Image::disk_attribute( VectorAttribute * disk, break; case CDROM: //Always use CDROM type for these ones - disk_attr_type = "CDROM" + disk_attr_type = "CDROM"; disk->replace("READONLY","YES"); break; } diff --git a/src/image/ImagePool.cc b/src/image/ImagePool.cc index 9c8b25e452..3fde07920c 100644 --- a/src/image/ImagePool.cc +++ b/src/image/ImagePool.cc @@ -35,7 +35,7 @@ ImagePool::ImagePool(SqlDB * db, const string& __default_type, const string& __default_dev_prefix, vector& restricted_attrs): - PoolSQL(db,Image::table) + PoolSQL(db, Image::table, true) { ostringstream sql; diff --git a/src/pool/PoolSQL.cc b/src/pool/PoolSQL.cc index 12835f7cc6..f67c70332a 100644 --- a/src/pool/PoolSQL.cc +++ b/src/pool/PoolSQL.cc @@ -50,8 +50,8 @@ int PoolSQL::init_cb(void *nil, int num, char **values, char **names) /* -------------------------------------------------------------------------- */ -PoolSQL::PoolSQL(SqlDB * _db, const char * _table): - db(_db), lastOID(-1), table(_table) +PoolSQL::PoolSQL(SqlDB * _db, const char * _table, bool cache_by_name): + db(_db), lastOID(-1), table(_table), uses_name_pool(cache_by_name) { ostringstream oss; @@ -216,23 +216,28 @@ PoolObjectSQL * PoolSQL::get( return 0; } - string okey = key(objectsql->name,objectsql->uid); - name_index = name_pool.find(okey); + if ( uses_name_pool ) + { + string okey = key(objectsql->name,objectsql->uid); - if ( name_index != name_pool.end() ) - { - name_index->second->lock(); + name_index = name_pool.find(okey); - PoolObjectSQL * tmp_ptr = name_index->second; + if ( name_index != name_pool.end() ) + { + name_index->second->lock(); - name_pool.erase(okey); - pool.erase(tmp_ptr->oid); + PoolObjectSQL * tmp_ptr = name_index->second; - delete tmp_ptr; + name_pool.erase(okey); + pool.erase(tmp_ptr->oid); + + delete tmp_ptr; + } + + name_pool.insert(make_pair(okey, objectsql)); } pool.insert(make_pair(objectsql->oid,objectsql)); - name_pool.insert(make_pair(okey, objectsql)); if ( olock == true ) { @@ -257,6 +262,11 @@ PoolObjectSQL * PoolSQL::get( PoolObjectSQL * PoolSQL::get(const string& name, int ouid, bool olock) { + if ( uses_name_pool == false ) + { + return 0; + } + map::iterator index; PoolObjectSQL * objectsql; @@ -344,6 +354,11 @@ void PoolSQL::update_cache_index(string& old_name, string& new_name, int new_uid) { + if ( uses_name_pool == false ) + { + return; + } + map::iterator index; lock(); @@ -398,10 +413,14 @@ void PoolSQL::replace() else { PoolObjectSQL * tmp_ptr = index->second; - string okey = key(tmp_ptr->name,tmp_ptr->uid); pool.erase(index); - name_pool.erase(okey); + + if ( uses_name_pool ) + { + string okey = key(tmp_ptr->name,tmp_ptr->uid); + name_pool.erase(okey); + } delete tmp_ptr; diff --git a/src/pool/test/TestPoolSQL.h b/src/pool/test/TestPoolSQL.h index b847785a04..8b53e41e74 100644 --- a/src/pool/test/TestPoolSQL.h +++ b/src/pool/test/TestPoolSQL.h @@ -118,7 +118,7 @@ class TestPool : public PoolSQL { public: - TestPool(SqlDB *db):PoolSQL(db,"test_pool"){}; + TestPool(SqlDB *db):PoolSQL(db,"test_pool",true){}; ~TestPool(){}; TestObjectSQL * get( diff --git a/src/um/UserPool.cc b/src/um/UserPool.cc index 34bbbb17ae..5f1b89813e 100644 --- a/src/um/UserPool.cc +++ b/src/um/UserPool.cc @@ -54,7 +54,7 @@ string UserPool::oneadmin_name; UserPool::UserPool(SqlDB * db, time_t __session_expiration_time): - PoolSQL(db,User::table) + PoolSQL(db, User::table, true) { int one_uid = -1; int server_uid = -1; diff --git a/src/vm/VirtualMachinePool.cc b/src/vm/VirtualMachinePool.cc index 206d2b4cb4..f4e8fd6d02 100644 --- a/src/vm/VirtualMachinePool.cc +++ b/src/vm/VirtualMachinePool.cc @@ -29,7 +29,7 @@ VirtualMachinePool::VirtualMachinePool(SqlDB * db, const string& hook_location, const string& remotes_location, vector& restricted_attrs) - : PoolSQL(db,VirtualMachine::table) + : PoolSQL(db, VirtualMachine::table, false) { const VectorAttribute * vattr; diff --git a/src/vnm/VirtualNetworkPool.cc b/src/vnm/VirtualNetworkPool.cc index 348863b813..4a1ca02857 100644 --- a/src/vnm/VirtualNetworkPool.cc +++ b/src/vnm/VirtualNetworkPool.cc @@ -34,7 +34,7 @@ unsigned int VirtualNetworkPool::_default_size; VirtualNetworkPool::VirtualNetworkPool(SqlDB * db, const string& prefix, int __default_size): - PoolSQL(db,VirtualNetwork::table) + PoolSQL(db, VirtualNetwork::table, true) { istringstream iss; size_t pos = 0; From 1696bb669ea848fba70209f146783103c0534f7e Mon Sep 17 00:00:00 2001 From: "Ruben S. Montero" Date: Fri, 20 Apr 2012 11:04:49 +0200 Subject: [PATCH 2/2] bug #1237: Move access to uses_name_pool to critical section --- src/pool/PoolSQL.cc | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/pool/PoolSQL.cc b/src/pool/PoolSQL.cc index f67c70332a..06d47051d8 100644 --- a/src/pool/PoolSQL.cc +++ b/src/pool/PoolSQL.cc @@ -199,8 +199,6 @@ PoolObjectSQL * PoolSQL::get( } else { - map::iterator name_index; - objectsql = create(); objectsql->oid = oid; @@ -217,10 +215,12 @@ PoolObjectSQL * PoolSQL::get( } if ( uses_name_pool ) - { - string okey = key(objectsql->name,objectsql->uid); + { + map::iterator name_index; + string okey; - name_index = name_pool.find(okey); + okey = key(objectsql->name,objectsql->uid); + name_index = name_pool.find(okey); if ( name_index != name_pool.end() ) { @@ -262,11 +262,6 @@ PoolObjectSQL * PoolSQL::get( PoolObjectSQL * PoolSQL::get(const string& name, int ouid, bool olock) { - if ( uses_name_pool == false ) - { - return 0; - } - map::iterator index; PoolObjectSQL * objectsql; @@ -274,6 +269,12 @@ PoolObjectSQL * PoolSQL::get(const string& name, int ouid, bool olock) lock(); + if ( uses_name_pool == false ) + { + unlock(); + return 0; + } + index = name_pool.find(key(name,ouid)); if ( index != name_pool.end() && index->second->isValid() == true ) @@ -354,15 +355,16 @@ void PoolSQL::update_cache_index(string& old_name, string& new_name, int new_uid) { - if ( uses_name_pool == false ) - { - return; - } - map::iterator index; lock(); + if ( uses_name_pool == false ) + { + unlock(); + return; + } + string old_key = key(old_name, old_uid); string new_key = key(new_name, new_uid);