# HG changeset patch # User Alain Mazy # Date 1701351998 -3600 # Node ID 4d0bacbd0fbaa1af99ccdcb53e549d8b25cd773e # Parent 3cdea26ece7399ed2390c22cf96cc8eea8f90755 rewrote CreateInstance to make it compatible with READ COMMITED transactions diff -r 3cdea26ece73 -r 4d0bacbd0fba PostgreSQL/Plugins/CreateInstance.sql --- a/PostgreSQL/Plugins/CreateInstance.sql Wed Nov 29 10:24:18 2023 +0100 +++ b/PostgreSQL/Plugins/CreateInstance.sql Thu Nov 30 14:46:38 2023 +0100 @@ -1,4 +1,4 @@ -CREATE FUNCTION CreateInstance( +CREATE OR REPLACE FUNCTION CreateInstance( IN patient TEXT, IN study TEXT, IN series TEXT, @@ -17,67 +17,44 @@ countRecycling BIGINT; BEGIN - SELECT internalId FROM Resources INTO instanceKey WHERE publicId = instance AND resourceType = 3; - - IF NOT (instanceKey IS NULL) THEN - -- This instance already exists, stop here - isNewInstance := 0; - ELSE - SELECT internalId FROM Resources INTO patientKey WHERE publicId = patient AND resourceType = 0; - SELECT internalId FROM Resources INTO studyKey WHERE publicId = study AND resourceType = 1; - SELECT internalId FROM Resources INTO seriesKey WHERE publicId = series AND resourceType = 2; + isNewPatient := 1; + isNewStudy := 1; + isNewSeries := 1; + isNewInstance := 1; - IF patientKey IS NULL THEN - -- Must create a new patient - IF NOT (studyKey IS NULL AND seriesKey IS NULL AND instanceKey IS NULL) THEN - RAISE EXCEPTION 'Broken invariant'; - END IF; - - INSERT INTO Resources VALUES (DEFAULT, 0, patient, NULL) RETURNING internalId INTO patientKey; - isNewPatient := 1; - ELSE - isNewPatient := 0; - END IF; - - IF (patientKey IS NULL) THEN - RAISE EXCEPTION 'Broken invariant'; - END IF; + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 0, patient, NULL); + exception + when unique_violation then + isNewPatient := 0; + end; + select internalid into patientKey from "resources" where publicId=patient and resourcetype = 0; - IF studyKey IS NULL THEN - -- Must create a new study - IF NOT (seriesKey IS NULL AND instanceKey IS NULL) THEN - RAISE EXCEPTION 'Broken invariant'; - END IF; - - INSERT INTO Resources VALUES (DEFAULT, 1, study, patientKey) RETURNING internalId INTO studyKey; - isNewStudy := 1; - ELSE - isNewStudy := 0; - END IF; - - IF (studyKey IS NULL) THEN - RAISE EXCEPTION 'Broken invariant'; - END IF; + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 1, study, patientKey); + exception + when unique_violation then + isNewStudy := 0; + end; + select internalid into studyKey from "resources" where publicId=study and resourcetype = 1; - IF seriesKey IS NULL THEN - -- Must create a new series - IF NOT (instanceKey IS NULL) THEN - RAISE EXCEPTION 'Broken invariant'; - END IF; + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 2, series, studyKey); + exception + when unique_violation then + isNewSeries := 0; + end; + select internalid into seriesKey from "resources" where publicId=series and resourcetype = 2; - INSERT INTO Resources VALUES (DEFAULT, 2, series, studyKey) RETURNING internalId INTO seriesKey; - isNewSeries := 1; - ELSE - isNewSeries := 0; - END IF; - - IF (seriesKey IS NULL OR NOT instanceKey IS NULL) THEN - RAISE EXCEPTION 'Broken invariant'; - END IF; + BEGIN + INSERT INTO "resources" VALUES (DEFAULT, 3, instance, seriesKey); + exception + when unique_violation then + isNewInstance := 0; + end; + select internalid into instanceKey from "resources" where publicId=instance and resourcetype = 3; - INSERT INTO Resources VALUES (DEFAULT, 3, instance, seriesKey) RETURNING internalId INTO instanceKey; - isNewInstance := 1; - + IF isNewInstance > 0 THEN -- Move the patient to the end of the recycling order SELECT seq FROM PatientRecyclingOrder WHERE patientId = patientKey INTO patientSeq; diff -r 3cdea26ece73 -r 4d0bacbd0fba PostgreSQL/Plugins/PostgreSQLIndex.cpp --- a/PostgreSQL/Plugins/PostgreSQLIndex.cpp Wed Nov 29 10:24:18 2023 +0100 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.cpp Thu Nov 30 14:46:38 2023 +0100 @@ -204,7 +204,7 @@ int property = 0; if (!LookupGlobalIntegerProperty(property, manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_HasCreateInstance) || - property != 2) + property != 3) { LOG(INFO) << "Installing the CreateInstance extension"; @@ -215,12 +215,16 @@ "IN patient TEXT, IN study TEXT, IN series TEXT, in instance TEXT)"); } + // property == 2 was a first version of the CreateInstance -> we need to replace it by the new one + // property == 3 is a new version (in v5.2) with same signature and CREATE OR UPDATE + // -> we can replace the previous one without deleting it + // and we can create it if it has never been created. std::string query; Orthanc::EmbeddedResources::GetFileResource (query, Orthanc::EmbeddedResources::POSTGRESQL_CREATE_INSTANCE); t.GetDatabaseTransaction().ExecuteMultiLines(query); - SetGlobalIntegerProperty(manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_HasCreateInstance, 2); + SetGlobalIntegerProperty(manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_HasCreateInstance, 3); } @@ -367,7 +371,8 @@ result = static_cast(statement.ReadInteger64(0)); } - assert(result == IndexBackend::GetTotalCompressedSize(manager)); + // disabled because this is not alway true while transactions are being executed in READ COMITTED TRANSACTION. This is however true when no files are being delete/added + //assert(result == IndexBackend::GetTotalCompressedSize(manager)); return result; } @@ -388,7 +393,8 @@ result = static_cast(statement.ReadInteger64(0)); } - assert(result == IndexBackend::GetTotalUncompressedSize(manager)); + // disabled because this is not alway true while transactions are being executed in READ COMITTED TRANSACTION. This is however true when no files are being delete/added + // assert(result == IndexBackend::GetTotalUncompressedSize(manager)); return result; } @@ -416,16 +422,7 @@ args.SetUtf8Value("series", hashSeries); args.SetUtf8Value("instance", hashInstance); - { // The CreateInstance procedure is not 100% safe in highly concurrent environments when the - // transaction isolation is set to "READ COMMITED": (e.g, with 10 clients - // anonymizing studies in parallel with the "ResourceModification" config set to 8, we have observed - // the same series being created twice). Therefore, we protect the whole CreateInstance procedure - // with an advisory lock - PostgreSQLDatabase& db = dynamic_cast(manager.GetDatabase()); - PostgreSQLDatabase::TransientAdvisoryLock lock(db, POSTGRESQL_LOCK_CREATE_INSTANCE, 100, 1); - - statement.Execute(args); - } + statement.Execute(args); if (statement.IsDone() || statement.GetResultFieldsCount() != 8) @@ -484,7 +481,9 @@ result = static_cast(statement.ReadInteger64(0)); } + // disabled because this is not alway true while transactions are being executed in READ COMITTED TRANSACTION. This is however true when no files are being delete/added assert(result == IndexBackend::GetResourcesCount(manager, resourceType)); + return result; }