Mercurial > hg > orthanc-databases
changeset 137:52b3859ee0b7
MySQL: acquiring named locks instead of numbers
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Thu, 09 May 2019 11:29:17 +0200 |
parents | 3266785d5627 |
children | 5c5cd59c991f |
files | Framework/MySQL/MySQLDatabase.cpp Framework/MySQL/MySQLDatabase.h MySQL/Plugins/MySQLDefinitions.h MySQL/Plugins/MySQLIndex.cpp MySQL/Plugins/MySQLStorageArea.cpp MySQL/UnitTests/UnitTestsMain.cpp |
diffstat | 6 files changed, 163 insertions(+), 62 deletions(-) [+] |
line wrap: on
line diff
--- a/Framework/MySQL/MySQLDatabase.cpp Thu May 09 09:52:11 2019 +0200 +++ b/Framework/MySQL/MySQLDatabase.cpp Thu May 09 11:29:17 2019 +0200 @@ -290,41 +290,65 @@ } - bool MySQLDatabase::RunAdvisoryLockStatement(const std::string& s) + bool MySQLDatabase::RunAdvisoryLockStatement(Query& query, + const std::string& lock) { - Query query(s, false); + const std::string& dbName = parameters_.GetDatabase(); + + // Prepend the name of the lock by the database name. This allows + // to create a namespace for advisory locks: + // https://groups.google.com/d/msg/orthanc-users/yV3LSTh_TjI/MQIcvnMlAQAJ + std::string prefix; + prefix.reserve(dbName.size()); + for (size_t i = 0; i < dbName.size(); i++) + { + if (isalnum(dbName[i]) || + dbName[i] == '$' || + dbName[i] == '_') + { + prefix.push_back(dbName[i]); + } + } + + query.SetType("lock", ValueType_Utf8String); + MySQLStatement statement(*this, query); - MySQLTransaction t(*this); Dictionary args; + args.SetUtf8Value("lock", prefix + "." + lock); - std::auto_ptr<IResult> result(t.Execute(statement, args)); + bool success; - bool success = (!result->IsDone() && - result->GetField(0).GetType() == ValueType_Integer64 && - dynamic_cast<const Integer64Value&>(result->GetField(0)).GetValue() == 1); + { + MySQLTransaction t(*this); + std::auto_ptr<IResult> result(t.Execute(statement, args)); - t.Commit(); + success = (!result->IsDone() && + result->GetField(0).GetType() == ValueType_Integer64 && + dynamic_cast<const Integer64Value&>(result->GetField(0)).GetValue() == 1); + + t.Commit(); + } return success; } - bool MySQLDatabase::AcquireAdvisoryLock(int32_t lock) + bool MySQLDatabase::AcquireAdvisoryLock(const std::string& lock) { - return RunAdvisoryLockStatement("SELECT GET_LOCK('Lock" + - boost::lexical_cast<std::string>(lock) + "', 0);"); + Query query("SELECT GET_LOCK(${lock}, 0)", false); + return RunAdvisoryLockStatement(query, lock); } - bool MySQLDatabase::ReleaseAdvisoryLock(int32_t lock) + bool MySQLDatabase::ReleaseAdvisoryLock(const std::string& lock) { - return RunAdvisoryLockStatement("SELECT RELEASE_LOCK('Lock" + - boost::lexical_cast<std::string>(lock) + "');"); + Query query("SELECT RELEASE_LOCK(${lock})", false); + return RunAdvisoryLockStatement(query, lock); } - void MySQLDatabase::AdvisoryLock(int32_t lock) + void MySQLDatabase::AdvisoryLock(const std::string& lock) { if (!AcquireAdvisoryLock(lock)) { @@ -513,7 +537,7 @@ MySQLDatabase::TransientAdvisoryLock:: TransientAdvisoryLock(MySQLDatabase& database, - int32_t lock) : + const std::string& lock) : database_(database), lock_(lock) {
--- a/Framework/MySQL/MySQLDatabase.h Thu May 09 09:52:11 2019 +0200 +++ b/Framework/MySQL/MySQLDatabase.h Thu May 09 11:29:17 2019 +0200 @@ -44,7 +44,8 @@ void Close(); - bool RunAdvisoryLockStatement(const std::string& statement); + bool RunAdvisoryLockStatement(Query& query, + const std::string& lock); public: MySQLDatabase(const MySQLParameters& parameters); @@ -72,11 +73,11 @@ bool LookupGlobalIntegerVariable(int64_t& value, const std::string& variable); - bool AcquireAdvisoryLock(int32_t lock); + bool AcquireAdvisoryLock(const std::string& lock); - bool ReleaseAdvisoryLock(int32_t lock); + bool ReleaseAdvisoryLock(const std::string& lock); - void AdvisoryLock(int32_t lock); + void AdvisoryLock(const std::string& lock); void Execute(const std::string& sql, bool arobaseSeparator); @@ -104,11 +105,11 @@ { private: MySQLDatabase& database_; - int32_t lock_; + std::string lock_; public: TransientAdvisoryLock(MySQLDatabase& database, - int32_t lock); + const std::string& lock); ~TransientAdvisoryLock(); };
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/MySQL/Plugins/MySQLDefinitions.h Thu May 09 11:29:17 2019 +0200 @@ -0,0 +1,49 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2019 Osimis S.A., Belgium + * + * This program is free software: you can redistribute it and/or + * modify it under the terms of the GNU Affero General Public License + * as published by the Free Software Foundation, either version 3 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + **/ + + +#pragma once + +#include <stdint.h> + + +/** + * This advisory lock is used if the "Lock" option is set to "true", + * in order to prevent the execution of two PostgreSQL index plugins + * on the same database. + **/ +static const char* const MYSQL_LOCK_INDEX = "orthanc_index_lock"; + + +/** + * This advisory lock is used if the "Lock" option is set to "true", + * in order to prevent the execution of two PostgreSQL storage area + * plugins on the same database. + **/ +static const char* const MYSQL_LOCK_STORAGE = "orthanc_storage_lock"; + + +/** + * Transient advisory lock to protect the setup of the database, + * because concurrent statements like "CREATE TABLE" are not protected + * by transactions. + * https://groups.google.com/d/msg/orthanc-users/yV3LSTh_TjI/h3PRApJFBAAJ + **/ +static const char* const MYSQL_LOCK_DATABASE_SETUP = "orthanc_setup_lock";
--- a/MySQL/Plugins/MySQLIndex.cpp Thu May 09 09:52:11 2019 +0200 +++ b/MySQL/Plugins/MySQLIndex.cpp Thu May 09 11:29:17 2019 +0200 @@ -24,6 +24,7 @@ #include "../../Framework/Plugins/GlobalProperties.h" #include "../../Framework/MySQL/MySQLDatabase.h" #include "../../Framework/MySQL/MySQLTransaction.h" +#include "MySQLDefinitions.h" #include <EmbeddedResources.h> // Auto-generated file @@ -69,23 +70,10 @@ std::auto_ptr<MySQLDatabase> db(new MySQLDatabase(parameters_)); db->Open(); - db->Execute("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE", false); - if (parameters_.HasLock()) { - db->AdvisoryLock(42 /* some arbitrary constant */); - } - - /** - * Try and acquire a transient advisory lock to protect the setup - * of the database, because concurrent statements like "CREATE - * TABLE" are not protected by transactions. - * https://groups.google.com/d/msg/orthanc-users/yV3LSTh_TjI/h3PRApJFBAAJ - **/ - MySQLDatabase::TransientAdvisoryLock lock(*db, 44 /* some arbitrary constant */); - - { + MySQLDatabase::TransientAdvisoryLock lock(*db, MYSQL_LOCK_DATABASE_SETUP); MySQLTransaction t(*db); db->Execute("ALTER DATABASE " + parameters_.GetDatabase() + @@ -183,6 +171,19 @@ t.Commit(); } + + /** + * WARNING: This lock must be acquired after + * "MYSQL_LOCK_DATABASE_SETUP" is released. Indeed, in MySQL < + * 5.7, it is impossible to acquire more than one lock at a time, + * as calling "SELECT GET_LOCK()" releases all the + * previously-acquired locks. + * https://dev.mysql.com/doc/refman/5.7/en/locking-functions.html + **/ + if (parameters_.HasLock()) + { + db->AdvisoryLock(MYSQL_LOCK_INDEX); + } return db.release(); }
--- a/MySQL/Plugins/MySQLStorageArea.cpp Thu May 09 09:52:11 2019 +0200 +++ b/MySQL/Plugins/MySQLStorageArea.cpp Thu May 09 11:29:17 2019 +0200 @@ -23,6 +23,7 @@ #include "../../Framework/MySQL/MySQLDatabase.h" #include "../../Framework/MySQL/MySQLTransaction.h" +#include "MySQLDefinitions.h" #include <Core/Logging.h> @@ -37,20 +38,8 @@ db->Open(); - if (parameters_.HasLock()) { - db->AdvisoryLock(43 /* some arbitrary constant */); - } - - /** - * Try and acquire a transient advisory lock to protect the setup - * of the database, because concurrent statements like "CREATE - * TABLE" are not protected by transactions. - * https://groups.google.com/d/msg/orthanc-users/yV3LSTh_TjI/h3PRApJFBAAJ - **/ - MySQLDatabase::TransientAdvisoryLock lock(*db, 44 /* some arbitrary constant */); - - { + MySQLDatabase::TransientAdvisoryLock lock(*db, MYSQL_LOCK_DATABASE_SETUP); MySQLTransaction t(*db); int64_t size; @@ -82,6 +71,19 @@ t.Commit(); } + /** + * WARNING: This lock must be acquired after + * "MYSQL_LOCK_DATABASE_SETUP" is released. Indeed, in MySQL < + * 5.7, it is impossible to acquire more than one lock at a time, + * as calling "SELECT GET_LOCK()" releases all the + * previously-acquired locks. + * https://dev.mysql.com/doc/refman/5.7/en/locking-functions.html + **/ + if (parameters_.HasLock()) + { + db->AdvisoryLock(MYSQL_LOCK_STORAGE); + } + return db.release(); }
--- a/MySQL/UnitTests/UnitTestsMain.cpp Thu May 09 09:52:11 2019 +0200 +++ b/MySQL/UnitTests/UnitTestsMain.cpp Thu May 09 11:29:17 2019 +0200 @@ -70,36 +70,60 @@ OrthancDatabases::MySQLDatabase db1(globalParameters_); db1.Open(); - ASSERT_FALSE(db1.ReleaseAdvisoryLock(43)); // lock counter = 0 - ASSERT_TRUE(db1.AcquireAdvisoryLock(43)); // lock counter = 1 + ASSERT_FALSE(db1.ReleaseAdvisoryLock("mylock")); // lock counter = 0 + ASSERT_TRUE(db1.AcquireAdvisoryLock("mylock")); // lock counter = 1 // OK, as this is the same connection - ASSERT_TRUE(db1.AcquireAdvisoryLock(43)); // lock counter = 2 - ASSERT_TRUE(db1.ReleaseAdvisoryLock(43)); // lock counter = 1 + ASSERT_TRUE(db1.AcquireAdvisoryLock("mylock")); // lock counter = 1 + ASSERT_TRUE(db1.ReleaseAdvisoryLock("mylock")); // lock counter = 0 // Try and release twice the lock - ASSERT_TRUE(db1.ReleaseAdvisoryLock(43)); // lock counter = 0 - ASSERT_FALSE(db1.ReleaseAdvisoryLock(43)); // cannot unlock - ASSERT_TRUE(db1.AcquireAdvisoryLock(43)); // lock counter = 1 + ASSERT_FALSE(db1.ReleaseAdvisoryLock("mylock")); // lock counter = 0 + ASSERT_TRUE(db1.AcquireAdvisoryLock("mylock")); // lock counter = 1 { OrthancDatabases::MySQLDatabase db2(globalParameters_); db2.Open(); // The "db1" is still actively locking - ASSERT_FALSE(db2.AcquireAdvisoryLock(43)); + ASSERT_FALSE(db2.AcquireAdvisoryLock("mylock")); // Release the "db1" lock - ASSERT_TRUE(db1.ReleaseAdvisoryLock(43)); - ASSERT_FALSE(db1.ReleaseAdvisoryLock(43)); + ASSERT_TRUE(db1.ReleaseAdvisoryLock("mylock")); + ASSERT_FALSE(db1.ReleaseAdvisoryLock("mylock")); // "db2" can now acquire the lock, but not "db1" - ASSERT_TRUE(db2.AcquireAdvisoryLock(43)); - ASSERT_FALSE(db1.AcquireAdvisoryLock(43)); + ASSERT_TRUE(db2.AcquireAdvisoryLock("mylock")); + ASSERT_FALSE(db1.AcquireAdvisoryLock("mylock")); } // "db2" is closed, "db1" can now acquire the lock - ASSERT_TRUE(db1.AcquireAdvisoryLock(43)); + ASSERT_TRUE(db1.AcquireAdvisoryLock("mylock")); +} + + + +/** + * WARNING: The following test only succeeds if MySQL >= 5.7. This is + * because in MySQL < 5.7, acquiring a lock by calling "SELECT + * GET_LOCK()" releases all the previously acquired locks! + **/ +TEST(MySQL, DISABLED_Lock3) +{ + OrthancDatabases::MySQLDatabase::ClearDatabase(globalParameters_); + + OrthancDatabases::MySQLDatabase db1(globalParameters_); + db1.Open(); + + ASSERT_TRUE(db1.AcquireAdvisoryLock("mylock1")); // lock counter = 1 + ASSERT_TRUE(db1.AcquireAdvisoryLock("mylock2")); // lock counter = 1 + + { + OrthancDatabases::MySQLDatabase db2(globalParameters_); + db2.Open(); + + ASSERT_FALSE(db2.AcquireAdvisoryLock("mylock1")); + } }