Mercurial > hg > orthanc
changeset 5497:4dd50c4b985a pg-transactions
merge default -> pg-transactions
line wrap: on
line diff
--- a/NEWS Tue Jan 23 17:04:31 2024 +0100 +++ b/NEWS Tue Jan 23 17:05:28 2024 +0100 @@ -1,6 +1,15 @@ Pending changes in the mainline =============================== +General +------- + +* Performance: + - Databases: + - At startup, when using a database plugin, display the latency to access the DB. + - Added support for new DB primitives to enable the "READ COMMITTED" transaction mode + in the PostgreSQL plugin. + REST API --------
--- a/OrthancFramework/Resources/CodeGeneration/ErrorCodes.json Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancFramework/Resources/CodeGeneration/ErrorCodes.json Tue Jan 23 17:05:28 2024 +0100 @@ -256,6 +256,12 @@ "Name": "ForbiddenAccess", "Description": "Access to a resource is forbidden" }, + { + "Code": 46, + "HttpStatus": 409, + "Name": "DuplicateResource", + "Description": "Duplicate resource" + },
--- a/OrthancFramework/Sources/Enumerations.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancFramework/Sources/Enumerations.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -182,6 +182,9 @@ case ErrorCode_ForbiddenAccess: return "Access to a resource is forbidden"; + case ErrorCode_DuplicateResource: + return "Duplicate resource"; + case ErrorCode_SQLiteNotOpened: return "SQLite: The database is not opened"; @@ -2264,6 +2267,9 @@ case ErrorCode_ForbiddenAccess: return HttpStatus_403_Forbidden; + case ErrorCode_DuplicateResource: + return HttpStatus_409_Conflict; + case ErrorCode_CreateDicomNotString: return HttpStatus_400_BadRequest;
--- a/OrthancFramework/Sources/Enumerations.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancFramework/Sources/Enumerations.h Tue Jan 23 17:05:28 2024 +0100 @@ -170,6 +170,7 @@ ErrorCode_Revision = 43 /*!< A bad revision number was provided, which might indicate conflict between multiple writers */, ErrorCode_MainDicomTagsMultiplyDefined = 44 /*!< A main DICOM Tag has been defined multiple times for the same resource level */, ErrorCode_ForbiddenAccess = 45 /*!< Access to a resource is forbidden */, + ErrorCode_DuplicateResource = 46 /*!< Duplicate resource */, ErrorCode_SQLiteNotOpened = 1000 /*!< SQLite: The database is not opened */, ErrorCode_SQLiteAlreadyOpened = 1001 /*!< SQLite: Connection is already open */, ErrorCode_SQLiteCannotOpen = 1002 /*!< SQLite: Unable to open the database */,
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -43,7 +43,7 @@ namespace Orthanc { class OrthancPluginDatabase::Transaction : - public IDatabaseWrapper::ITransaction, + public BaseDatabaseWrapper::BaseTransaction, public Compatibility::ICreateInstance, public Compatibility::IGetChildrenMetadata, public Compatibility::ILookupResources, @@ -243,6 +243,11 @@ that_.activeTransaction_ = NULL; } + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE + { + return that_.GetDatabaseCapabilities(); + } + IDatabaseListener& GetDatabaseListener() const { return listener_; @@ -1472,7 +1477,8 @@ payload_(payload), activeTransaction_(NULL), fastGetTotalSize_(false), - currentDiskSize_(0) + currentDiskSize_(0), + dbCapabilities_(false, false, false, false, false, false) { static const char* const MISSING = " Missing extension in database index plugin: ";
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabase.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabase.h Tue Jan 23 17:05:28 2024 +0100 @@ -25,7 +25,7 @@ #if ORTHANC_ENABLE_PLUGINS == 1 #include "../../../OrthancFramework/Sources/SharedLibrary.h" -#include "../../Sources/Database/IDatabaseWrapper.h" +#include "../../Sources/Database/BaseDatabaseWrapper.h" #include "../Include/orthanc/OrthancCDatabasePlugin.h" #include "PluginsErrorDictionary.h" @@ -45,7 +45,7 @@ * able to rollback the modifications. Read-only accesses didn't * start a transaction, as they were protected by the global mutex. **/ - class OrthancPluginDatabase : public IDatabaseWrapper + class OrthancPluginDatabase : public BaseDatabaseWrapper { private: class Transaction; @@ -65,6 +65,7 @@ Transaction* activeTransaction_; bool fastGetTotalSize_; uint64_t currentDiskSize_; + IDatabaseWrapper::Capabilities dbCapabilities_; OrthancPluginDatabaseContext* GetContext() { @@ -94,11 +95,6 @@ { } - virtual bool HasFlushToDisk() const ORTHANC_OVERRIDE - { - return false; - } - virtual IDatabaseWrapper::ITransaction* StartTransaction(TransactionType type, IDatabaseListener& listener) ORTHANC_OVERRIDE; @@ -108,14 +104,14 @@ virtual void Upgrade(unsigned int targetVersion, IStorageArea& storageArea) ORTHANC_OVERRIDE; - virtual bool HasRevisionsSupport() const ORTHANC_OVERRIDE + const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE { - return false; // No support for revisions in old API + return dbCapabilities_; } - virtual bool HasLabelsSupport() const ORTHANC_OVERRIDE + virtual uint64_t MeasureLatency() ORTHANC_OVERRIDE { - return false; + throw OrthancException(ErrorCode_NotImplemented); // only implemented in V4 } void AnswerReceived(const _OrthancPluginDatabaseAnswer& answer);
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -45,7 +45,7 @@ namespace Orthanc { - class OrthancPluginDatabaseV3::Transaction : public IDatabaseWrapper::ITransaction + class OrthancPluginDatabaseV3::Transaction : public BaseDatabaseWrapper::BaseTransaction { private: OrthancPluginDatabaseV3& that_; @@ -278,6 +278,10 @@ } } + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE + { + return that_.GetDatabaseCapabilities(); + } virtual void Rollback() ORTHANC_OVERRIDE { @@ -1083,7 +1087,14 @@ library_(library), errorDictionary_(errorDictionary), database_(database), - serverIdentifier_(serverIdentifier) + serverIdentifier_(serverIdentifier), + dbCapabilities_(false, /* hasFlushToDisk */ + false, /* revision support is updated in open() */ + false, /* hasLabelsSupport */ + false, /* hasAtomicIncrementGlobalProperty */ + false, /* hasUpdateAndGetStatistics */ + false /* hasMeasureLatency */) + { CLOG(INFO, PLUGINS) << "Identifier of this Orthanc server for the global properties " << "of the custom database: \"" << serverIdentifier << "\""; @@ -1190,6 +1201,11 @@ void OrthancPluginDatabaseV3::Open() { CheckSuccess(backend_.open(database_)); + + // update the db capabilities + uint8_t hasRevisions; + CheckSuccess(backend_.hasRevisionsSupport(database_, &hasRevisions)); + dbCapabilities_.hasRevisionsSupport_ = (hasRevisions != 0); } @@ -1250,12 +1266,4 @@ } } - - bool OrthancPluginDatabaseV3::HasRevisionsSupport() const - { - // WARNING: This method requires "Open()" to have been called - uint8_t hasRevisions; - CheckSuccess(backend_.hasRevisionsSupport(database_, &hasRevisions)); - return (hasRevisions != 0); - } }
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.h Tue Jan 23 17:05:28 2024 +0100 @@ -25,13 +25,13 @@ #if ORTHANC_ENABLE_PLUGINS == 1 #include "../../../OrthancFramework/Sources/SharedLibrary.h" -#include "../../Sources/Database/IDatabaseWrapper.h" +#include "../../Sources/Database/BaseDatabaseWrapper.h" #include "../Include/orthanc/OrthancCDatabasePlugin.h" #include "PluginsErrorDictionary.h" namespace Orthanc { - class OrthancPluginDatabaseV3 : public IDatabaseWrapper + class OrthancPluginDatabaseV3 : public BaseDatabaseWrapper { private: class Transaction; @@ -41,6 +41,7 @@ OrthancPluginDatabaseBackendV3 backend_; void* database_; std::string serverIdentifier_; + IDatabaseWrapper::Capabilities dbCapabilities_; void CheckSuccess(OrthancPluginErrorCode code) const; @@ -67,11 +68,6 @@ { } - virtual bool HasFlushToDisk() const ORTHANC_OVERRIDE - { - return false; - } - virtual IDatabaseWrapper::ITransaction* StartTransaction(TransactionType type, IDatabaseListener& listener) ORTHANC_OVERRIDE; @@ -81,12 +77,16 @@ virtual void Upgrade(unsigned int targetVersion, IStorageArea& storageArea) ORTHANC_OVERRIDE; - virtual bool HasRevisionsSupport() const ORTHANC_OVERRIDE; + virtual uint64_t MeasureLatency() ORTHANC_OVERRIDE + { + throw OrthancException(ErrorCode_NotImplemented); // only implemented in V4 + } - virtual bool HasLabelsSupport() const ORTHANC_OVERRIDE + const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE { - return false; + return dbCapabilities_; } + }; }
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -229,7 +229,7 @@ bool isSingleResource, int64_t resource) { - if (database_.HasLabelsSupport()) + if (database_.GetDatabaseCapabilities().HasLabelsSupport()) { DatabasePluginMessages::TransactionRequest request; request.mutable_list_labels()->set_single_resource(isSingleResource); @@ -305,6 +305,10 @@ } } + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE + { + return database_.GetDatabaseCapabilities(); + } void* GetTransactionObject() { @@ -772,7 +776,40 @@ } } - + + virtual int64_t IncrementGlobalProperty(GlobalProperty property, + int64_t increment, + bool shared) ORTHANC_OVERRIDE + { + DatabasePluginMessages::TransactionRequest request; + request.mutable_increment_global_property()->set_server_id(shared ? "" : database_.GetServerIdentifier()); + request.mutable_increment_global_property()->set_property(property); + request.mutable_increment_global_property()->set_increment(increment); + + DatabasePluginMessages::TransactionResponse response; + ExecuteTransaction(response, DatabasePluginMessages::OPERATION_INCREMENT_GLOBAL_PROPERTY, request); + + return response.increment_global_property().new_value(); + } + + virtual void UpdateAndGetStatistics(int64_t& patientsCount, + int64_t& studiesCount, + int64_t& seriesCount, + int64_t& instancesCount, + int64_t& compressedSize, + int64_t& uncompressedSize) ORTHANC_OVERRIDE + { + DatabasePluginMessages::TransactionResponse response; + ExecuteTransaction(response, DatabasePluginMessages::OPERATION_UPDATE_AND_GET_STATISTICS); + + patientsCount = response.update_and_get_statistics().patients_count(); + studiesCount = response.update_and_get_statistics().studies_count(); + seriesCount = response.update_and_get_statistics().series_count(); + instancesCount = response.update_and_get_statistics().instances_count(); + compressedSize = response.update_and_get_statistics().total_compressed_size(); + uncompressedSize = response.update_and_get_statistics().total_uncompressed_size(); + } + virtual bool LookupMetadata(std::string& target, int64_t& revision, int64_t id, @@ -948,7 +985,7 @@ LabelsConstraint labelsConstraint, uint32_t limit) ORTHANC_OVERRIDE { - if (!database_.HasLabelsSupport() && + if (!database_.GetDatabaseCapabilities().HasLabelsSupport() && !labels.empty()) { throw OrthancException(ErrorCode_InternalError); @@ -1197,7 +1234,7 @@ virtual void AddLabel(int64_t resource, const std::string& label) ORTHANC_OVERRIDE { - if (database_.HasLabelsSupport()) + if (database_.GetDatabaseCapabilities().HasLabelsSupport()) { DatabasePluginMessages::TransactionRequest request; request.mutable_add_label()->set_id(resource); @@ -1216,7 +1253,7 @@ virtual void RemoveLabel(int64_t resource, const std::string& label) ORTHANC_OVERRIDE { - if (database_.HasLabelsSupport()) + if (database_.GetDatabaseCapabilities().HasLabelsSupport()) { DatabasePluginMessages::TransactionRequest request; request.mutable_remove_label()->set_id(resource); @@ -1256,9 +1293,7 @@ serverIdentifier_(serverIdentifier), open_(false), databaseVersion_(0), - hasFlushToDisk_(false), - hasRevisionsSupport_(false), - hasLabelsSupport_(false) + dbCapabilities_(false, false, false, false, false, false) // updated in Open() { CLOG(INFO, PLUGINS) << "Identifier of this Orthanc server for the global properties " << "of the custom database: \"" << serverIdentifier << "\""; @@ -1325,10 +1360,15 @@ DatabasePluginMessages::DatabaseRequest request; DatabasePluginMessages::DatabaseResponse response; ExecuteDatabase(response, *this, DatabasePluginMessages::OPERATION_GET_SYSTEM_INFORMATION, request); - databaseVersion_ = response.get_system_information().database_version(); - hasFlushToDisk_ = response.get_system_information().supports_flush_to_disk(); - hasRevisionsSupport_ = response.get_system_information().supports_revisions(); - hasLabelsSupport_ = response.get_system_information().supports_labels(); + + const ::Orthanc::DatabasePluginMessages::GetSystemInformation_Response& systemInfo = response.get_system_information(); + databaseVersion_ = systemInfo.database_version(); + dbCapabilities_.hasFlushToDisk_ = systemInfo.supports_flush_to_disk(); + dbCapabilities_.hasRevisionsSupport_ = systemInfo.supports_revisions(); + dbCapabilities_.hasLabelsSupport_ = systemInfo.supports_labels(); + dbCapabilities_.hasAtomicIncrementGlobalProperty_ = systemInfo.supports_increment_global_property(); + dbCapabilities_.hasUpdateAndGetStatistics_ = systemInfo.has_update_and_get_statistics(); + dbCapabilities_.hasMeasureLatency_ = systemInfo.has_measure_latency(); } open_ = true; @@ -1350,23 +1390,11 @@ } - bool OrthancPluginDatabaseV4::HasFlushToDisk() const - { - if (!open_) - { - throw OrthancException(ErrorCode_BadSequenceOfCalls); - } - else - { - return hasFlushToDisk_; - } - } - void OrthancPluginDatabaseV4::FlushToDisk() { if (!open_ || - !hasFlushToDisk_) + !GetDatabaseCapabilities().HasFlushToDisk()) { throw OrthancException(ErrorCode_BadSequenceOfCalls); } @@ -1438,8 +1466,8 @@ } } - - bool OrthancPluginDatabaseV4::HasRevisionsSupport() const + + uint64_t OrthancPluginDatabaseV4::MeasureLatency() { if (!open_) { @@ -1447,12 +1475,23 @@ } else { - return hasRevisionsSupport_; + try + { + DatabasePluginMessages::DatabaseRequest request; + DatabasePluginMessages::DatabaseResponse response; + + ExecuteDatabase(response, *this, DatabasePluginMessages::OPERATION_MEASURE_LATENCY, request); + return response.measure_latency().latency_us(); + + } + catch (OrthancException& e) + { + throw; + } } } - - bool OrthancPluginDatabaseV4::HasLabelsSupport() const + const IDatabaseWrapper::Capabilities& OrthancPluginDatabaseV4::GetDatabaseCapabilities() const { if (!open_) { @@ -1460,7 +1499,9 @@ } else { - return hasLabelsSupport_; + return dbCapabilities_; } } + + }
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.h Tue Jan 23 17:05:28 2024 +0100 @@ -42,9 +42,7 @@ std::string serverIdentifier_; bool open_; unsigned int databaseVersion_; - bool hasFlushToDisk_; - bool hasRevisionsSupport_; - bool hasLabelsSupport_; + IDatabaseWrapper::Capabilities dbCapabilities_; void CheckSuccess(OrthancPluginErrorCode code) const; @@ -82,8 +80,6 @@ virtual void FlushToDisk() ORTHANC_OVERRIDE; - virtual bool HasFlushToDisk() const ORTHANC_OVERRIDE; - virtual IDatabaseWrapper::ITransaction* StartTransaction(TransactionType type, IDatabaseListener& listener) ORTHANC_OVERRIDE; @@ -93,9 +89,9 @@ virtual void Upgrade(unsigned int targetVersion, IStorageArea& storageArea) ORTHANC_OVERRIDE; - virtual bool HasRevisionsSupport() const ORTHANC_OVERRIDE; + virtual uint64_t MeasureLatency() ORTHANC_OVERRIDE; - virtual bool HasLabelsSupport() const ORTHANC_OVERRIDE; + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE; }; }
--- a/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h Tue Jan 23 17:05:28 2024 +0100 @@ -246,6 +246,7 @@ OrthancPluginErrorCode_Revision = 43 /*!< A bad revision number was provided, which might indicate conflict between multiple writers */, OrthancPluginErrorCode_MainDicomTagsMultiplyDefined = 44 /*!< A main DICOM Tag has been defined multiple times for the same resource level */, OrthancPluginErrorCode_ForbiddenAccess = 45 /*!< Access to a resource is forbidden */, + OrthancPluginErrorCode_DuplicateResource = 46 /*!< Duplicate resource */, OrthancPluginErrorCode_SQLiteNotOpened = 1000 /*!< SQLite: The database is not opened */, OrthancPluginErrorCode_SQLiteAlreadyOpened = 1001 /*!< SQLite: Connection is already open */, OrthancPluginErrorCode_SQLiteCannotOpen = 1002 /*!< SQLite: Unable to open the database */,
--- a/OrthancServer/Plugins/Include/orthanc/OrthancDatabasePlugin.proto Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Plugins/Include/orthanc/OrthancDatabasePlugin.proto Tue Jan 23 17:05:28 2024 +0100 @@ -121,6 +121,7 @@ OPERATION_START_TRANSACTION = 4; OPERATION_UPGRADE = 5; OPERATION_FINALIZE_TRANSACTION = 6; + OPERATION_MEASURE_LATENCY = 7; // New in Orthanc 1.12.X } enum TransactionType { @@ -136,6 +137,9 @@ bool supports_flush_to_disk = 2; bool supports_revisions = 3; bool supports_labels = 4; + bool supports_increment_global_property = 5; + bool has_update_and_get_statistics = 6; + bool has_measure_latency = 7; } } @@ -198,6 +202,15 @@ } } +message MeasureLatency { + message Request { + } + message Response { + int64 latency_us = 1; + } +} + + message DatabaseRequest { sfixed64 database = 1; DatabaseOperation operation = 2; @@ -208,7 +221,8 @@ FlushToDisk.Request flush_to_disk = 103; StartTransaction.Request start_transaction = 104; Upgrade.Request upgrade = 105; - FinalizeTransaction.Request finalize_transaction = 106; + FinalizeTransaction.Request finalize_transaction = 106; + MeasureLatency.Request measure_latency = 107; } message DatabaseResponse { @@ -219,6 +233,7 @@ StartTransaction.Response start_transaction = 104; Upgrade.Response upgrade = 105; FinalizeTransaction.Response finalize_transaction = 106; + MeasureLatency.Response measure_latency = 107; } @@ -275,6 +290,8 @@ OPERATION_ADD_LABEL = 45; // New in Orthanc 1.12.0 OPERATION_REMOVE_LABEL = 46; // New in Orthanc 1.12.0 OPERATION_LIST_LABELS = 47; // New in Orthanc 1.12.0 + OPERATION_INCREMENT_GLOBAL_PROPERTY = 48; // New in Orthanc 1.12.X + OPERATION_UPDATE_AND_GET_STATISTICS = 49; // New in Orthanc 1.12.X } message Rollback { @@ -628,6 +645,30 @@ } } +message IncrementGlobalProperty { + message Request { + string server_id = 1; + int32 property = 2; + int64 increment = 3; + } + message Response { + int64 new_value = 1; + } +} + +message UpdateAndGetStatistics { + message Request { + } + message Response { + int64 patients_count = 1; + int64 studies_count = 2; + int64 series_count = 3; + int64 instances_count = 4; + int64 total_compressed_size = 5; + int64 total_uncompressed_size = 6; + } +} + message ClearMainDicomTags { message Request { int64 id = 1; @@ -834,6 +875,8 @@ AddLabel.Request add_label = 145; RemoveLabel.Request remove_label = 146; ListLabels.Request list_labels = 147; + IncrementGlobalProperty.Request increment_global_property = 148; + UpdateAndGetStatistics.Request update_and_get_statistics = 149; } message TransactionResponse { @@ -885,6 +928,8 @@ AddLabel.Response add_label = 145; RemoveLabel.Response remove_label = 146; ListLabels.Response list_labels = 147; + IncrementGlobalProperty.Response increment_global_property = 148; + UpdateAndGetStatistics.Response update_and_get_statistics = 149; } enum RequestType {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/OrthancServer/Sources/Database/BaseDatabaseWrapper.h Tue Jan 23 17:05:28 2024 +0100 @@ -0,0 +1,59 @@ +/** + * 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) 2021-2023 Sebastien Jodogne, ICTEAM UCLouvain, Belgium + * + * This program is free software: you can redistribute it and/or + * modify it under the terms of the GNU 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 + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + **/ + + +#pragma once + +#include "IDatabaseWrapper.h" +#include "../../../OrthancFramework/Sources/OrthancException.h" + +namespace Orthanc +{ + /** + * This class provides a default "not implemented" implementation + * for all recent methods (1.12.X) + **/ + class BaseDatabaseWrapper : public IDatabaseWrapper + { + public: + class BaseTransaction : public IDatabaseWrapper::ITransaction + { + virtual int64_t IncrementGlobalProperty(GlobalProperty property, + int64_t increment, + bool shared) ORTHANC_OVERRIDE + { + throw OrthancException(ErrorCode_NotImplemented); // Not supported + } + + virtual void UpdateAndGetStatistics(int64_t& patientsCount, + int64_t& studiesCount, + int64_t& seriesCount, + int64_t& instancesCount, + int64_t& compressedSize, + int64_t& uncompressedSize) ORTHANC_OVERRIDE + { + throw OrthancException(ErrorCode_NotImplemented); // Not supported + } + + }; + + }; +} \ No newline at end of file
--- a/OrthancServer/Sources/Database/IDatabaseWrapper.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/Database/IDatabaseWrapper.h Tue Jan 23 17:05:28 2024 +0100 @@ -39,10 +39,74 @@ class DatabaseConstraint; class ResourcesContent; + class OrthancPluginDatabaseV3; + class OrthancPluginDatabaseV4; class IDatabaseWrapper : public boost::noncopyable { public: + + struct Capabilities + { + friend OrthancPluginDatabaseV3; + friend OrthancPluginDatabaseV4; + + protected: + bool hasFlushToDisk_; + bool hasRevisionsSupport_; + bool hasLabelsSupport_; + bool hasAtomicIncrementGlobalProperty_; + bool hasUpdateAndGetStatistics_; + bool hasMeasureLatency_; + + public: + Capabilities(bool hasFlushToDisk, + bool hasRevisionsSupport, + bool hasLabelsSupport, + bool hasAtomicIncrementGlobalProperty, + bool hasUpdateAndGetStatistics, + bool hasMeasureLatency) + : hasFlushToDisk_(hasFlushToDisk), + hasRevisionsSupport_(hasRevisionsSupport), + hasLabelsSupport_(hasLabelsSupport), + hasAtomicIncrementGlobalProperty_(hasAtomicIncrementGlobalProperty), + hasUpdateAndGetStatistics_(hasUpdateAndGetStatistics), + hasMeasureLatency_(hasMeasureLatency) + { + } + + bool HasFlushToDisk() const + { + return hasFlushToDisk_; + } + + bool HasRevisionsSupport() const + { + return hasRevisionsSupport_; + } + + bool HasLabelsSupport() const + { + return hasLabelsSupport_; + } + + bool HasAtomicIncrementGlobalProperty() const + { + return hasAtomicIncrementGlobalProperty_; + } + + bool HasUpdateAndGetStatistics() const + { + return hasUpdateAndGetStatistics_; + } + + bool HasMeasureLatency() const + { + return hasMeasureLatency_; + } + + }; + struct CreateInstanceResult : public boost::noncopyable { bool isNewPatient_; @@ -257,6 +321,19 @@ // List all the labels that are present in any resource virtual void ListAllLabels(std::set<std::string>& target) = 0; + + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const = 0; + + virtual int64_t IncrementGlobalProperty(GlobalProperty property, + int64_t increment, + bool shared) = 0; + + virtual void UpdateAndGetStatistics(int64_t& patientsCount, + int64_t& studiesCount, + int64_t& seriesCount, + int64_t& instancesCount, + int64_t& compressedSize, + int64_t& uncompressedSize) = 0; }; @@ -270,8 +347,6 @@ virtual void FlushToDisk() = 0; - virtual bool HasFlushToDisk() const = 0; - virtual ITransaction* StartTransaction(TransactionType type, IDatabaseListener& listener) = 0; @@ -280,8 +355,8 @@ virtual void Upgrade(unsigned int targetVersion, IStorageArea& storageArea) = 0; - virtual bool HasRevisionsSupport() const = 0; + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const = 0; - virtual bool HasLabelsSupport() const = 0; + virtual uint64_t MeasureLatency() = 0; }; }
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -300,19 +300,27 @@ boost::mutex::scoped_lock lock_; IDatabaseListener& listener_; SignalRemainingAncestor& signalRemainingAncestor_; + const IDatabaseWrapper::Capabilities& dbCapabilities_; public: TransactionBase(boost::mutex& mutex, SQLite::Connection& db, IDatabaseListener& listener, - SignalRemainingAncestor& signalRemainingAncestor) : + SignalRemainingAncestor& signalRemainingAncestor, + const IDatabaseWrapper::Capabilities& dbCapabilities) : UnitTestsTransaction(db), lock_(mutex), listener_(listener), - signalRemainingAncestor_(signalRemainingAncestor) + signalRemainingAncestor_(signalRemainingAncestor), + dbCapabilities_(dbCapabilities) { } + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE + { + return dbCapabilities_; + } + IDatabaseListener& GetListener() const { return listener_; @@ -1137,6 +1145,7 @@ target.insert(s.ColumnString(0)); } } + }; @@ -1234,7 +1243,7 @@ public: ReadWriteTransaction(SQLiteDatabaseWrapper& that, IDatabaseListener& listener) : - TransactionBase(that.mutex_, that.db_, listener, *that.signalRemainingAncestor_), + TransactionBase(that.mutex_, that.db_, listener, *that.signalRemainingAncestor_, that.GetDatabaseCapabilities()), that_(that), transaction_(new SQLite::Transaction(that_.db_)) { @@ -1288,7 +1297,7 @@ public: ReadOnlyTransaction(SQLiteDatabaseWrapper& that, IDatabaseListener& listener) : - TransactionBase(that.mutex_, that.db_, listener, *that.signalRemainingAncestor_), + TransactionBase(that.mutex_, that.db_, listener, *that.signalRemainingAncestor_, that.GetDatabaseCapabilities()), that_(that) { if (that_.activeTransaction_ != NULL) @@ -1322,7 +1331,13 @@ SQLiteDatabaseWrapper::SQLiteDatabaseWrapper(const std::string& path) : activeTransaction_(NULL), signalRemainingAncestor_(NULL), - version_(0) + version_(0), + dbCapabilities_(true, /* hasFlushToDisk */ + false, /* hasRevisionsSupport TODO: implement revisions in SQLite */ + true, /* hasLabelsSupport */ + false, /* hasAtomicIncrementGlobalProperty */ + false, /* hasUpdateAndGetStatistics */ + false /* hasMeasureLatency */) { db_.Open(path); } @@ -1331,7 +1346,13 @@ SQLiteDatabaseWrapper::SQLiteDatabaseWrapper() : activeTransaction_(NULL), signalRemainingAncestor_(NULL), - version_(0) + version_(0), + dbCapabilities_(true, /* hasFlushToDisk */ + false, /* hasRevisionsSupport TODO: implement revisions in SQLite */ + true, /* hasLabelsSupport */ + false, /* hasAtomicIncrementGlobalProperty */ + false, /* hasUpdateAndGetStatistics */ + false /* hasMeasureLatency */) { db_.OpenInMemory(); }
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h Tue Jan 23 17:05:28 2024 +0100 @@ -22,7 +22,7 @@ #pragma once -#include "IDatabaseWrapper.h" +#include "BaseDatabaseWrapper.h" #include "../../../OrthancFramework/Sources/SQLite/Connection.h" @@ -35,7 +35,7 @@ * translates low-level requests into SQL statements. Mutual * exclusion MUST be implemented at a higher level. **/ - class SQLiteDatabaseWrapper : public IDatabaseWrapper + class SQLiteDatabaseWrapper : public BaseDatabaseWrapper { private: class TransactionBase; @@ -51,6 +51,7 @@ TransactionBase* activeTransaction_; SignalRemainingAncestor* signalRemainingAncestor_; unsigned int version_; + IDatabaseWrapper::Capabilities dbCapabilities_; void GetChangesInternal(std::list<ServerIndexChange>& target, bool& done, @@ -79,11 +80,6 @@ virtual void FlushToDisk() ORTHANC_OVERRIDE; - virtual bool HasFlushToDisk() const ORTHANC_OVERRIDE - { - return true; - } - virtual unsigned int GetDatabaseVersion() ORTHANC_OVERRIDE { return version_; @@ -92,24 +88,23 @@ virtual void Upgrade(unsigned int targetVersion, IStorageArea& storageArea) ORTHANC_OVERRIDE; - virtual bool HasRevisionsSupport() const ORTHANC_OVERRIDE + virtual const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const ORTHANC_OVERRIDE { - return false; // TODO - REVISIONS + return dbCapabilities_; } - virtual bool HasLabelsSupport() const ORTHANC_OVERRIDE + virtual uint64_t MeasureLatency() ORTHANC_OVERRIDE { - return true; + throw OrthancException(ErrorCode_NotImplemented); } - /** * The "StartTransaction()" method is guaranteed to return a class * derived from "UnitTestsTransaction". The methods of * "UnitTestsTransaction" give access to additional information * about the underlying SQLite database to be used in unit tests. **/ - class UnitTestsTransaction : public ITransaction + class UnitTestsTransaction : public BaseDatabaseWrapper::BaseTransaction { protected: SQLite::Connection& db_;
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -607,7 +607,7 @@ Transaction transaction(db_, *factory_, TransactionType_ReadOnly); // TODO - Only if not "TransactionType_Implicit" { - ReadOnlyTransaction t(transaction.GetDatabaseTransaction(), transaction.GetContext(), db_.HasLabelsSupport()); + ReadOnlyTransaction t(transaction.GetDatabaseTransaction(), transaction.GetContext(), db_.GetDatabaseCapabilities()); readOperations->Apply(t); } transaction.Commit(); @@ -618,7 +618,7 @@ Transaction transaction(db_, *factory_, TransactionType_ReadWrite); { - ReadWriteTransaction t(transaction.GetDatabaseTransaction(), transaction.GetContext(), db_.HasLabelsSupport()); + ReadWriteTransaction t(transaction.GetDatabaseTransaction(), transaction.GetContext(), db_.GetDatabaseCapabilities()); writeOperations->Apply(t); } transaction.Commit(); @@ -632,6 +632,7 @@ { if (attempt >= maxRetries_) { + LOG(ERROR) << "Maximum transactions retries reached " << e.GetDetails(); throw; } else @@ -654,7 +655,6 @@ StatelessDatabaseOperations::StatelessDatabaseOperations(IDatabaseWrapper& db) : db_(db), mainDicomTagsRegistry_(new MainDicomTagsRegistry), - hasFlushToDisk_(db.HasFlushToDisk()), maxRetries_(0) { } @@ -939,7 +939,7 @@ } if ((expandFlags & ExpandResourceFlags_IncludeLabels) && - transaction.HasLabelsSupport()) + transaction.GetDatabaseCapabilities().HasLabelsSupport()) { transaction.ListLabels(target.labels_, internalId); } @@ -1103,7 +1103,54 @@ /* out */ uint64_t& countSeries, /* out */ uint64_t& countInstances) { - class Operations : public ReadOnlyOperationsT6<uint64_t&, uint64_t&, uint64_t&, uint64_t&, uint64_t&, uint64_t&> + // new code that updates and gets all statistics. + // I.e, PostgreSQL now store "changes" to apply to the statistics to prevent row locking + // of the GlobalIntegers table while multiple clients are inserting/deleting new resources. + // Then, the statistics are updated when requested to make sure they are correct. + class Operations : public IReadWriteOperations + { + private: + int64_t diskSize_; + int64_t uncompressedSize_; + int64_t countPatients_; + int64_t countStudies_; + int64_t countSeries_; + int64_t countInstances_; + + public: + Operations() : + diskSize_(0), + uncompressedSize_(0), + countPatients_(0), + countStudies_(0), + countSeries_(0), + countInstances_(0) + { + } + + void GetValues(uint64_t& diskSize, + uint64_t& uncompressedSize, + uint64_t& countPatients, + uint64_t& countStudies, + uint64_t& countSeries, + uint64_t& countInstances) const + { + diskSize = static_cast<uint64_t>(diskSize_); + uncompressedSize = static_cast<uint64_t>(uncompressedSize_); + countPatients = static_cast<uint64_t>(countPatients_); + countStudies = static_cast<uint64_t>(countStudies_); + countSeries = static_cast<uint64_t>(countSeries_); + countInstances = static_cast<uint64_t>(countInstances_); + } + + virtual void Apply(ReadWriteTransaction& transaction) ORTHANC_OVERRIDE + { + transaction.UpdateAndGetStatistics(countPatients_, countStudies_, countSeries_, countInstances_, diskSize_, uncompressedSize_); + } + }; + + // legacy oprations that reads each entry individualy + class LegacyOperations : public ReadOnlyOperationsT6<uint64_t&, uint64_t&, uint64_t&, uint64_t&, uint64_t&, uint64_t&> { public: virtual void ApplyTuple(ReadOnlyTransaction& transaction, @@ -1117,10 +1164,20 @@ tuple.get<5>() = transaction.GetResourcesCount(ResourceType_Instance); } }; - - Operations operations; - operations.Apply(*this, diskSize, uncompressedSize, countPatients, - countStudies, countSeries, countInstances); + + if (GetDatabaseCapabilities().HasUpdateAndGetStatistics()) + { + Operations operations; + Apply(operations); + + operations.GetValues(diskSize, uncompressedSize, countPatients, countStudies, countSeries, countInstances); + } + else + { + LegacyOperations operations; + operations.Apply(*this, diskSize, uncompressedSize, countPatients, + countStudies, countSeries, countInstances); + } } @@ -1961,7 +2018,7 @@ }; if (!labels.empty() && - !db_.HasLabelsSupport()) + !db_.GetDatabaseCapabilities().HasLabelsSupport()) { throw OrthancException(ErrorCode_NotImplemented, "The database backend doesn't support labels"); } @@ -2431,32 +2488,39 @@ virtual void Apply(ReadWriteTransaction& transaction) ORTHANC_OVERRIDE { - std::string oldString; - - if (transaction.LookupGlobalProperty(oldString, sequence_, shared_)) + if (transaction.GetDatabaseCapabilities().HasAtomicIncrementGlobalProperty()) { - uint64_t oldValue; - - try - { - oldValue = boost::lexical_cast<uint64_t>(oldString); - } - catch (boost::bad_lexical_cast&) - { - LOG(ERROR) << "Cannot read the global sequence " - << boost::lexical_cast<std::string>(sequence_) << ", resetting it"; - oldValue = 0; - } - - newValue_ = oldValue + 1; + newValue_ = static_cast<uint64_t>(transaction.IncrementGlobalProperty(sequence_, shared_, 1)); } else { - // Initialize the sequence at "1" - newValue_ = 1; + std::string oldString; + + if (transaction.LookupGlobalProperty(oldString, sequence_, shared_)) + { + uint64_t oldValue; + + try + { + oldValue = boost::lexical_cast<uint64_t>(oldString); + } + catch (boost::bad_lexical_cast&) + { + LOG(ERROR) << "Cannot read the global sequence " + << boost::lexical_cast<std::string>(sequence_) << ", resetting it"; + oldValue = 0; + } + + newValue_ = oldValue + 1; + } + else + { + // Initialize the sequence at "1" + newValue_ = 1; + } + + transaction.SetGlobalProperty(sequence_, shared_, boost::lexical_cast<std::string>(newValue_)); } - - transaction.SetGlobalProperty(sequence_, shared_, boost::lexical_cast<std::string>(newValue_)); } }; @@ -3147,7 +3211,13 @@ if (!transaction.CreateInstance(status, instanceId, hashPatient_, hashStudy_, hashSeries_, hashInstance_)) { - throw OrthancException(ErrorCode_InternalError, "No new instance while overwriting; this should not happen."); + // Note that, sometime, it does not create a new instance, + // in very rare occasions in READ COMMITTED mode when multiple clients are pushing the same instance at the same time, + // this thread will not create the instance because another thread has created it in the meantime. + // At the end, there is always a thread that creates the instance and this is what we expect. + + // Note, we must delete the attachments that have already been stored from this failed insertion (they have not yet been added into the DB) + throw OrthancException(ErrorCode_DuplicateResource, "No new instance while overwriting; this might happen if another client has pushed the same instance at the same time."); } } else @@ -3694,6 +3764,6 @@ bool StatelessDatabaseOperations::HasLabelsSupport() { boost::shared_lock<boost::shared_mutex> lock(mutex_); - return db_.HasLabelsSupport(); + return db_.GetDatabaseCapabilities().HasLabelsSupport(); } }
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.h Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.h Tue Jan 23 17:05:28 2024 +0100 @@ -176,17 +176,17 @@ { private: ITransactionContext& context_; - bool hasLabelsSupport_; - + const IDatabaseWrapper::Capabilities& dbCapabilities_; + protected: IDatabaseWrapper::ITransaction& transaction_; public: explicit ReadOnlyTransaction(IDatabaseWrapper::ITransaction& transaction, ITransactionContext& context, - bool hasLabelsSupport) : + const IDatabaseWrapper::Capabilities& dbCapabilities) : context_(context), - hasLabelsSupport_(hasLabelsSupport), + dbCapabilities_(dbCapabilities), transaction_(transaction) { } @@ -196,9 +196,9 @@ return context_; } - bool HasLabelsSupport() const + const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const { - return hasLabelsSupport_; + return dbCapabilities_; } /** @@ -392,8 +392,8 @@ public: ReadWriteTransaction(IDatabaseWrapper::ITransaction& transaction, ITransactionContext& context, - bool hasLabelsSupport) : - ReadOnlyTransaction(transaction, context, hasLabelsSupport) + const IDatabaseWrapper::Capabilities& dbCapabilities) : + ReadOnlyTransaction(transaction, context, dbCapabilities) { } @@ -463,6 +463,23 @@ transaction_.SetGlobalProperty(property, shared, value); } + int64_t IncrementGlobalProperty(GlobalProperty sequence, + bool shared, + int64_t increment) + { + return transaction_.IncrementGlobalProperty(sequence, shared, increment); + } + + void UpdateAndGetStatistics(int64_t& patientsCount, + int64_t& studiesCount, + int64_t& seriesCount, + int64_t& instancesCount, + int64_t& compressedSize, + int64_t& uncompressedSize) + { + return transaction_.UpdateAndGetStatistics(patientsCount, studiesCount, seriesCount, instancesCount, compressedSize, uncompressedSize); + } + void SetMetadata(int64_t id, MetadataType type, const std::string& value, @@ -540,7 +557,6 @@ IDatabaseWrapper& db_; boost::shared_ptr<MainDicomTagsRegistry> mainDicomTagsRegistry_; // "shared_ptr" because of PImpl - bool hasFlushToDisk_; // Mutex to protect the configuration options boost::shared_mutex mutex_; @@ -575,12 +591,13 @@ return db_.GetDatabaseVersion(); } + const IDatabaseWrapper::Capabilities& GetDatabaseCapabilities() const + { + return db_.GetDatabaseCapabilities(); + } + void FlushToDisk(); - bool HasFlushToDisk() const - { - return hasFlushToDisk_; - } void Apply(IReadOnlyOperations& operations);
--- a/OrthancServer/Sources/ServerContext.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/ServerContext.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -710,9 +710,29 @@ typedef std::map<MetadataType, std::string> InstanceMetadata; InstanceMetadata instanceMetadata; - result.SetStatus(index_.Store( - instanceMetadata, summary, attachments, dicom.GetMetadata(), dicom.GetOrigin(), overwrite, - hasTransferSyntax, transferSyntax, hasPixelDataOffset, pixelDataOffset, pixelDataVR, isReconstruct)); + + try + { + result.SetStatus(index_.Store( + instanceMetadata, summary, attachments, dicom.GetMetadata(), dicom.GetOrigin(), overwrite, + hasTransferSyntax, transferSyntax, hasPixelDataOffset, pixelDataOffset, pixelDataVR, isReconstruct)); + } + catch (OrthancException& ex) + { + if (ex.GetErrorCode() == ErrorCode_DuplicateResource) + { + LOG(WARNING) << "Duplicate instance, deleting the attachments"; + + accessor.Remove(dicomInfo); + + if (dicomUntilPixelData.IsValid()) + { + accessor.Remove(dicomUntilPixelData); + } + + throw; + } + } // Only keep the metadata for the "instance" level dicom.ClearMetadata();
--- a/OrthancServer/Sources/ServerIndex.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/ServerIndex.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -331,7 +331,7 @@ // execution of Orthanc StandaloneRecycling(maximumStorageMode_, maximumStorageSize_, maximumPatients_); - if (HasFlushToDisk()) + if (GetDatabaseCapabilities().HasFlushToDisk()) { flushThread_ = boost::thread(FlushThread, this, threadSleepGranularityMilliseconds); }
--- a/OrthancServer/Sources/main.cpp Tue Jan 23 17:04:31 2024 +0100 +++ b/OrthancServer/Sources/main.cpp Tue Jan 23 17:05:28 2024 +0100 @@ -820,6 +820,7 @@ PrintErrorCode(ErrorCode_Revision, "A bad revision number was provided, which might indicate conflict between multiple writers"); PrintErrorCode(ErrorCode_MainDicomTagsMultiplyDefined, "A main DICOM Tag has been defined multiple times for the same resource level"); PrintErrorCode(ErrorCode_ForbiddenAccess, "Access to a resource is forbidden"); + PrintErrorCode(ErrorCode_DuplicateResource, "Duplicate resource"); PrintErrorCode(ErrorCode_SQLiteNotOpened, "SQLite: The database is not opened"); PrintErrorCode(ErrorCode_SQLiteAlreadyOpened, "SQLite: Connection is already open"); PrintErrorCode(ErrorCode_SQLiteCannotOpen, "SQLite: Unable to open the database"); @@ -1643,7 +1644,7 @@ if (lock.GetConfiguration().GetBooleanParameter(CHECK_REVISIONS, false)) { - if (database.HasRevisionsSupport()) + if (database.GetDatabaseCapabilities().HasRevisionsSupport()) { LOG(INFO) << "Handling of revisions is enabled, and the custom database back-end *has* " << "support for revisions of metadata and attachments"; @@ -1666,11 +1667,18 @@ } } - if (!database.HasLabelsSupport()) + if (!database.GetDatabaseCapabilities().HasLabelsSupport()) { LOG(WARNING) << "The custom database back-end has *no* support for labels"; } + if (database.GetDatabaseCapabilities().HasMeasureLatency()) + { + uint64_t latency = database.MeasureLatency(); + LOG(WARNING) << "The DB latency is " << latency << " µs"; + } + + bool success = ConfigureServerContext(database, storageArea, plugins, loadJobsFromDatabase); database.Close();