Mercurial > hg > orthanc-databases
changeset 135:e26690365c25
MySQL: Added an advisory lock to avoid race conditions during database setup
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Wed, 08 May 2019 21:09:18 +0200 |
parents | cc3dc759c989 |
children | 3266785d5627 |
files | Framework/MySQL/MySQLDatabase.cpp Framework/MySQL/MySQLDatabase.h MySQL/NEWS MySQL/Plugins/MySQLIndex.cpp MySQL/Plugins/MySQLStorageArea.cpp MySQL/UnitTests/UnitTestsMain.cpp |
diffstat | 6 files changed, 147 insertions(+), 22 deletions(-) [+] |
line wrap: on
line diff
--- a/Framework/MySQL/MySQLDatabase.cpp Wed May 08 20:21:13 2019 +0200 +++ b/Framework/MySQL/MySQLDatabase.cpp Wed May 08 21:09:18 2019 +0200 @@ -35,6 +35,7 @@ #include <mysqld_error.h> #include <memory> +#include <boost/thread.hpp> namespace OrthancDatabases { @@ -289,32 +290,45 @@ } + bool MySQLDatabase::RunAdvisoryLockStatement(const std::string& s) + { + Query query(s, false); + MySQLStatement statement(*this, query); + + MySQLTransaction t(*this); + Dictionary args; + + std::auto_ptr<IResult> result(t.Execute(statement, args)); + + bool 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) + { + return RunAdvisoryLockStatement("SELECT GET_LOCK('Lock" + + boost::lexical_cast<std::string>(lock) + "', 0);"); + } + + + bool MySQLDatabase::ReleaseAdvisoryLock(int32_t lock) + { + return RunAdvisoryLockStatement("SELECT RELEASE_LOCK('Lock" + + boost::lexical_cast<std::string>(lock) + "');"); + } + + void MySQLDatabase::AdvisoryLock(int32_t lock) { - try - { - Query query("SELECT GET_LOCK('Lock" + - boost::lexical_cast<std::string>(lock) + "', 0);", false); - MySQLStatement statement(*this, query); - - MySQLTransaction t(*this); - Dictionary args; - - std::auto_ptr<IResult> result(t.Execute(statement, args)); - - if (result->IsDone() || - result->GetField(0).GetType() != ValueType_Integer64 || - dynamic_cast<const Integer64Value&>(result->GetField(0)).GetValue() != 1) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); - } - - t.Commit(); - } - catch (Orthanc::OrthancException&) + if (!AcquireAdvisoryLock(lock)) { LOG(ERROR) << "The MySQL database is locked by another instance of Orthanc"; - Close(); throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); } } @@ -495,4 +509,39 @@ return true; } + + + MySQLDatabase::TransientAdvisoryLock:: + TransientAdvisoryLock(MySQLDatabase& database, + int32_t lock) : + database_(database), + lock_(lock) + { + bool locked = true; + + for (unsigned int i = 0; i < 10; i++) + { + if (database_.AcquireAdvisoryLock(lock_)) + { + locked = false; + break; + } + else + { + boost::this_thread::sleep(boost::posix_time::milliseconds(500)); + } + } + + if (locked) + { + LOG(ERROR) << "Cannot acquire a transient advisory lock"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_Plugin); + } + } + + + MySQLDatabase::TransientAdvisoryLock::~TransientAdvisoryLock() + { + database_.ReleaseAdvisoryLock(lock_); + } }
--- a/Framework/MySQL/MySQLDatabase.h Wed May 08 20:21:13 2019 +0200 +++ b/Framework/MySQL/MySQLDatabase.h Wed May 08 21:09:18 2019 +0200 @@ -44,6 +44,8 @@ void Close(); + bool RunAdvisoryLockStatement(const std::string& statement); + public: MySQLDatabase(const MySQLParameters& parameters); @@ -70,6 +72,10 @@ bool LookupGlobalIntegerVariable(int64_t& value, const std::string& variable); + bool AcquireAdvisoryLock(int32_t lock); + + bool ReleaseAdvisoryLock(int32_t lock); + void AdvisoryLock(int32_t lock); void Execute(const std::string& sql, @@ -93,5 +99,18 @@ static void GlobalFinalization(); static bool IsValidDatabaseIdentifier(const std::string& s); + + class TransientAdvisoryLock + { + private: + MySQLDatabase& database_; + int32_t lock_; + + public: + TransientAdvisoryLock(MySQLDatabase& database, + int32_t lock); + + ~TransientAdvisoryLock(); + }; }; }
--- a/MySQL/NEWS Wed May 08 20:21:13 2019 +0200 +++ b/MySQL/NEWS Wed May 08 21:09:18 2019 +0200 @@ -2,6 +2,7 @@ =============================== * Implementation of new extensions: LookupResourceAndParent and GetAllMetadata +* Added an advisory lock to avoid race conditions during database setup Release 2.0 (2019-01-23)
--- a/MySQL/Plugins/MySQLIndex.cpp Wed May 08 20:21:13 2019 +0200 +++ b/MySQL/Plugins/MySQLIndex.cpp Wed May 08 21:09:18 2019 +0200 @@ -76,6 +76,14 @@ { 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 */); { MySQLTransaction t(*db);
--- a/MySQL/Plugins/MySQLStorageArea.cpp Wed May 08 20:21:13 2019 +0200 +++ b/MySQL/Plugins/MySQLStorageArea.cpp Wed May 08 21:09:18 2019 +0200 @@ -42,6 +42,14 @@ 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 */); + { MySQLTransaction t(*db);
--- a/MySQL/UnitTests/UnitTestsMain.cpp Wed May 08 20:21:13 2019 +0200 +++ b/MySQL/UnitTests/UnitTestsMain.cpp Wed May 08 21:09:18 2019 +0200 @@ -63,6 +63,46 @@ } +TEST(MySQL, Lock2) +{ + OrthancDatabases::MySQLDatabase::ClearDatabase(globalParameters_); + + OrthancDatabases::MySQLDatabase db1(globalParameters_); + db1.Open(); + + ASSERT_FALSE(db1.ReleaseAdvisoryLock(43)); // lock counter = 0 + ASSERT_TRUE(db1.AcquireAdvisoryLock(43)); // 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 + + // 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 + + { + OrthancDatabases::MySQLDatabase db2(globalParameters_); + db2.Open(); + + // The "db1" is still actively locking + ASSERT_FALSE(db2.AcquireAdvisoryLock(43)); + + // Release the "db1" lock + ASSERT_TRUE(db1.ReleaseAdvisoryLock(43)); + ASSERT_FALSE(db1.ReleaseAdvisoryLock(43)); + + // "db2" can now acquire the lock, but not "db1" + ASSERT_TRUE(db2.AcquireAdvisoryLock(43)); + ASSERT_FALSE(db1.AcquireAdvisoryLock(43)); + } + + // "db2" is closed, "db1" can now acquire the lock + ASSERT_TRUE(db1.AcquireAdvisoryLock(43)); +} + + static int64_t CountFiles(OrthancDatabases::MySQLDatabase& db) { OrthancDatabases::Query query("SELECT COUNT(*) FROM StorageArea", true);