Mercurial > hg > orthanc-databases
changeset 704:bcea50e40d6e sql-opti
Dynamic connection pool + remove ON DELETE CASCADE on Resources.parentID since this is now implemented in the DeletedResource function + fix unit tests for PG
line wrap: on
line diff
--- a/Framework/Common/DatabaseManager.cpp Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Common/DatabaseManager.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -160,7 +160,9 @@ DatabaseManager::DatabaseManager(IDatabaseFactory* factory) : factory_(factory), - dialect_(Dialect_Unknown) + dialect_(Dialect_Unknown), + creationTime_(boost::posix_time::second_clock::universal_time()), + lastUseTime_(boost::posix_time::second_clock::universal_time()) { if (factory == NULL) { @@ -168,6 +170,20 @@ } } + uint64_t DatabaseManager::GetElapsedSecondsSinceCreation() const + { + boost::posix_time::ptime now = boost::posix_time::second_clock::universal_time(); + boost::posix_time::time_duration diff = now - creationTime_; + return static_cast<uint64_t>(diff.total_seconds()); + } + + uint64_t DatabaseManager::GetElapsedSecondsSinceLastUse() const + { + boost::posix_time::ptime now = boost::posix_time::second_clock::universal_time(); + boost::posix_time::time_duration diff = now - lastUseTime_; + return static_cast<uint64_t>(diff.total_seconds()); + } + IDatabase& DatabaseManager::GetDatabase() { @@ -218,6 +234,8 @@ } transaction_.reset(GetDatabase().CreateTransaction(type)); + + lastUseTime_= boost::posix_time::second_clock::universal_time(); } catch (Orthanc::OrthancException& e) {
--- a/Framework/Common/DatabaseManager.h Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Common/DatabaseManager.h Thu Jun 26 22:31:58 2025 +0200 @@ -30,6 +30,7 @@ #include <Enumerations.h> #include <memory> +#include <boost/date_time/posix_time/posix_time.hpp> namespace OrthancDatabases @@ -56,6 +57,8 @@ std::unique_ptr<ITransaction> transaction_; CachedStatements cachedStatements_; Dialect dialect_; + boost::posix_time::ptime creationTime_; + boost::posix_time::ptime lastUseTime_; void CloseIfUnavailable(Orthanc::ErrorCode e); @@ -88,6 +91,9 @@ void RollbackTransaction(); + uint64_t GetElapsedSecondsSinceCreation() const; + + uint64_t GetElapsedSecondsSinceLastUse() const; // This class is only used in the "StorageBackend" and in // "IDatabaseBackend::ConfigureDatabase()"
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Framework/Plugins/BaseIndexConnectionsPool.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -0,0 +1,177 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2023 Osimis S.A., Belgium + * Copyright (C) 2024-2025 Orthanc Team SRL, Belgium + * Copyright (C) 2021-2025 Sebastien Jodogne, ICTEAM UCLouvain, 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/>. + **/ + + +#include "BaseIndexConnectionsPool.h" + +#include <Logging.h> + + +namespace OrthancDatabases +{ +// class BaseIndexConnectionsPool::ManagerReference : public Orthanc::IDynamicObject +// { +// private: +// DatabaseManager* manager_; + +// public: +// explicit ManagerReference(DatabaseManager& manager) : +// manager_(&manager) +// { +// } + +// DatabaseManager& GetManager() +// { +// assert(manager_ != NULL); +// return *manager_; +// } +// }; + + + void BaseIndexConnectionsPool::HousekeepingThread(BaseIndexConnectionsPool* that) + { +#if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 12, 2) + OrthancPluginSetCurrentThreadName(OrthancPlugins::GetGlobalContext(), "DB HOUSEKEEPING"); +#endif + + boost::posix_time::ptime lastInvocation = boost::posix_time::second_clock::local_time(); + + while (that->housekeepingContinue_) + { + if (boost::posix_time::second_clock::local_time() - lastInvocation >= that->housekeepingDelay_) + { + try + { + { + Accessor accessor(*that); + accessor.GetBackend().PerformDbHousekeeping(accessor.GetManager()); + } + + that->PerformPoolHousekeeping(); + } + catch (Orthanc::OrthancException& e) + { + LOG(ERROR) << "Exception during the database housekeeping: " << e.What(); + } + catch (...) + { + LOG(ERROR) << "Native exception during the database houskeeping"; + } + + lastInvocation = boost::posix_time::second_clock::local_time(); + } + + boost::this_thread::sleep(boost::posix_time::milliseconds(1000)); + } + } + + + BaseIndexConnectionsPool::BaseIndexConnectionsPool(IndexBackend* backend, + unsigned int houseKeepingDelaySeconds) : + backend_(backend), + housekeepingContinue_(true), + housekeepingDelay_(boost::posix_time::seconds(houseKeepingDelaySeconds)) + { + if (backend == NULL) + { + throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer); + } + else if (backend->HasPerformDbHousekeeping() && + houseKeepingDelaySeconds == 0) + { + throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange, + "The delay between two executions of housekeeping cannot be zero second"); + } + else + { + context_ = backend_->GetContext(); + } + } + + + BaseIndexConnectionsPool::~BaseIndexConnectionsPool() + { + } + + + void BaseIndexConnectionsPool::StartHousekeepingThread() + { + housekeepingContinue_ = true; + + if (backend_->HasPerformDbHousekeeping()) + { + housekeepingThread_ = boost::thread(HousekeepingThread, this); + } + } + + void BaseIndexConnectionsPool::StopHousekeepingThread() + { + housekeepingContinue_ = false; + if (housekeepingThread_.joinable()) + { + housekeepingThread_.join(); + } + } + + + BaseIndexConnectionsPool::Accessor::Accessor(BaseIndexConnectionsPool& pool) : + // lock_(pool.connectionsMutex_), + pool_(pool), + manager_(NULL) + { + for (;;) + { + std::unique_ptr<DatabaseManager> manager(pool.GetConnection()); + if (manager.get() != NULL) + { + manager_ = manager.release(); + return; + } + boost::this_thread::sleep(boost::posix_time::millisec(100)); + } + } + + + BaseIndexConnectionsPool::Accessor::~Accessor() + { + assert(manager_ != NULL); + pool_.ReleaseConnection(manager_); + // boost::unique_lock<boost::shared_mutex> lock(pool_.connectionsMutex_); + // pool_.availableConnections_.push_front(manager_); + // pool_.availableConnectionsSemaphore_.Release(1); + + + } + + + IndexBackend& BaseIndexConnectionsPool::Accessor::GetBackend() const + { + return *pool_.backend_; + } + + + DatabaseManager& BaseIndexConnectionsPool::Accessor::GetManager() const + { + assert(manager_ != NULL); + return *manager_; + } +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Framework/Plugins/BaseIndexConnectionsPool.h Thu Jun 26 22:31:58 2025 +0200 @@ -0,0 +1,94 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2023 Osimis S.A., Belgium + * Copyright (C) 2024-2025 Orthanc Team SRL, Belgium + * Copyright (C) 2021-2025 Sebastien Jodogne, ICTEAM UCLouvain, 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 "IdentifierTag.h" +#include "IndexBackend.h" + +#include <MultiThreading/SharedMessageQueue.h> +#include <MultiThreading/Semaphore.h> + +#include <list> +#include <boost/thread.hpp> + +namespace OrthancDatabases +{ + class BaseIndexConnectionsPool : public boost::noncopyable + { + protected: + + std::unique_ptr<IndexBackend> backend_; + OrthancPluginContext* context_; + boost::mutex connectionsMutex_; + + bool housekeepingContinue_; + boost::thread housekeepingThread_; + boost::posix_time::time_duration housekeepingDelay_; + + static void HousekeepingThread(BaseIndexConnectionsPool* that); + + virtual void PerformPoolHousekeeping() + {} + + void StartHousekeepingThread(); + + void StopHousekeepingThread(); + + virtual DatabaseManager* GetConnection() = 0; + + virtual void ReleaseConnection(DatabaseManager* manager) = 0; + + public: + BaseIndexConnectionsPool(IndexBackend* backend /* takes ownership */, + unsigned int houseKeepingDelaySeconds); + + virtual ~BaseIndexConnectionsPool(); + + OrthancPluginContext* GetContext() const + { + return context_; + } + + virtual void OpenConnections(bool hasIdentifierTags, + const std::list<IdentifierTag>& identifierTags) = 0; + + virtual void CloseConnections() = 0; + + class Accessor : public boost::noncopyable + { + private: + BaseIndexConnectionsPool& pool_; + DatabaseManager* manager_; + + public: + explicit Accessor(BaseIndexConnectionsPool& pool); + + ~Accessor(); + + IndexBackend& GetBackend() const; + + DatabaseManager& GetManager() const; + }; + }; +}
--- a/Framework/Plugins/DatabaseBackendAdapterV4.cpp Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Plugins/DatabaseBackendAdapterV4.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -26,6 +26,7 @@ #if defined(ORTHANC_PLUGINS_VERSION_IS_ABOVE) // Macro introduced in Orthanc 1.3.1 # if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 12, 0) +#include "DynamicIndexConnectionsPool.h" #include "IndexConnectionsPool.h" #include "MessagesToolbox.h" @@ -422,13 +423,13 @@ static void ProcessDatabaseOperation(Orthanc::DatabasePluginMessages::DatabaseResponse& response, const Orthanc::DatabasePluginMessages::DatabaseRequest& request, - IndexConnectionsPool& pool) + BaseIndexConnectionsPool& pool) { switch (request.operation()) { case Orthanc::DatabasePluginMessages::OPERATION_GET_SYSTEM_INFORMATION: { - IndexConnectionsPool::Accessor accessor(pool); + BaseIndexConnectionsPool::Accessor accessor(pool); response.mutable_get_system_information()->set_database_version(accessor.GetBackend().GetDatabaseVersion(accessor.GetManager())); response.mutable_get_system_information()->set_supports_flush_to_disk(false); response.mutable_get_system_information()->set_supports_revisions(accessor.GetBackend().HasRevisionsSupport()); @@ -485,7 +486,7 @@ case Orthanc::DatabasePluginMessages::OPERATION_START_TRANSACTION: { - std::unique_ptr<IndexConnectionsPool::Accessor> transaction(new IndexConnectionsPool::Accessor(pool)); + std::unique_ptr<BaseIndexConnectionsPool::Accessor> transaction(new BaseIndexConnectionsPool::Accessor(pool)); switch (request.start_transaction().type()) { @@ -507,7 +508,7 @@ case Orthanc::DatabasePluginMessages::OPERATION_UPGRADE: { - IndexConnectionsPool::Accessor accessor(pool); + BaseIndexConnectionsPool::Accessor accessor(pool); OrthancPluginStorageArea* storageArea = reinterpret_cast<OrthancPluginStorageArea*>(request.upgrade().storage_area()); accessor.GetBackend().UpgradeDatabase(accessor.GetManager(), request.upgrade().target_version(), storageArea); break; @@ -515,7 +516,7 @@ case Orthanc::DatabasePluginMessages::OPERATION_FINALIZE_TRANSACTION: { - IndexConnectionsPool::Accessor* transaction = reinterpret_cast<IndexConnectionsPool::Accessor*>(request.finalize_transaction().transaction()); + BaseIndexConnectionsPool::Accessor* transaction = reinterpret_cast<BaseIndexConnectionsPool::Accessor*>(request.finalize_transaction().transaction()); if (transaction == NULL) { @@ -532,7 +533,7 @@ #if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 12, 3) case Orthanc::DatabasePluginMessages::OPERATION_MEASURE_LATENCY: { - IndexConnectionsPool::Accessor accessor(pool); + BaseIndexConnectionsPool::Accessor accessor(pool); response.mutable_measure_latency()->set_latency_us(accessor.GetBackend().MeasureLatency(accessor.GetManager())); break; } @@ -1360,7 +1361,7 @@ return OrthancPluginErrorCode_InternalError; } - IndexConnectionsPool& pool = *reinterpret_cast<IndexConnectionsPool*>(rawPool); + BaseIndexConnectionsPool& pool = *reinterpret_cast<BaseIndexConnectionsPool*>(rawPool); try { @@ -1374,7 +1375,7 @@ case Orthanc::DatabasePluginMessages::REQUEST_TRANSACTION: { - IndexConnectionsPool::Accessor& transaction = *reinterpret_cast<IndexConnectionsPool::Accessor*>(request.transaction_request().transaction()); + BaseIndexConnectionsPool::Accessor& transaction = *reinterpret_cast<BaseIndexConnectionsPool::Accessor*>(request.transaction_request().transaction()); ProcessTransactionOperation(*response.mutable_transaction_response(), request.transaction_request(), transaction.GetBackend(), transaction.GetManager()); break; @@ -1432,7 +1433,7 @@ { if (rawPool != NULL) { - IndexConnectionsPool* pool = reinterpret_cast<IndexConnectionsPool*>(rawPool); + BaseIndexConnectionsPool* pool = reinterpret_cast<BaseIndexConnectionsPool*>(rawPool); if (isBackendInUse_) { @@ -1454,10 +1455,20 @@ void DatabaseBackendAdapterV4::Register(IndexBackend* backend, size_t countConnections, + bool useDynamicConnectionPool, unsigned int maxDatabaseRetries, unsigned int housekeepingDelaySeconds) { - std::unique_ptr<IndexConnectionsPool> pool(new IndexConnectionsPool(backend, countConnections, housekeepingDelaySeconds)); + std::unique_ptr<BaseIndexConnectionsPool> pool; + + if (useDynamicConnectionPool) + { + pool.reset(new DynamicIndexConnectionsPool(backend, countConnections, housekeepingDelaySeconds)); + } + else + { + pool.reset(new IndexConnectionsPool(backend, countConnections, housekeepingDelaySeconds)); + } if (isBackendInUse_) {
--- a/Framework/Plugins/DatabaseBackendAdapterV4.h Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Plugins/DatabaseBackendAdapterV4.h Thu Jun 26 22:31:58 2025 +0200 @@ -50,6 +50,7 @@ public: static void Register(IndexBackend* backend, size_t countConnections, + bool useDynamicConnectionPool, unsigned int maxDatabaseRetries, unsigned int housekeepingDelaySeconds);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Framework/Plugins/DynamicIndexConnectionsPool.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -0,0 +1,198 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2023 Osimis S.A., Belgium + * Copyright (C) 2024-2025 Orthanc Team SRL, Belgium + * Copyright (C) 2021-2025 Sebastien Jodogne, ICTEAM UCLouvain, 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/>. + **/ + + +#include "DynamicIndexConnectionsPool.h" + +#include <Logging.h> + + +namespace OrthancDatabases +{ + void DynamicIndexConnectionsPool::PerformPoolHousekeeping() + { + CleanupOldConnections(); + + OrthancPluginSetMetricsValue(OrthancPlugins::GetGlobalContext(), "orthanc_index_active_connections_count", maxConnectionsCount_ - connectionsSemaphore_.GetAvailableResourcesCount(), OrthancPluginMetricsType_Default); + } + + + DynamicIndexConnectionsPool::DynamicIndexConnectionsPool(IndexBackend* backend, + size_t maxConnectionsCount, + unsigned int houseKeepingDelaySeconds) : + BaseIndexConnectionsPool(backend, houseKeepingDelaySeconds), + maxConnectionsCount_(maxConnectionsCount), + connectionsSemaphore_(maxConnectionsCount), + availableConnectionsSemaphore_(0) + { + if (maxConnectionsCount == 0) + { + throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange, + "There must be a non-zero number of connections to the database"); + } + } + + + DynamicIndexConnectionsPool::~DynamicIndexConnectionsPool() + { + boost::mutex::scoped_lock lock(connectionsMutex_); + + for (std::list<DatabaseManager*>::iterator + it = connections_.begin(); it != connections_.end(); ++it) + { + assert(*it != NULL); + delete *it; + } + } + + + void DynamicIndexConnectionsPool::OpenConnections(bool hasIdentifierTags, + const std::list<IdentifierTag>& identifierTags) + { + assert(backend_.get() != NULL); + + DynamicIndexConnectionsPool::Accessor accessor(*this); + backend_->ConfigureDatabase(accessor.GetManager(), hasIdentifierTags, identifierTags); + + StartHousekeepingThread(); + } + + + void DynamicIndexConnectionsPool::CloseConnections() + { + StopHousekeepingThread(); + + boost::mutex::scoped_lock lock(connectionsMutex_); + + for (std::list<DatabaseManager*>::iterator + it = connections_.begin(); it != connections_.end(); ++it) + { + assert(*it != NULL); + (*it)->Close(); + } + } + + DatabaseManager* DynamicIndexConnectionsPool::GetConnection() + { + if (availableConnectionsSemaphore_.TryAcquire(1)) // there is a connection directly available, take it + { + // LOG(INFO) << "--- Reusing an available connection"; + + boost::mutex::scoped_lock lock(connectionsMutex_); + + std::unique_ptr<DatabaseManager> manager(availableConnections_.front()); + availableConnections_.pop_front(); + return manager.release(); + } + else if (connectionsSemaphore_.TryAcquire(1)) // no connection directly available, check if we can create a new one + { + // LOG(INFO) << "--- Creating a new connection"; + + boost::mutex::scoped_lock lock(connectionsMutex_); + connections_.push_back(new DatabaseManager(backend_->CreateDatabaseFactory())); + connections_.back()->GetDatabase(); // Make sure to open the database connection + + // no need to push it in the availableConnections since it is being used immediately + return connections_.back(); + } + else // unable to get a connection now + { + return NULL; + } + + } + + void DynamicIndexConnectionsPool::CleanupOldConnections() + { + boost::mutex::scoped_lock lock(connectionsMutex_); + + while (availableConnectionsSemaphore_.TryAcquire(1)) + { + DatabaseManager* manager = availableConnections_.front(); + if (manager->GetElapsedSecondsSinceLastUse() > 60 || manager->GetElapsedSecondsSinceCreation() > 3600) + { + // LOG(INFO) << "--- Deleting an old connection"; + + availableConnections_.pop_front(); + connections_.remove(manager); + + delete manager; + connectionsSemaphore_.Release(1); + } + else + { + availableConnectionsSemaphore_.Release(1); // we have not consumed it + break; + } + } + + } + + void DynamicIndexConnectionsPool::ReleaseConnection(DatabaseManager* manager) + { + boost::mutex::scoped_lock lock(connectionsMutex_); + availableConnections_.push_front(manager); + availableConnectionsSemaphore_.Release(1); + } + + // DynamicIndexConnectionsPool::Accessor::Accessor(DynamicIndexConnectionsPool& pool) : + // // lock_(pool.connectionsMutex_), + // pool_(pool), + // manager_(NULL) + // { + // for (;;) + // { + // std::unique_ptr<DatabaseManager> manager(pool.GetConnection()); + // if (manager.get() != NULL) + // { + // manager_ = manager.release(); + // return; + // } + // boost::this_thread::sleep(boost::posix_time::millisec(100)); + // } + // } + + + // DynamicIndexConnectionsPool::Accessor::~Accessor() + // { + // assert(manager_ != NULL); + // pool_.ReleaseConnection(manager_); + // // boost::unique_lock<boost::shared_mutex> lock(pool_.connectionsMutex_); + // // pool_.availableConnections_.push_front(manager_); + // // pool_.availableConnectionsSemaphore_.Release(1); + + + // } + + + // IndexBackend& DynamicIndexConnectionsPool::Accessor::GetBackend() const + // { + // return *pool_.backend_; + // } + + + // DatabaseManager& DynamicIndexConnectionsPool::Accessor::GetManager() const + // { + // assert(manager_ != NULL); + // return *manager_; + // } +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Framework/Plugins/DynamicIndexConnectionsPool.h Thu Jun 26 22:31:58 2025 +0200 @@ -0,0 +1,71 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2023 Osimis S.A., Belgium + * Copyright (C) 2024-2025 Orthanc Team SRL, Belgium + * Copyright (C) 2021-2025 Sebastien Jodogne, ICTEAM UCLouvain, 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 "IdentifierTag.h" +#include "IndexBackend.h" +#include "BaseIndexConnectionsPool.h" + +#include <MultiThreading/SharedMessageQueue.h> +#include <MultiThreading/Semaphore.h> + +#include <list> +#include <boost/thread.hpp> + +namespace OrthancDatabases +{ + class DynamicIndexConnectionsPool : public BaseIndexConnectionsPool + { + private: + boost::mutex connectionsMutex_; + size_t maxConnectionsCount_; + Orthanc::Semaphore connectionsSemaphore_; + std::list<DatabaseManager*> connections_; + Orthanc::Semaphore availableConnectionsSemaphore_; + std::list<DatabaseManager*> availableConnections_; + + void CleanupOldConnections(); + + protected: + + virtual DatabaseManager* GetConnection() ORTHANC_OVERRIDE; + + virtual void ReleaseConnection(DatabaseManager* manager) ORTHANC_OVERRIDE; + + virtual void PerformPoolHousekeeping() ORTHANC_OVERRIDE; + + public: + DynamicIndexConnectionsPool(IndexBackend* backend /* takes ownership */, + size_t maxConnectionsCount, + unsigned int houseKeepingDelaySeconds); + + virtual ~DynamicIndexConnectionsPool(); + + virtual void OpenConnections(bool hasIdentifierTags, + const std::list<IdentifierTag>& identifierTags) ORTHANC_OVERRIDE; + + virtual void CloseConnections() ORTHANC_OVERRIDE; + + }; +}
--- a/Framework/Plugins/IndexBackend.cpp Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Plugins/IndexBackend.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -2080,9 +2080,7 @@ break; case Dialect_PostgreSQL: - statement.reset(new DatabaseManager::CachedStatement( - STATEMENT_FROM_HERE, manager, - "SELECT CAST(COUNT(*) AS BIGINT) FROM PatientRecyclingOrder")); + throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented); break; case Dialect_MSSQL: @@ -3035,6 +3033,7 @@ void IndexBackend::Register(IndexBackend* backend, size_t countConnections, + bool useDynamicConnectionPool, unsigned int maxDatabaseRetries, unsigned int housekeepingDelaySeconds) { @@ -3050,7 +3049,7 @@ # if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 12, 0) if (OrthancPluginCheckVersionAdvanced(backend->GetContext(), 1, 12, 0) == 1) { - DatabaseBackendAdapterV4::Register(backend, countConnections, maxDatabaseRetries, housekeepingDelaySeconds); + DatabaseBackendAdapterV4::Register(backend, countConnections, useDynamicConnectionPool, maxDatabaseRetries, housekeepingDelaySeconds); return; } # endif
--- a/Framework/Plugins/IndexBackend.h Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Plugins/IndexBackend.h Thu Jun 26 22:31:58 2025 +0200 @@ -477,6 +477,7 @@ **/ static void Register(IndexBackend* backend, size_t countConnections, + bool useDynamicConnectionPool, unsigned int maxDatabaseRetries, unsigned int housekeepingDelaySeconds);
--- a/Framework/Plugins/IndexConnectionsPool.cpp Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Plugins/IndexConnectionsPool.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -47,63 +47,17 @@ }; - void IndexConnectionsPool::HousekeepingThread(IndexConnectionsPool* that) - { -#if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 12, 2) - OrthancPluginSetCurrentThreadName(OrthancPlugins::GetGlobalContext(), "DB HOUSEKEEPING"); -#endif - - boost::posix_time::ptime lastInvocation = boost::posix_time::second_clock::local_time(); - - while (that->housekeepingContinue_) - { - if (boost::posix_time::second_clock::local_time() - lastInvocation >= that->housekeepingDelay_) - { - try - { - Accessor accessor(*that); - accessor.GetBackend().PerformDbHousekeeping(accessor.GetManager()); - } - catch (Orthanc::OrthancException& e) - { - LOG(ERROR) << "Exception during the database housekeeping: " << e.What(); - } - catch (...) - { - LOG(ERROR) << "Native exception during the database houskeeping"; - } - - lastInvocation = boost::posix_time::second_clock::local_time(); - } - - boost::this_thread::sleep(boost::posix_time::milliseconds(1000)); - } - } - - IndexConnectionsPool::IndexConnectionsPool(IndexBackend* backend, size_t countConnections, unsigned int houseKeepingDelaySeconds) : - backend_(backend), - countConnections_(countConnections), - housekeepingContinue_(true), - housekeepingDelay_(boost::posix_time::seconds(houseKeepingDelaySeconds)) + BaseIndexConnectionsPool(backend, houseKeepingDelaySeconds), + countConnections_(countConnections) { if (countConnections == 0) { throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange, "There must be a non-zero number of connections to the database"); } - else if (backend == NULL) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer); - } - else if (backend->HasPerformDbHousekeeping() && - houseKeepingDelaySeconds == 0) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange, - "The delay between two executions of housekeeping cannot be zero second"); - } else { context_ = backend_->GetContext(); @@ -122,6 +76,13 @@ } + void IndexConnectionsPool::PerformPoolHousekeeping() + { + // this is actually a fixed value ! + OrthancPluginSetMetricsValue(OrthancPlugins::GetGlobalContext(), "orthanc_index_active_connections_count", countConnections_, OrthancPluginMetricsType_Default); + } + + void IndexConnectionsPool::OpenConnections(bool hasIdentifierTags, const std::list<IdentifierTag>& identifierTags) { @@ -152,13 +113,7 @@ availableConnections_.Enqueue(new ManagerReference(**it)); } - // Start the housekeeping thread - housekeepingContinue_ = true; - - if (backend_->HasPerformDbHousekeeping()) - { - housekeepingThread_ = boost::thread(HousekeepingThread, this); - } + StartHousekeepingThread(); } else { @@ -169,14 +124,7 @@ void IndexConnectionsPool::CloseConnections() { - { - // Stop the housekeeping thread - housekeepingContinue_ = false; - if (housekeepingThread_.joinable()) - { - housekeepingThread_.join(); - } - } + StopHousekeepingThread(); boost::unique_lock<boost::shared_mutex> lock(connectionsMutex_); @@ -199,40 +147,20 @@ } } - - IndexConnectionsPool::Accessor::Accessor(IndexConnectionsPool& pool) : - lock_(pool.connectionsMutex_), - pool_(pool), - manager_(NULL) + DatabaseManager* IndexConnectionsPool::GetConnection() { - for (;;) + std::unique_ptr<Orthanc::IDynamicObject> manager(availableConnections_.Dequeue(1)); + if (manager.get() != NULL) { - std::unique_ptr<Orthanc::IDynamicObject> manager(pool.availableConnections_.Dequeue(100)); - if (manager.get() != NULL) - { - manager_ = &dynamic_cast<ManagerReference&>(*manager).GetManager(); - return; - } + return &dynamic_cast<ManagerReference&>(*manager).GetManager(); } + return NULL; } - - IndexConnectionsPool::Accessor::~Accessor() + void IndexConnectionsPool::ReleaseConnection(DatabaseManager* manager) { - assert(manager_ != NULL); - pool_.availableConnections_.Enqueue(new ManagerReference(*manager_)); + assert(manager != NULL); + availableConnections_.Enqueue(new ManagerReference(*manager)); } - - IndexBackend& IndexConnectionsPool::Accessor::GetBackend() const - { - return *pool_.backend_; - } - - - DatabaseManager& IndexConnectionsPool::Accessor::GetManager() const - { - assert(manager_ != NULL); - return *manager_; - } }
--- a/Framework/Plugins/IndexConnectionsPool.h Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Plugins/IndexConnectionsPool.h Thu Jun 26 22:31:58 2025 +0200 @@ -25,6 +25,7 @@ #include "IdentifierTag.h" #include "IndexBackend.h" +#include "BaseIndexConnectionsPool.h" #include <MultiThreading/SharedMessageQueue.h> @@ -33,55 +34,34 @@ namespace OrthancDatabases { - class IndexConnectionsPool : public boost::noncopyable + class IndexConnectionsPool : public BaseIndexConnectionsPool { private: class ManagerReference; - std::unique_ptr<IndexBackend> backend_; - OrthancPluginContext* context_; boost::shared_mutex connectionsMutex_; size_t countConnections_; std::list<DatabaseManager*> connections_; Orthanc::SharedMessageQueue availableConnections_; - bool housekeepingContinue_; - boost::thread housekeepingThread_; - boost::posix_time::time_duration housekeepingDelay_; + + protected: - static void HousekeepingThread(IndexConnectionsPool* that); + virtual DatabaseManager* GetConnection() ORTHANC_OVERRIDE; + + virtual void ReleaseConnection(DatabaseManager* manager) ORTHANC_OVERRIDE; + + virtual void PerformPoolHousekeeping() ORTHANC_OVERRIDE; public: IndexConnectionsPool(IndexBackend* backend /* takes ownership */, size_t countConnections, unsigned int houseKeepingDelaySeconds); - ~IndexConnectionsPool(); - - OrthancPluginContext* GetContext() const - { - return context_; - } - - void OpenConnections(bool hasIdentifierTags, - const std::list<IdentifierTag>& identifierTags); - - void CloseConnections(); + virtual ~IndexConnectionsPool(); - class Accessor : public boost::noncopyable - { - private: - boost::shared_lock<boost::shared_mutex> lock_; - IndexConnectionsPool& pool_; - DatabaseManager* manager_; - - public: - explicit Accessor(IndexConnectionsPool& pool); + virtual void OpenConnections(bool hasIdentifierTags, + const std::list<IdentifierTag>& identifierTags) ORTHANC_OVERRIDE; - ~Accessor(); - - IndexBackend& GetBackend() const; - - DatabaseManager& GetManager() const; - }; + virtual void CloseConnections() ORTHANC_OVERRIDE; }; }
--- a/Framework/Plugins/IndexUnitTests.h Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/Plugins/IndexUnitTests.h Thu Jun 26 22:31:58 2025 +0200 @@ -40,12 +40,17 @@ #if ORTHANC_ENABLE_POSTGRESQL == 1 # define HAS_REVISIONS 1 +// we can not test patient protection in PG because it is now deeply intricated in the CreateInstance function that is too difficult to call from here +# define CAN_TEST_PATIENT_PROTECTION 0 #elif ORTHANC_ENABLE_MYSQL == 1 # define HAS_REVISIONS 0 +# define CAN_TEST_PATIENT_PROTECTION 1 #elif ORTHANC_ENABLE_SQLITE == 1 # define HAS_REVISIONS 1 +# define CAN_TEST_PATIENT_PROTECTION 1 #elif ORTHANC_ENABLE_ODBC == 1 # define HAS_REVISIONS 1 +# define CAN_TEST_PATIENT_PROTECTION 1 #else # error Unknown database backend #endif @@ -266,54 +271,53 @@ ASSERT_EQ(0u, db.GetResourcesCount(*manager, OrthancPluginResourceType_Study)); ASSERT_EQ(0u, db.GetResourcesCount(*manager, OrthancPluginResourceType_Series)); - int64_t a = db.CreateResource(*manager, "study", OrthancPluginResourceType_Study); - ASSERT_TRUE(db.IsExistingResource(*manager, a)); - ASSERT_FALSE(db.IsExistingResource(*manager, a + 1)); + int64_t studyId = db.CreateResource(*manager, "study", OrthancPluginResourceType_Study); + ASSERT_TRUE(db.IsExistingResource(*manager, studyId)); + ASSERT_FALSE(db.IsExistingResource(*manager, studyId + 1)); - int64_t b; + int64_t tmp; OrthancPluginResourceType t; - ASSERT_FALSE(db.LookupResource(b, t, *manager, "world")); - ASSERT_TRUE(db.LookupResource(b, t, *manager, "study")); - ASSERT_EQ(a, b); + ASSERT_FALSE(db.LookupResource(tmp, t, *manager, "world")); + ASSERT_TRUE(db.LookupResource(tmp, t, *manager, "study")); + ASSERT_EQ(studyId, tmp); ASSERT_EQ(OrthancPluginResourceType_Study, t); - b = db.CreateResource(*manager, "series", OrthancPluginResourceType_Series); - ASSERT_NE(a, b); + int64_t seriesId = db.CreateResource(*manager, "series", OrthancPluginResourceType_Series); + ASSERT_NE(studyId, seriesId); - ASSERT_EQ("study", db.GetPublicId(*manager, a)); - ASSERT_EQ("series", db.GetPublicId(*manager, b)); - ASSERT_EQ(OrthancPluginResourceType_Study, db.GetResourceType(*manager, a)); - ASSERT_EQ(OrthancPluginResourceType_Series, db.GetResourceType(*manager, b)); + ASSERT_EQ("study", db.GetPublicId(*manager, studyId)); + ASSERT_EQ("series", db.GetPublicId(*manager, seriesId)); + ASSERT_EQ(OrthancPluginResourceType_Study, db.GetResourceType(*manager, studyId)); + ASSERT_EQ(OrthancPluginResourceType_Series, db.GetResourceType(*manager, seriesId)); - db.AttachChild(*manager, a, b); + db.AttachChild(*manager, studyId, seriesId); - int64_t c; - ASSERT_FALSE(db.LookupParent(c, *manager, a)); - ASSERT_TRUE(db.LookupParent(c, *manager, b)); - ASSERT_EQ(a, c); + ASSERT_FALSE(db.LookupParent(tmp, *manager, studyId)); + ASSERT_TRUE(db.LookupParent(tmp, *manager, seriesId)); + ASSERT_EQ(studyId, tmp); - c = db.CreateResource(*manager, "series2", OrthancPluginResourceType_Series); - db.AttachChild(*manager, a, c); + int64_t series2Id = db.CreateResource(*manager, "series2", OrthancPluginResourceType_Series); + db.AttachChild(*manager, studyId, series2Id); ASSERT_EQ(3u, db.GetAllResourcesCount(*manager)); ASSERT_EQ(0u, db.GetResourcesCount(*manager, OrthancPluginResourceType_Patient)); ASSERT_EQ(1u, db.GetResourcesCount(*manager, OrthancPluginResourceType_Study)); ASSERT_EQ(2u, db.GetResourcesCount(*manager, OrthancPluginResourceType_Series)); - ASSERT_FALSE(db.GetParentPublicId(s, *manager, a)); - ASSERT_TRUE(db.GetParentPublicId(s, *manager, b)); ASSERT_EQ("study", s); - ASSERT_TRUE(db.GetParentPublicId(s, *manager, c)); ASSERT_EQ("study", s); + ASSERT_FALSE(db.GetParentPublicId(s, *manager, studyId)); + ASSERT_TRUE(db.GetParentPublicId(s, *manager, seriesId)); ASSERT_EQ("study", s); + ASSERT_TRUE(db.GetParentPublicId(s, *manager, series2Id)); ASSERT_EQ("study", s); std::list<std::string> children; - db.GetChildren(children, *manager, a); + db.GetChildren(children, *manager, studyId); ASSERT_EQ(2u, children.size()); - db.GetChildren(children, *manager, b); + db.GetChildren(children, *manager, seriesId); ASSERT_EQ(0u, children.size()); - db.GetChildren(children, *manager, c); + db.GetChildren(children, *manager, series2Id); ASSERT_EQ(0u, children.size()); std::list<std::string> cp; - db.GetChildrenPublicId(cp, *manager, a); + db.GetChildrenPublicId(cp, *manager, studyId); ASSERT_EQ(2u, cp.size()); ASSERT_TRUE(cp.front() == "series" || cp.front() == "series2"); ASSERT_TRUE(cp.back() == "series" || cp.back() == "series2"); @@ -332,17 +336,17 @@ ASSERT_NE(pub.front(), pub.back()); std::list<int64_t> ci; - db.GetChildrenInternalId(ci, *manager, a); + db.GetChildrenInternalId(ci, *manager, studyId); ASSERT_EQ(2u, ci.size()); - ASSERT_TRUE(ci.front() == b || ci.front() == c); - ASSERT_TRUE(ci.back() == b || ci.back() == c); + ASSERT_TRUE(ci.front() == seriesId || ci.front() == series2Id); + ASSERT_TRUE(ci.back() == seriesId || ci.back() == series2Id); ASSERT_NE(ci.front(), ci.back()); - db.SetMetadata(*manager, a, Orthanc::MetadataType_ModifiedFrom, "modified", 42); - db.SetMetadata(*manager, a, Orthanc::MetadataType_LastUpdate, "update2", 43); + db.SetMetadata(*manager, studyId, Orthanc::MetadataType_ModifiedFrom, "modified", 42); + db.SetMetadata(*manager, studyId, Orthanc::MetadataType_LastUpdate, "update2", 43); int64_t revision = -1; - ASSERT_FALSE(db.LookupMetadata(s, revision, *manager, b, Orthanc::MetadataType_LastUpdate)); - ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); + ASSERT_FALSE(db.LookupMetadata(s, revision, *manager, seriesId, Orthanc::MetadataType_LastUpdate)); + ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, studyId, Orthanc::MetadataType_LastUpdate)); ASSERT_EQ("update2", s); #if HAS_REVISIONS == 1 @@ -351,8 +355,8 @@ ASSERT_EQ(0, revision); #endif - db.SetMetadata(*manager, a, Orthanc::MetadataType_LastUpdate, reinterpret_cast<const char*>(UTF8), 44); - ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); + db.SetMetadata(*manager, studyId, Orthanc::MetadataType_LastUpdate, reinterpret_cast<const char*>(UTF8), 44); + ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, studyId, Orthanc::MetadataType_LastUpdate)); ASSERT_STREQ(reinterpret_cast<const char*>(UTF8), s.c_str()); #if HAS_REVISIONS == 1 @@ -362,12 +366,12 @@ #endif std::list<int32_t> md; - db.ListAvailableMetadata(md, *manager, a); + db.ListAvailableMetadata(md, *manager, studyId); ASSERT_EQ(2u, md.size()); ASSERT_TRUE(md.front() == Orthanc::MetadataType_ModifiedFrom || md.back() == Orthanc::MetadataType_ModifiedFrom); ASSERT_TRUE(md.front() == Orthanc::MetadataType_LastUpdate || md.back() == Orthanc::MetadataType_LastUpdate); std::string mdd; - ASSERT_TRUE(db.LookupMetadata(mdd, revision, *manager, a, Orthanc::MetadataType_ModifiedFrom)); + ASSERT_TRUE(db.LookupMetadata(mdd, revision, *manager, studyId, Orthanc::MetadataType_ModifiedFrom)); ASSERT_EQ("modified", mdd); #if HAS_REVISIONS == 1 @@ -376,7 +380,7 @@ ASSERT_EQ(0, revision); #endif - ASSERT_TRUE(db.LookupMetadata(mdd, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); + ASSERT_TRUE(db.LookupMetadata(mdd, revision, *manager, studyId, Orthanc::MetadataType_LastUpdate)); ASSERT_EQ(reinterpret_cast<const char*>(UTF8), mdd); #if HAS_REVISIONS == 1 @@ -385,16 +389,16 @@ ASSERT_EQ(0, revision); #endif - db.ListAvailableMetadata(md, *manager, b); + db.ListAvailableMetadata(md, *manager, seriesId); ASSERT_EQ(0u, md.size()); - ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); - db.DeleteMetadata(*manager, a, Orthanc::MetadataType_LastUpdate); - ASSERT_FALSE(db.LookupMetadata(s, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); - db.DeleteMetadata(*manager, b, Orthanc::MetadataType_LastUpdate); - ASSERT_FALSE(db.LookupMetadata(s, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); + ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, studyId, Orthanc::MetadataType_LastUpdate)); + db.DeleteMetadata(*manager, studyId, Orthanc::MetadataType_LastUpdate); + ASSERT_FALSE(db.LookupMetadata(s, revision, *manager, studyId, Orthanc::MetadataType_LastUpdate)); + db.DeleteMetadata(*manager, seriesId, Orthanc::MetadataType_LastUpdate); + ASSERT_FALSE(db.LookupMetadata(s, revision, *manager, studyId, Orthanc::MetadataType_LastUpdate)); - db.ListAvailableMetadata(md, *manager, a); + db.ListAvailableMetadata(md, *manager, studyId); ASSERT_EQ(1u, md.size()); ASSERT_EQ(Orthanc::MetadataType_ModifiedFrom, md.front()); @@ -404,32 +408,32 @@ std::list<int32_t> fc; - OrthancPluginAttachment a1; - a1.uuid = "uuid1"; - a1.contentType = Orthanc::FileContentType_Dicom; - a1.uncompressedSize = 42; - a1.uncompressedHash = "md5_1"; - a1.compressionType = Orthanc::CompressionType_None; - a1.compressedSize = 42; - a1.compressedHash = "md5_1"; + OrthancPluginAttachment att1; + att1.uuid = "uuid1"; + att1.contentType = Orthanc::FileContentType_Dicom; + att1.uncompressedSize = 42; + att1.uncompressedHash = "md5_1"; + att1.compressionType = Orthanc::CompressionType_None; + att1.compressedSize = 42; + att1.compressedHash = "md5_1"; - OrthancPluginAttachment a2; - a2.uuid = "uuid2"; - a2.contentType = Orthanc::FileContentType_DicomAsJson; - a2.uncompressedSize = 4242; - a2.uncompressedHash = "md5_2"; - a2.compressionType = Orthanc::CompressionType_None; - a2.compressedSize = 4242; - a2.compressedHash = "md5_2"; + OrthancPluginAttachment att2; + att2.uuid = "uuid2"; + att2.contentType = Orthanc::FileContentType_DicomAsJson; + att2.uncompressedSize = 4242; + att2.uncompressedHash = "md5_2"; + att2.compressionType = Orthanc::CompressionType_None; + att2.compressedSize = 4242; + att2.compressedHash = "md5_2"; - db.AddAttachment(*manager, a, a1, 42); - db.ListAvailableAttachments(fc, *manager, a); + db.AddAttachment(*manager, studyId, att1, 42); + db.ListAvailableAttachments(fc, *manager, studyId); ASSERT_EQ(1u, fc.size()); ASSERT_EQ(Orthanc::FileContentType_Dicom, fc.front()); - db.AddAttachment(*manager, a, a2, 43); - db.ListAvailableAttachments(fc, *manager, a); + db.AddAttachment(*manager, studyId, att2, 43); + db.ListAvailableAttachments(fc, *manager, studyId); ASSERT_EQ(2u, fc.size()); - ASSERT_FALSE(db.LookupAttachment(*output, revision, *manager, b, Orthanc::FileContentType_Dicom)); + ASSERT_FALSE(db.LookupAttachment(*output, revision, *manager, seriesId, Orthanc::FileContentType_Dicom)); ASSERT_EQ(4284u, db.GetTotalCompressedSize(*manager)); ASSERT_EQ(4284u, db.GetTotalUncompressedSize(*manager)); @@ -442,7 +446,7 @@ expectedAttachment->compressionType = Orthanc::CompressionType_None; expectedAttachment->compressedSize = 42; expectedAttachment->compressedHash = "md5_1"; - ASSERT_TRUE(db.LookupAttachment(*output, revision, *manager, a, Orthanc::FileContentType_Dicom)); + ASSERT_TRUE(db.LookupAttachment(*output, revision, *manager, studyId, Orthanc::FileContentType_Dicom)); #if HAS_REVISIONS == 1 ASSERT_EQ(42, revision); @@ -459,7 +463,7 @@ expectedAttachment->compressedSize = 4242; expectedAttachment->compressedHash = "md5_2"; revision = -1; - ASSERT_TRUE(db.LookupAttachment(*output, revision, *manager, a, Orthanc::FileContentType_DicomAsJson)); + ASSERT_TRUE(db.LookupAttachment(*output, revision, *manager, studyId, Orthanc::FileContentType_DicomAsJson)); #if HAS_REVISIONS == 1 ASSERT_EQ(43, revision); @@ -467,21 +471,21 @@ ASSERT_EQ(0, revision); #endif - db.ListAvailableAttachments(fc, *manager, b); + db.ListAvailableAttachments(fc, *manager, seriesId); ASSERT_EQ(0u, fc.size()); - db.DeleteAttachment(*output, *manager, a, Orthanc::FileContentType_Dicom); - db.ListAvailableAttachments(fc, *manager, a); + db.DeleteAttachment(*output, *manager, studyId, Orthanc::FileContentType_Dicom); + db.ListAvailableAttachments(fc, *manager, studyId); ASSERT_EQ(1u, fc.size()); ASSERT_EQ(Orthanc::FileContentType_DicomAsJson, fc.front()); - db.DeleteAttachment(*output, *manager, a, Orthanc::FileContentType_DicomAsJson); - db.ListAvailableAttachments(fc, *manager, a); + db.DeleteAttachment(*output, *manager, studyId, Orthanc::FileContentType_DicomAsJson); + db.ListAvailableAttachments(fc, *manager, studyId); ASSERT_EQ(0u, fc.size()); - db.SetIdentifierTag(*manager, a, 0x0010, 0x0020, "patient"); - db.SetIdentifierTag(*manager, a, 0x0020, 0x000d, "study"); - db.SetMainDicomTag(*manager, a, 0x0010, 0x0020, "patient"); - db.SetMainDicomTag(*manager, a, 0x0020, 0x000d, "study"); - db.SetMainDicomTag(*manager, a, 0x0008, 0x1030, reinterpret_cast<const char*>(UTF8)); + db.SetIdentifierTag(*manager, studyId, 0x0010, 0x0020, "patient"); + db.SetIdentifierTag(*manager, studyId, 0x0020, 0x000d, "study"); + db.SetMainDicomTag(*manager, studyId, 0x0010, 0x0020, "patient"); + db.SetMainDicomTag(*manager, studyId, 0x0020, 0x000d, "study"); + db.SetMainDicomTag(*manager, studyId, 0x0008, 0x1030, reinterpret_cast<const char*>(UTF8)); expectedDicomTags.clear(); expectedDicomTags.push_back(OrthancPluginDicomTag()); @@ -498,13 +502,13 @@ expectedDicomTags.back().value = reinterpret_cast<const char*>(UTF8); countDicomTags = 0; - db.GetMainDicomTags(*output, *manager, a); + db.GetMainDicomTags(*output, *manager, studyId); ASSERT_EQ(3u, countDicomTags); db.LookupIdentifier(ci, *manager, OrthancPluginResourceType_Study, 0x0010, 0x0020, OrthancPluginIdentifierConstraint_Equal, "patient"); ASSERT_EQ(1u, ci.size()); - ASSERT_EQ(a, ci.front()); + ASSERT_EQ(studyId, ci.front()); db.LookupIdentifier(ci, *manager, OrthancPluginResourceType_Study, 0x0010, 0x0020, OrthancPluginIdentifierConstraint_Equal, "study"); ASSERT_EQ(0u, ci.size()); @@ -534,8 +538,11 @@ db.GetAllPublicIds(pub, *manager, OrthancPluginResourceType_Instance); ASSERT_EQ(0u, pub.size()); ASSERT_EQ(3u, db.GetAllResourcesCount(*manager)); - ASSERT_EQ(0u, db.GetUnprotectedPatientsCount(*manager)); // No patient was inserted - ASSERT_TRUE(db.IsExistingResource(*manager, c)); + #if CAN_TEST_PATIENT_PROTECTION == 1 + ASSERT_EQ(0u, db.GetUnprotectedPatientsCount(*manager)); // No patient was inserted + #endif + + ASSERT_TRUE(db.IsExistingResource(*manager, series2Id)); { // A transaction is needed here for MySQL, as it was not possible @@ -547,7 +554,7 @@ deletedResources.clear(); remainingAncestor.reset(); - db.DeleteResource(*output, *manager, c); + db.DeleteResource(*output, *manager, series2Id); ASSERT_EQ(0u, deletedAttachments.size()); ASSERT_EQ(1u, deletedResources.size()); @@ -563,22 +570,22 @@ deletedResources.clear(); remainingAncestor.reset(); - ASSERT_FALSE(db.IsExistingResource(*manager, c)); - ASSERT_TRUE(db.IsExistingResource(*manager, a)); - ASSERT_TRUE(db.IsExistingResource(*manager, b)); + ASSERT_FALSE(db.IsExistingResource(*manager, series2Id)); + ASSERT_TRUE(db.IsExistingResource(*manager, studyId)); + ASSERT_TRUE(db.IsExistingResource(*manager, seriesId)); ASSERT_EQ(2u, db.GetAllResourcesCount(*manager)); { // An explicit transaction is needed here manager->StartTransaction(TransactionType_ReadWrite); - db.DeleteResource(*output, *manager, a); + db.DeleteResource(*output, *manager, studyId); // delete the study that only has one series left -> 2 resources shall be deleted manager->CommitTransaction(); } ASSERT_EQ(0u, db.GetAllResourcesCount(*manager)); - ASSERT_FALSE(db.IsExistingResource(*manager, a)); - ASSERT_FALSE(db.IsExistingResource(*manager, b)); - ASSERT_FALSE(db.IsExistingResource(*manager, c)); + ASSERT_FALSE(db.IsExistingResource(*manager, studyId)); + ASSERT_FALSE(db.IsExistingResource(*manager, seriesId)); + ASSERT_FALSE(db.IsExistingResource(*manager, series2Id)); ASSERT_EQ(0u, deletedAttachments.size()); ASSERT_EQ(2u, deletedResources.size()); @@ -587,6 +594,7 @@ ASSERT_FALSE(remainingAncestor.get() != NULL); ASSERT_EQ(0u, db.GetAllResourcesCount(*manager)); +#if CAN_TEST_PATIENT_PROTECTION == 1 ASSERT_EQ(0u, db.GetUnprotectedPatientsCount(*manager)); int64_t p1 = db.CreateResource(*manager, "patient1", OrthancPluginResourceType_Patient); int64_t p2 = db.CreateResource(*manager, "patient2", OrthancPluginResourceType_Patient); @@ -618,6 +626,14 @@ ASSERT_EQ(p1, r); { + manager->StartTransaction(TransactionType_ReadWrite); + db.DeleteResource(*output, *manager, p1); + db.DeleteResource(*output, *manager, p3); + manager->CommitTransaction(); + } +#endif + + { // Test creating a large property of 16MB (large properties are // notably necessary to serialize jobs) // https://groups.google.com/g/orthanc-users/c/1Y3nTBdr0uE/m/K7PA5pboAgAJ @@ -643,14 +659,6 @@ ASSERT_EQ(longProperty, tmp); } - { - manager->StartTransaction(TransactionType_ReadWrite); - db.DeleteResource(*output, *manager, p1); - db.DeleteResource(*output, *manager, p3); - manager->CommitTransaction(); - } - - for (size_t level = 0; level < 4; level++) { for (size_t attachmentLevel = 0; attachmentLevel < 4; attachmentLevel++)
--- a/Framework/PostgreSQL/PostgreSQLDatabase.cpp Wed Jun 04 15:28:35 2025 +0200 +++ b/Framework/PostgreSQL/PostgreSQLDatabase.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -63,7 +63,7 @@ { if (pg_ != NULL) { - LOG(INFO) << "Closing connection to PostgreSQL"; + LOG(TRACE) << "Closing connection to PostgreSQL"; PQfinish(reinterpret_cast<PGconn*>(pg_)); pg_ = NULL; }
--- a/PostgreSQL/CMakeLists.txt Wed Jun 04 15:28:35 2025 +0200 +++ b/PostgreSQL/CMakeLists.txt Thu Jun 26 22:31:58 2025 +0200 @@ -94,7 +94,7 @@ POSTGRESQL_UPGRADE_REV1_TO_REV2 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev1ToRev2.sql POSTGRESQL_UPGRADE_REV2_TO_REV3 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev2ToRev3.sql POSTGRESQL_UPGRADE_REV3_TO_REV4 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev3ToRev4.sql - POSTGRESQL_UPGRADE_REV4_TO_REV99 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev4ToRev99.sql + POSTGRESQL_UPGRADE_REV4_TO_REV499 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev4ToRev499.sql )
--- a/PostgreSQL/NEWS Wed Jun 04 15:28:35 2025 +0200 +++ b/PostgreSQL/NEWS Thu Jun 26 22:31:58 2025 +0200 @@ -10,6 +10,16 @@ Minimal Postgresql Server version: 9 Optimal Postgresql Server version: 11+ +* New configuration "UseDynamicConnectionPool" to dynamically create/release + connections to the PostgreSQL database. + When this option is disabled (default), the connections are created once + at the start of the plugin and are kept alive for the whole execution. + Using the dynamic mode enables cleanup of some temporary tables that + could otherwise accumulate dead rows. + When using a dynamic pool, connections are released either after 60 seconds + of being idle or one hour after their creation. +* New metrics "orthanc_index_active_connections_count" showing the current number + of active connections. Maintenance: * Optimized the CreateInstance SQL query.
--- a/PostgreSQL/Plugins/IndexPlugin.cpp Wed Jun 04 15:28:35 2025 +0200 +++ b/PostgreSQL/Plugins/IndexPlugin.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -79,11 +79,12 @@ try { const size_t countConnections = postgresql.GetUnsignedIntegerValue("IndexConnectionsCount", 50); + const bool useDynamicConnectionPool = postgresql.GetBooleanValue("UseDynamicConnectionPool", false); const unsigned int housekeepingDelaySeconds = postgresql.GetUnsignedIntegerValue("HousekeepingInterval", 1); OrthancDatabases::PostgreSQLParameters parameters(postgresql); OrthancDatabases::IndexBackend::Register( - new OrthancDatabases::PostgreSQLIndex(context, parameters, readOnly), countConnections, + new OrthancDatabases::PostgreSQLIndex(context, parameters, readOnly), countConnections, useDynamicConnectionPool, parameters.GetMaxConnectionRetries(), housekeepingDelaySeconds); } catch (Orthanc::OrthancException& e)
--- a/PostgreSQL/Plugins/PostgreSQLIndex.cpp Wed Jun 04 15:28:35 2025 +0200 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.cpp Thu Jun 26 22:31:58 2025 +0200 @@ -49,7 +49,7 @@ static const GlobalProperty GlobalProperty_HasComputeStatisticsReadOnly = GlobalProperty_DatabaseInternal4; } -#define CURRENT_DB_REVISION 99 +#define CURRENT_DB_REVISION 499 namespace OrthancDatabases { @@ -242,10 +242,10 @@ std::string query; Orthanc::EmbeddedResources::GetFileResource - (query, Orthanc::EmbeddedResources::POSTGRESQL_UPGRADE_REV4_TO_REV99); + (query, Orthanc::EmbeddedResources::POSTGRESQL_UPGRADE_REV4_TO_REV499); t.GetDatabaseTransaction().ExecuteMultiLines(query); hasAppliedAnUpgrade = true; - currentRevision = 99; + currentRevision = 499; } if (hasAppliedAnUpgrade) @@ -483,12 +483,35 @@ void PostgreSQLIndex::ClearDeletedFiles(DatabaseManager& manager) { - // not used anymore in PostgreSQL + { // note: the temporary table lifespan is the session, not the transaction -> that's why we need the IF NOT EXISTS + DatabaseManager::CachedStatement statement( + STATEMENT_FROM_HERE, manager, + "SELECT CreateDeletedFilesTemporaryTable()" + ); + statement.ExecuteWithoutResult(); + } } void PostgreSQLIndex::ClearDeletedResources(DatabaseManager& manager) { - // not used anymore in PostgreSQL + { // note: the temporary table lifespan is the session, not the transaction -> that's why we need the IF NOT EXISTS + DatabaseManager::CachedStatement statement( + STATEMENT_FROM_HERE, manager, + "CREATE TEMPORARY TABLE IF NOT EXISTS DeletedResources(" + "resourceType INTEGER NOT NULL," + "publicId VARCHAR(64) NOT NULL" + ");" + ); + statement.Execute(); + } + { + DatabaseManager::CachedStatement statement( + STATEMENT_FROM_HERE, manager, + "DELETE FROM DeletedResources;" + ); + + statement.Execute(); + } } void PostgreSQLIndex::ClearRemainingAncestor(DatabaseManager& manager)
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/PostgreSQL/Plugins/SQL/Downgrades/Rev499ToRev4.sql Thu Jun 26 22:31:58 2025 +0200 @@ -0,0 +1,241 @@ +-- This file contains an SQL procedure to downgrade from schema Rev499 to Rev4 (version = 6). + + +-- Re-installs the old PatientRecycling +----------- + +CREATE TABLE IF NOT EXISTS PatientRecyclingOrder( + seq BIGSERIAL NOT NULL PRIMARY KEY, + patientId BIGINT REFERENCES Resources(internalId) ON DELETE CASCADE, + CONSTRAINT UniquePatientId UNIQUE (patientId) + ); + +CREATE INDEX IF NOT EXISTS PatientRecyclingIndex ON PatientRecyclingOrder(patientId); + +DROP TRIGGER IF EXISTS PatientAdded ON Resources; + +CREATE OR REPLACE FUNCTION PatientAddedOrUpdated( + IN patient_id BIGINT, + IN is_update BIGINT + ) +RETURNS VOID AS $body$ +BEGIN + DECLARE + newSeq BIGINT; + BEGIN + IF is_update > 0 THEN + -- Note: Protected patients are not listed in this table ! So, they won't be updated + WITH deleted_rows AS ( + DELETE FROM PatientRecyclingOrder + WHERE PatientRecyclingOrder.patientId = patient_id + RETURNING patientId + ) + INSERT INTO PatientRecyclingOrder (patientId) + SELECT patientID FROM deleted_rows + WHERE EXISTS(SELECT 1 FROM deleted_rows); + ELSE + INSERT INTO PatientRecyclingOrder VALUES (DEFAULT, patient_id); + END IF; + END; +END; +$body$ LANGUAGE plpgsql; + +CREATE OR REPLACE FUNCTION PatientAddedFunc() +RETURNS TRIGGER AS $body$ +BEGIN + -- The "0" corresponds to "OrthancPluginResourceType_Patient" + IF new.resourceType = 0 THEN + PERFORM PatientAddedOrUpdated(new.internalId, 0); + END IF; + RETURN NULL; +END; +$body$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS PatientAdded on Resources; +CREATE TRIGGER PatientAdded +AFTER INSERT ON Resources +FOR EACH ROW +EXECUTE PROCEDURE PatientAddedFunc(); + +DROP FUNCTION IF EXISTS ProtectPatient(patient_id BIGINT); + +DROP FUNCTION IF EXISTS UnprotectPatient; + +-- repopulate the PatientRecyclingOrderTable +WITH UnprotectedPatients AS (SELECT r.internalId + FROM Resources r + RIGHT JOIN Metadata m ON r.internalId = m.id AND m.type = 19 -- 19 = PatientRecyclingOrder + WHERE r.resourceType = 0 + AND NOT EXISTS (SELECT 1 FROM Metadata m + WHERE m.id = r.internalId AND m.type = 18 AND m.value = 'true') -- 18 = IsProtected + ORDER BY CAST(m.value AS INTEGER) ASC) + +INSERT INTO PatientRecyclingOrder (patientId) +SELECT internalId +FROM UnprotectedPatients; + +DROP SEQUENCE IF EXISTS PatientRecyclingOrderSequence; + +-- remove the IsProtected and PatientRecyclingOrder metadata +DELETE FROM Metadata WHERE type IN (18, 19); + +-- Re-installs the old CreateInstance method +----------- + +CREATE OR REPLACE FUNCTION CreateInstance( + IN patient_public_id TEXT, + IN study_public_id TEXT, + IN series_public_id TEXT, + IN instance_public_id TEXT, + OUT is_new_patient BIGINT, + OUT is_new_study BIGINT, + OUT is_new_series BIGINT, + OUT is_new_instance BIGINT, + OUT patient_internal_id BIGINT, + OUT study_internal_id BIGINT, + OUT series_internal_id BIGINT, + OUT instance_internal_id BIGINT) AS $body$ + +BEGIN + is_new_patient := 1; + is_new_study := 1; + is_new_series := 1; + is_new_instance := 1; + + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 0, patient_public_id, NULL, 0) RETURNING internalid INTO patient_internal_id; + EXCEPTION + WHEN unique_violation THEN + is_new_patient := 0; + SELECT internalid INTO patient_internal_id FROM "resources" WHERE publicId = patient_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction + END; + + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 1, study_public_id, patient_internal_id, 0) RETURNING internalid INTO study_internal_id; + EXCEPTION + WHEN unique_violation THEN + is_new_study := 0; + SELECT internalid INTO study_internal_id FROM "resources" WHERE publicId = study_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction END; + END; + + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 2, series_public_id, study_internal_id, 0) RETURNING internalid INTO series_internal_id; + EXCEPTION + WHEN unique_violation THEN + is_new_series := 0; + SELECT internalid INTO series_internal_id FROM "resources" WHERE publicId = series_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction END; + END; + + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 3, instance_public_id, series_internal_id, 0) RETURNING internalid INTO instance_internal_id; + EXCEPTION + WHEN unique_violation THEN + is_new_instance := 0; + SELECT internalid INTO instance_internal_id FROM "resources" WHERE publicId = instance_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction + END; + + IF is_new_instance > 0 THEN + -- Move the patient to the end of the recycling order. + PERFORM PatientAddedOrUpdated(patient_internal_id, 1); + END IF; +END; +$body$ LANGUAGE plpgsql; + +-- Restore the DeleteResource function that has been optimized + +------------------- DeleteResource function ------------------- + +CREATE OR REPLACE FUNCTION DeleteResource( + IN id BIGINT, + OUT remaining_ancestor_resource_type INTEGER, + OUT remaining_anncestor_public_id TEXT) AS $body$ + +DECLARE + deleted_row RECORD; + locked_row RECORD; + +BEGIN + + SET client_min_messages = warning; -- suppress NOTICE: relation "deletedresources" already exists, skipping + + -- note: temporary tables are created at session (connection) level -> they are likely to exist + -- these tables are used by the triggers + CREATE TEMPORARY TABLE IF NOT EXISTS DeletedResources( + resourceType INTEGER NOT NULL, + publicId VARCHAR(64) NOT NULL + ); + + RESET client_min_messages; + + -- clear the temporary table in case it has been created earlier in the session + DELETE FROM DeletedResources; + + -- create/clear the DeletedFiles temporary table + PERFORM CreateDeletedFilesTemporaryTable(); + + -- Before deleting an object, we need to lock its parent until the end of the transaction to avoid that + -- 2 threads deletes the last 2 instances of a series at the same time -> none of them would realize + -- that they are deleting the last instance and the parent resources would not be deleted. + -- Locking only the immediate parent is sufficient to prevent from this. + SELECT * INTO locked_row FROM resources WHERE internalid = (SELECT parentid FROM resources WHERE internalid = id) FOR UPDATE; + + -- delete the resource itself + DELETE FROM Resources WHERE internalId=id RETURNING * INTO deleted_row; + -- note: there is a ResourceDeletedFunc trigger that will execute here and delete the parents if there are no remaining children + + + -- If this resource still has siblings, keep track of the remaining parent + -- (a parent that must not be deleted but whose LastUpdate must be updated) + SELECT resourceType, publicId INTO remaining_ancestor_resource_type, remaining_anncestor_public_id + FROM Resources + WHERE internalId = deleted_row.parentId + AND EXISTS (SELECT 1 FROM Resources WHERE parentId = deleted_row.parentId); + +END; + +$body$ LANGUAGE plpgsql; + + +-- restore the DeletedResource trigger + +------------------- ResourceDeleted trigger ------------------- +DROP TRIGGER IF EXISTS ResourceDeleted ON Resources; + +-- The following trigger combines 2 triggers from SQLite: +-- ResourceDeleted + ResourceDeletedParentCleaning +CREATE OR REPLACE FUNCTION ResourceDeletedFunc() +RETURNS TRIGGER AS $body$ +BEGIN + -- RAISE NOTICE 'ResourceDeletedFunc %', old.publicId; + INSERT INTO DeletedResources VALUES (old.resourceType, old.publicId); + + -- If this resource is the latest child, delete the parent + DELETE FROM Resources WHERE internalId = old.parentId + AND NOT EXISTS (SELECT 1 FROM Resources WHERE parentId = old.parentId); + RETURN NULL; +END; +$body$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS ResourceDeleted on Resources; +CREATE TRIGGER ResourceDeleted +AFTER DELETE ON Resources +FOR EACH ROW +EXECUTE PROCEDURE ResourceDeletedFunc(); + + +-- remove the new DeleteAttachment function + +DROP FUNCTION IF EXISTS DeleteAttachment; + +-- Restore the ON DELETE CASCADE on the Resources.parentId +-- Drop the existing foreign key constraint and add a new one without ON DELETE CASCADE in a single command +ALTER TABLE Resources +DROP CONSTRAINT IF EXISTS resources_parentid_fkey, +ADD CONSTRAINT resources_parentid_fkey FOREIGN KEY (parentId) REFERENCES Resources(internalId) ON DELETE CASCADE; + + +---------- + +-- set the global properties that actually documents the DB version, revision and some of the capabilities +-- modify only the ones that have changed +DELETE FROM GlobalProperties WHERE property IN (4, 11); +INSERT INTO GlobalProperties VALUES (4, 4); -- GlobalProperty_DatabasePatchLevel
--- a/PostgreSQL/Plugins/SQL/Downgrades/Rev99ToRev4.sql Wed Jun 04 15:28:35 2025 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,259 +0,0 @@ --- This file contains an SQL procedure to downgrade from schema Rev99 to Rev4 (version = 6). - - --- Re-installs the old PatientRecycling ------------ - -CREATE TABLE IF NOT EXISTS PatientRecyclingOrder( - seq BIGSERIAL NOT NULL PRIMARY KEY, - patientId BIGINT REFERENCES Resources(internalId) ON DELETE CASCADE, - CONSTRAINT UniquePatientId UNIQUE (patientId) - ); - -CREATE INDEX IF NOT EXISTS PatientRecyclingIndex ON PatientRecyclingOrder(patientId); - -DROP TRIGGER IF EXISTS PatientAdded ON Resources; - -CREATE OR REPLACE FUNCTION PatientAddedOrUpdated( - IN patient_id BIGINT, - IN is_update BIGINT - ) -RETURNS VOID AS $body$ -BEGIN - DECLARE - newSeq BIGINT; - BEGIN - IF is_update > 0 THEN - -- Note: Protected patients are not listed in this table ! So, they won't be updated - WITH deleted_rows AS ( - DELETE FROM PatientRecyclingOrder - WHERE PatientRecyclingOrder.patientId = patient_id - RETURNING patientId - ) - INSERT INTO PatientRecyclingOrder (patientId) - SELECT patientID FROM deleted_rows - WHERE EXISTS(SELECT 1 FROM deleted_rows); - ELSE - INSERT INTO PatientRecyclingOrder VALUES (DEFAULT, patient_id); - END IF; - END; -END; -$body$ LANGUAGE plpgsql; - -CREATE OR REPLACE FUNCTION PatientAddedFunc() -RETURNS TRIGGER AS $body$ -BEGIN - -- The "0" corresponds to "OrthancPluginResourceType_Patient" - IF new.resourceType = 0 THEN - PERFORM PatientAddedOrUpdated(new.internalId, 0); - END IF; - RETURN NULL; -END; -$body$ LANGUAGE plpgsql; - -DROP TRIGGER IF EXISTS PatientAdded on Resources; -CREATE TRIGGER PatientAdded -AFTER INSERT ON Resources -FOR EACH ROW -EXECUTE PROCEDURE PatientAddedFunc(); - -DROP FUNCTION IF EXISTS ProtectPatient(patient_id BIGINT); - -DROP FUNCTION IF EXISTS UnprotectPatient; - --- repopulate the PatientRecyclingOrderTable -WITH UnprotectedPatients AS (SELECT r.internalId - FROM Resources r - RIGHT JOIN Metadata m ON r.internalId = m.id AND m.type = 19 -- 19 = PatientRecyclingOrder - WHERE r.resourceType = 0 - AND NOT EXISTS (SELECT 1 FROM Metadata m - WHERE m.id = r.internalId AND m.type = 18 AND m.value = 'true') -- 18 = IsProtected - ORDER BY CAST(m.value AS INTEGER) ASC) - -INSERT INTO PatientRecyclingOrder (patientId) -SELECT internalId -FROM UnprotectedPatients; - -DROP SEQUENCE IF EXISTS PatientRecyclingOrderSequence; - --- remove the IsProtected and PatientRecyclingOrder metadata -DELETE FROM Metadata WHERE type IN (18, 19); - --- Re-installs the old CreateInstance method ------------ - -CREATE OR REPLACE FUNCTION CreateInstance( - IN patient_public_id TEXT, - IN study_public_id TEXT, - IN series_public_id TEXT, - IN instance_public_id TEXT, - OUT is_new_patient BIGINT, - OUT is_new_study BIGINT, - OUT is_new_series BIGINT, - OUT is_new_instance BIGINT, - OUT patient_internal_id BIGINT, - OUT study_internal_id BIGINT, - OUT series_internal_id BIGINT, - OUT instance_internal_id BIGINT) AS $body$ - -BEGIN - is_new_patient := 1; - is_new_study := 1; - is_new_series := 1; - is_new_instance := 1; - - BEGIN - INSERT INTO "resources" VALUES (DEFAULT, 0, patient_public_id, NULL, 0) RETURNING internalid INTO patient_internal_id; - EXCEPTION - WHEN unique_violation THEN - is_new_patient := 0; - SELECT internalid INTO patient_internal_id FROM "resources" WHERE publicId = patient_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction - END; - - BEGIN - INSERT INTO "resources" VALUES (DEFAULT, 1, study_public_id, patient_internal_id, 0) RETURNING internalid INTO study_internal_id; - EXCEPTION - WHEN unique_violation THEN - is_new_study := 0; - SELECT internalid INTO study_internal_id FROM "resources" WHERE publicId = study_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction END; - END; - - BEGIN - INSERT INTO "resources" VALUES (DEFAULT, 2, series_public_id, study_internal_id, 0) RETURNING internalid INTO series_internal_id; - EXCEPTION - WHEN unique_violation THEN - is_new_series := 0; - SELECT internalid INTO series_internal_id FROM "resources" WHERE publicId = series_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction END; - END; - - BEGIN - INSERT INTO "resources" VALUES (DEFAULT, 3, instance_public_id, series_internal_id, 0) RETURNING internalid INTO instance_internal_id; - EXCEPTION - WHEN unique_violation THEN - is_new_instance := 0; - SELECT internalid INTO instance_internal_id FROM "resources" WHERE publicId = instance_public_id FOR UPDATE; -- also locks the resource and its parent to prevent from deletion while we complete this transaction - END; - - IF is_new_instance > 0 THEN - -- Move the patient to the end of the recycling order. - PERFORM PatientAddedOrUpdated(patient_internal_id, 1); - END IF; -END; -$body$ LANGUAGE plpgsql; - --- Restore these 2 functions that have been optimized ------------ -------------------- DeleteResource function ------------------- - -CREATE OR REPLACE FUNCTION DeleteResource( - IN id BIGINT, - OUT remaining_ancestor_resource_type INTEGER, - OUT remaining_anncestor_public_id TEXT) AS $body$ - -DECLARE - deleted_row RECORD; - locked_row RECORD; - -BEGIN - - SET client_min_messages = warning; -- suppress NOTICE: relation "deletedresources" already exists, skipping - - -- note: temporary tables are created at session (connection) level -> they are likely to exist - -- these tables are used by the triggers - CREATE TEMPORARY TABLE IF NOT EXISTS DeletedResources( - resourceType INTEGER NOT NULL, - publicId VARCHAR(64) NOT NULL - ); - - RESET client_min_messages; - - -- clear the temporary table in case it has been created earlier in the session - DELETE FROM DeletedResources; - - -- create/clear the DeletedFiles temporary table - PERFORM CreateDeletedFilesTemporaryTable(); - - -- Before deleting an object, we need to lock its parent until the end of the transaction to avoid that - -- 2 threads deletes the last 2 instances of a series at the same time -> none of them would realize - -- that they are deleting the last instance and the parent resources would not be deleted. - -- Locking only the immediate parent is sufficient to prevent from this. - SELECT * INTO locked_row FROM resources WHERE internalid = (SELECT parentid FROM resources WHERE internalid = id) FOR UPDATE; - - -- delete the resource itself - DELETE FROM Resources WHERE internalId=id RETURNING * INTO deleted_row; - -- note: there is a ResourceDeletedFunc trigger that will execute here and delete the parents if there are no remaining children + - - -- If this resource still has siblings, keep track of the remaining parent - -- (a parent that must not be deleted but whose LastUpdate must be updated) - SELECT resourceType, publicId INTO remaining_ancestor_resource_type, remaining_anncestor_public_id - FROM Resources - WHERE internalId = deleted_row.parentId - AND EXISTS (SELECT 1 FROM Resources WHERE parentId = deleted_row.parentId); - -END; - -$body$ LANGUAGE plpgsql; - -CREATE OR REPLACE FUNCTION CreateDeletedFilesTemporaryTable( -) RETURNS VOID AS $body$ - -BEGIN - - SET client_min_messages = warning; -- suppress NOTICE: relation "deletedresources" already exists, skipping - - -- note: temporary tables are created at session (connection) level -> they are likely to exist - CREATE TEMPORARY TABLE IF NOT EXISTS DeletedFiles( - uuid VARCHAR(64) NOT NULL, - fileType INTEGER, - compressedSize BIGINT, - uncompressedSize BIGINT, - compressionType INTEGER, - uncompressedHash VARCHAR(40), - compressedHash VARCHAR(40) - ); - - RESET client_min_messages; - - -- clear the temporary table in case it has been created earlier in the session - DELETE FROM DeletedFiles; -END; - -$body$ LANGUAGE plpgsql; - --- restore the DeletedResource trigger - -------------------- ResourceDeleted trigger ------------------- -DROP TRIGGER IF EXISTS ResourceDeleted ON Resources; - --- The following trigger combines 2 triggers from SQLite: --- ResourceDeleted + ResourceDeletedParentCleaning -CREATE OR REPLACE FUNCTION ResourceDeletedFunc() -RETURNS TRIGGER AS $body$ -BEGIN - -- RAISE NOTICE 'ResourceDeletedFunc %', old.publicId; - INSERT INTO DeletedResources VALUES (old.resourceType, old.publicId); - - -- If this resource is the latest child, delete the parent - DELETE FROM Resources WHERE internalId = old.parentId - AND NOT EXISTS (SELECT 1 FROM Resources WHERE parentId = old.parentId); - RETURN NULL; -END; -$body$ LANGUAGE plpgsql; - -DROP TRIGGER IF EXISTS ResourceDeleted on Resources; -CREATE TRIGGER ResourceDeleted -AFTER DELETE ON Resources -FOR EACH ROW -EXECUTE PROCEDURE ResourceDeletedFunc(); - - --- remove the new DeleteAttachment function - -DROP FUNCTION IF EXISTS DeleteAttachment; - ----------- - --- set the global properties that actually documents the DB version, revision and some of the capabilities --- modify only the ones that have changed -DELETE FROM GlobalProperties WHERE property IN (4, 11); -INSERT INTO GlobalProperties VALUES (4, 4); -- GlobalProperty_DatabasePatchLevel
--- a/PostgreSQL/Plugins/SQL/PrepareIndex.sql Wed Jun 04 15:28:35 2025 +0200 +++ b/PostgreSQL/Plugins/SQL/PrepareIndex.sql Thu Jun 26 22:31:58 2025 +0200 @@ -14,7 +14,8 @@ internalId BIGSERIAL NOT NULL PRIMARY KEY, resourceType INTEGER NOT NULL, publicId VARCHAR(64) NOT NULL, - parentId BIGINT REFERENCES Resources(internalId) ON DELETE CASCADE, + -- parentId BIGINT REFERENCES Resources(internalId) ON DELETE CASCADE, + parentId BIGINT REFERENCES Resources(internalId), childCount INTEGER, CONSTRAINT UniquePublicId UNIQUE (publicId) ); @@ -182,25 +183,78 @@ deleted_parent_row RECORD; deleted_grand_parent_row RECORD; deleted_grand_grand_parent_row RECORD; - locked_row RECORD; + + locked_parent_row RECORD; + locked_resource_row RECORD; BEGIN - -- note: temporary tables are now created at transaction level and are dropped on commit - CREATE TEMPORARY TABLE DeletedResources( + SET client_min_messages = warning; -- suppress NOTICE: relation "deletedresources" already exists, skipping + + -- note: temporary tables are created at connection level -> they are likely to exist. + -- These tables are used by the triggers + CREATE TEMPORARY TABLE IF NOT EXISTS DeletedResources( resourceType INTEGER NOT NULL, publicId VARCHAR(64) NOT NULL - ) ON COMMIT DROP; + ); + + RESET client_min_messages; - + -- clear the temporary table in case it has been created earlier in the connection + DELETE FROM DeletedResources; + -- create/clear the DeletedFiles temporary table PERFORM CreateDeletedFilesTemporaryTable(); + -- Before deleting an object, we need to lock its parent until the end of the transaction to avoid that -- 2 threads deletes the last 2 instances of a series at the same time -> none of them would realize -- that they are deleting the last instance and the parent resources would not be deleted. -- Locking only the immediate parent is sufficient to prevent from this. - SELECT * INTO locked_row FROM resources WHERE internalid = (SELECT parentid FROM resources WHERE internalid = id) FOR UPDATE; + SELECT * INTO locked_parent_row FROM resources WHERE internalid = (SELECT parentid FROM resources WHERE internalid = id) FOR UPDATE; + + -- Before deleting the resource itself, we lock it to retrieve the resourceType and to make sure not 2 connections try to + -- delete it at the same time + SELECT * INTO locked_resource_row FROM resources WHERE internalid = id FOR UPDATE; + + -- before delete the resource itself, we must delete its grand-grand-children, the grand-children and its children no to violate + -- the parentId referencing an existing primary key constrain. This is actually implementing the ON DELETE CASCADE that was on the parentId in previous revisions. + + -- If this resource has grand-grand-children, delete them + if locked_resource_row.resourceType < 1 THEN + WITH grand_grand_children_to_delete AS (SELECT grandGrandChildLevel.internalId, grandGrandChildLevel.resourceType, grandGrandChildLevel.publicId + FROM Resources childLevel + INNER JOIN Resources grandChildLevel ON childLevel.internalId = grandChildLevel.parentId + INNER JOIN Resources grandGrandChildLevel ON grandChildLevel.internalId = grandGrandChildLevel.parentId + WHERE childLevel.parentId = id), + + deleted_grand_grand_children_rows AS (DELETE FROM Resources WHERE internalId IN (SELECT internalId FROM grand_grand_children_to_delete) + RETURNING resourceType, publicId) + + INSERT INTO DeletedResources SELECT resourceType, publicId FROM deleted_grand_grand_children_rows; + END IF; + + -- If this resource has grand-children, delete them + if locked_resource_row.resourceType < 2 THEN + WITH grand_children_to_delete AS (SELECT grandChildLevel.internalId, grandChildLevel.resourceType, grandChildLevel.publicId + FROM Resources childLevel + INNER JOIN Resources grandChildLevel ON childLevel.internalId = grandChildLevel.parentId + WHERE childLevel.parentId = id), + + deleted_grand_children_rows AS (DELETE FROM Resources WHERE internalId IN (SELECT internalId FROM grand_children_to_delete) + RETURNING resourceType, publicId) + + INSERT INTO DeletedResources SELECT resourceType, publicId FROM deleted_grand_children_rows; + END IF; + + -- If this resource has children, delete them + if locked_resource_row.resourceType < 3 THEN + WITH deleted_children AS (DELETE FROM Resources + WHERE parentId = id + RETURNING resourceType, publicId) + INSERT INTO DeletedResources SELECT resourceType, publicId FROM deleted_children; + END IF; + -- delete the resource itself DELETE FROM Resources WHERE internalId=id RETURNING * INTO deleted_resource_row; @@ -244,6 +298,7 @@ END IF; END IF; END IF; + END; $body$ LANGUAGE plpgsql; @@ -253,8 +308,10 @@ BEGIN - -- note: temporary tables are now created at transaction level and are dropped on commit - CREATE TEMPORARY TABLE DeletedFiles( + SET client_min_messages = warning; -- suppress NOTICE: relation "DeletedFiles" already exists, skipping + + -- note: temporary tables created at connection level -> they are likely to exist + CREATE TEMPORARY TABLE IF NOT EXISTS DeletedFiles( uuid VARCHAR(64) NOT NULL, fileType INTEGER, compressedSize BIGINT, @@ -262,8 +319,12 @@ compressionType INTEGER, uncompressedHash VARCHAR(40), compressedHash VARCHAR(40) - ) ON COMMIT DROP; + ); + RESET client_min_messages; + + -- clear the temporary table in case it has been created earlier in the connection + DELETE FROM DeletedFiles; END; $body$ LANGUAGE plpgsql; @@ -747,7 +808,7 @@ -- set the global properties that actually documents the DB version, revision and some of the capabilities DELETE FROM GlobalProperties WHERE property IN (1, 4, 6, 10, 11, 12, 13, 14); INSERT INTO GlobalProperties VALUES (1, 6); -- GlobalProperty_DatabaseSchemaVersion -INSERT INTO GlobalProperties VALUES (4, 99); -- GlobalProperty_DatabasePatchLevel +INSERT INTO GlobalProperties VALUES (4, 499); -- GlobalProperty_DatabasePatchLevel INSERT INTO GlobalProperties VALUES (6, 1); -- GlobalProperty_GetTotalSizeIsFast INSERT INTO GlobalProperties VALUES (10, 1); -- GlobalProperty_HasTrigramIndex INSERT INTO GlobalProperties VALUES (11, 3); -- GlobalProperty_HasCreateInstance -- this is actually the 3rd version of HasCreateInstance
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/PostgreSQL/Plugins/SQL/Upgrades/Rev4ToRev499.sql Thu Jun 26 22:31:58 2025 +0200 @@ -0,0 +1,59 @@ +CREATE SEQUENCE IF NOT EXISTS PatientRecyclingOrderSequence INCREMENT 1 START 1; + +-- the protection mechanisms changed in rev 499. We now use a metadata (18: IsProtected) +-- while, in the past, patients where protected by not appearing in the PatientRecyclingOrder + +-- Step 1: Identify all patients that are not in PatientRecyclingOrder (those are the protected patients) +-- The "0" corresponds to "OrthancPluginResourceType_Patient" +WITH ProtectedPatients AS ( + SELECT r.internalId AS internalId + FROM Resources r + LEFT JOIN PatientRecyclingOrder pro ON r.internalId = pro.patientId + WHERE r.resourceType = 0 + AND pro.patientId IS NULL +) +, UnprotectedPatients AS ( + SELECT patientId AS internalId + FROM PatientRecyclingOrder + ORDER BY seq ASC +) + +-- Step 2: Prepare the data for both metadata types +, MetadataToInsert AS ( + -- mark protected patient in 18: IsProtected + SELECT internalId, 18 AS metadataType, 'true' AS metadataValue + FROM ProtectedPatients + + UNION ALL + + -- copy previous recycling order in 19: RecyclingOrder + SELECT internalId, 19 AS metadataType, nextval('PatientRecyclingOrderSequence')::TEXT AS metadataValue + FROM UnprotectedPatients +) + +-- Step 3: Insert the Metadata (since the metadata are new, there should not be any conflicts) +INSERT INTO Metadata (id, type, value, revision) +SELECT internalId, metadataType, metadataValue, 0 +FROM MetadataToInsert +ON CONFLICT (id, type) +DO UPDATE SET value = EXCLUDED.value, revision = EXCLUDED.revision; + +-- The PatientRecyclingOrder table can now be removed + +DROP TABLE PatientRecyclingOrder; + +DROP TRIGGER IF EXISTS PatientAdded on Resources; +DROP FUNCTION IF EXISTS PatientAddedFunc; +DROP FUNCTION IF EXISTS PatientAddedOrUpdated; + +-- The DeletedResources trigger is not used anymore + +DROP TRIGGER IF EXISTS ResourceDeleted ON Resources; +DROP FUNCTION IF EXISTS ResourceDeletedFunc(); + +-- The ON DELETE CASCADE on the Resources.parentId has been removed since this is now implemented +-- in the DeleteResource function +-- Drop the existing foreign key constraint and add a new one without ON DELETE CASCADE in a single command +ALTER TABLE Resources +DROP CONSTRAINT IF EXISTS resources_parentid_fkey, +ADD CONSTRAINT resources_parentid_fkey FOREIGN KEY (parentId) REFERENCES Resources(internalId); \ No newline at end of file
--- a/PostgreSQL/Plugins/SQL/Upgrades/Rev4ToRev99.sql Wed Jun 04 15:28:35 2025 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,52 +0,0 @@ -CREATE SEQUENCE IF NOT EXISTS PatientRecyclingOrderSequence INCREMENT 1 START 1; - --- the protection mechanisms changed in rev 99. We now use a metadata (18: IsProtected) --- while, in the past, patients where protected by not appearing in the PatientRecyclingOrder - --- Step 1: Identify all patients that are not in PatientRecyclingOrder (those are the protected patients) --- The "0" corresponds to "OrthancPluginResourceType_Patient" -WITH ProtectedPatients AS ( - SELECT r.internalId AS internalId - FROM Resources r - LEFT JOIN PatientRecyclingOrder pro ON r.internalId = pro.patientId - WHERE r.resourceType = 0 - AND pro.patientId IS NULL -) -, UnprotectedPatients AS ( - SELECT patientId AS internalId - FROM PatientRecyclingOrder - ORDER BY seq ASC -) - --- Step 2: Prepare the data for both metadata types -, MetadataToInsert AS ( - -- mark protected patient in 18: IsProtected - SELECT internalId, 18 AS metadataType, 'true' AS metadataValue - FROM ProtectedPatients - - UNION ALL - - -- copy previous recycling order in 19: RecyclingOrder - SELECT internalId, 19 AS metadataType, nextval('PatientRecyclingOrderSequence')::TEXT AS metadataValue - FROM UnprotectedPatients -) - --- Step 3: Insert the Metadata (since the metadata are new, there should not be any conflicts) -INSERT INTO Metadata (id, type, value, revision) -SELECT internalId, metadataType, metadataValue, 0 -FROM MetadataToInsert -ON CONFLICT (id, type) -DO UPDATE SET value = EXCLUDED.value, revision = EXCLUDED.revision; - --- The PatientRecyclingOrder table can now be removed - -DROP TABLE PatientRecyclingOrder; - -DROP TRIGGER IF EXISTS PatientAdded on Resources; -DROP FUNCTION IF EXISTS PatientAddedFunc; -DROP FUNCTION IF EXISTS PatientAddedOrUpdated; - --- The DeletedResources trigger is not used anymore - -DROP TRIGGER IF EXISTS ResourceDeleted ON Resources; -DROP FUNCTION IF EXISTS ResourceDeletedFunc(); \ No newline at end of file
--- a/Resources/CMake/DatabasesPluginConfiguration.cmake Wed Jun 04 15:28:35 2025 +0200 +++ b/Resources/CMake/DatabasesPluginConfiguration.cmake Thu Jun 26 22:31:58 2025 +0200 @@ -111,10 +111,12 @@ list(APPEND DATABASES_SOURCES ${ORTHANC_CORE_SOURCES} + ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/BaseIndexConnectionsPool.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/DatabaseBackendAdapterV2.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/DatabaseBackendAdapterV3.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/DatabaseBackendAdapterV4.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/DatabaseConstraint.cpp + ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/DynamicIndexConnectionsPool.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/ISqlLookupFormatter.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/IndexBackend.cpp ${ORTHANC_DATABASES_ROOT}/Framework/Plugins/IndexConnectionsPool.cpp