# HG changeset patch # User Alain Mazy # Date 1702542345 -3600 # Node ID 326f8304daa1abce8136deac265a67a25afd18d6 # Parent 23c7af6f671a0cbb8b8b54c6ff4bc3e0bfccfdaf new creating temporary tables inside functions diff -r 23c7af6f671a -r 326f8304daa1 Framework/Common/DatabaseManager.cpp --- a/Framework/Common/DatabaseManager.cpp Wed Dec 13 16:52:06 2023 +0100 +++ b/Framework/Common/DatabaseManager.cpp Thu Dec 14 09:25:45 2023 +0100 @@ -571,8 +571,7 @@ } } - - void DatabaseManager::CachedStatement::Execute(const Dictionary& parameters) + void DatabaseManager::CachedStatement::ExecuteInternal(const Dictionary& parameters, bool withResults) { try { @@ -598,7 +597,14 @@ #endif */ - SetResult(GetTransaction().Execute(*statement_, parameters)); + if (withResults) + { + SetResult(GetTransaction().Execute(*statement_, parameters)); + } + else + { + GetTransaction().ExecuteWithoutResult(*statement_, parameters); + } } catch (Orthanc::OrthancException& e) { @@ -606,7 +612,18 @@ throw; } } + + + void DatabaseManager::CachedStatement::Execute(const Dictionary& parameters) + { + ExecuteInternal(parameters, true); + } + void DatabaseManager::CachedStatement::ExecuteWithoutResult(const Dictionary& parameters) + { + ExecuteInternal(parameters, false); + } + DatabaseManager::StandaloneStatement::StandaloneStatement(DatabaseManager& manager, const std::string& sql) : diff -r 23c7af6f671a -r 326f8304daa1 Framework/Common/DatabaseManager.h --- a/Framework/Common/DatabaseManager.h Wed Dec 13 16:52:06 2023 +0100 +++ b/Framework/Common/DatabaseManager.h Thu Dec 14 09:25:45 2023 +0100 @@ -221,6 +221,17 @@ } void Execute(const Dictionary& parameters); + + void ExecuteWithoutResult() + { + Dictionary parameters; + ExecuteWithoutResult(parameters); + } + + void ExecuteWithoutResult(const Dictionary& parameters); + + private: + void ExecuteInternal(const Dictionary& parameters, bool withResults); }; diff -r 23c7af6f671a -r 326f8304daa1 Framework/Common/ResultBase.cpp --- a/Framework/Common/ResultBase.cpp Wed Dec 13 16:52:06 2023 +0100 +++ b/Framework/Common/ResultBase.cpp Thu Dec 14 09:25:45 2023 +0100 @@ -56,11 +56,6 @@ for (size_t i = 0; i < fields_.size(); i++) { - if (fields_[i] == NULL) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer); - } - ValueType sourceType = fields_[i]->GetType(); ValueType targetType = expectedType_[i]; @@ -95,11 +90,11 @@ for (size_t i = 0; i < fields_.size(); i++) { fields_[i] = FetchField(i); + } - if (fields_[i] == NULL) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer); - } + if (fields_.size() == 1 && fields_[0] == NULL) // this is a "void" result + { + return; } ConvertFields(); diff -r 23c7af6f671a -r 326f8304daa1 Framework/PostgreSQL/PostgreSQLResult.cpp --- a/Framework/PostgreSQL/PostgreSQLResult.cpp Wed Dec 13 16:52:06 2023 +0100 +++ b/Framework/PostgreSQL/PostgreSQLResult.cpp Thu Dec 14 09:25:45 2023 +0100 @@ -260,6 +260,9 @@ case OIDOID: return new LargeObjectResult(database_, GetLargeObjectOid(column)); + case VOIDOID: + return NULL; + default: throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented); } diff -r 23c7af6f671a -r 326f8304daa1 NOTES --- a/NOTES Wed Dec 13 16:52:06 2023 +0100 +++ b/NOTES Thu Dec 14 09:25:45 2023 +0100 @@ -171,17 +171,17 @@ Orthanc 1.12.1 + PG 5.1 (serializable mode) : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 43.957 s Orthanc mainline + PG mainline (serializable mode) : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: XX s Orthanc mainline + PG mainline (read-committed mode) : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 19.861 s -Orthanc mainline + PG mainline (read-committed mode) : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 22.500 s (with temporary tables) - test_concurrent_anonymize_same_study deletion took: 32.265 s - +Orthanc mainline + PG mainline (read-committed mode) : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 20.027 s (with temporary tables) + test_concurrent_anonymize_same_study deletion took: 28.4 s + Orthanc 1.12.1 + PG 5.1 (serializable mode) : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 21.214 s Orthanc mainline + PG mainline (serializable mode) : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 23.010 s Orthanc mainline + PG mainline (read-committed mode) : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 1.71 s (with RemainingAncestor.id) -Orthanc mainline + PG mainline (read-committed mode) : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 3.26 s (with temporary tables) +Orthanc mainline + PG mainline (read-committed mode) : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 2.97 s (with temporary tables) Orthanc 1.12.1 + PG 5.1 (serializable mode) : test_upload_delete_same_study_from_multiple_threads with 5 workers and 30x repeat: 23.016 s Orthanc mainline + PG mainline (read-committed mode) : test_upload_delete_same_study_from_multiple_threads with 5 workers and 30x repeat: 8.788 s -Orthanc mainline + PG mainline (read-committed mode) : test_upload_delete_same_study_from_multiple_threads with 5 workers and 30x repeat: 17.129 s (with temporary tables) +Orthanc mainline + PG mainline (read-committed mode) : test_upload_delete_same_study_from_multiple_threads with 5 workers and 30x repeat: 11.872 s (with temporary tables) TODO: - assert((statement.Next(), statement.IsDone())) commented out -> create temporary tables ? diff -r 23c7af6f671a -r 326f8304daa1 PostgreSQL/Plugins/PostgreSQLIndex.cpp --- a/PostgreSQL/Plugins/PostgreSQLIndex.cpp Wed Dec 13 16:52:06 2023 +0100 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.cpp Thu Dec 14 09:25:45 2023 +0100 @@ -470,26 +470,11 @@ { // 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 DeletedFiles(" - "uuid VARCHAR(64) NOT NULL," - "fileType INTEGER, " - "compressedSize BIGINT, " - "uncompressedSize BIGINT, " - "compressionType INTEGER, " - "uncompressedHash VARCHAR(40)," - "compressedHash VARCHAR(40)" - ");" + "SELECT CreateDeletedFilesTemporaryTable()" ); - statement.Execute(); + statement.ExecuteWithoutResult(); } - { - DatabaseManager::CachedStatement statement( - STATEMENT_FROM_HERE, manager, - "DELETE FROM DeletedFiles;" - ); - statement.Execute(); - } } void PostgreSQLIndex::ClearDeletedResources(DatabaseManager& manager) @@ -517,35 +502,13 @@ void PostgreSQLIndex::ClearRemainingAncestor(DatabaseManager& manager) { - { // 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 RemainingAncestor(" - "resourceType INTEGER NOT NULL," - "publicId VARCHAR(64) NOT NULL" - ")" - ); - - statement.Execute(); - } - { - DatabaseManager::CachedStatement statement( - STATEMENT_FROM_HERE, manager, - "DELETE FROM RemainingAncestor;" - ); - - statement.Execute(); - } - } void PostgreSQLIndex::DeleteResource(IDatabaseBackendOutput& output, DatabaseManager& manager, int64_t id) { - ClearDeletedFiles(manager); - ClearDeletedResources(manager); - + // clearing of temporary table is now implemented in the funcion DeleteResource DatabaseManager::CachedStatement statement( STATEMENT_FROM_HERE, manager, "SELECT * FROM DeleteResource(${id})"); @@ -575,7 +538,6 @@ SignalDeletedFiles(output, manager); SignalDeletedResources(output, manager); - } diff -r 23c7af6f671a -r 326f8304daa1 PostgreSQL/Plugins/ResourceDeletedFunc.sql --- a/PostgreSQL/Plugins/ResourceDeletedFunc.sql Wed Dec 13 16:52:06 2023 +0100 +++ b/PostgreSQL/Plugins/ResourceDeletedFunc.sql Thu Dec 14 09:25:45 2023 +0100 @@ -37,6 +37,25 @@ 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(); + + + -- 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 + @@ -50,3 +69,30 @@ 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;