changeset 428:4d0bacbd0fba pg-transactions

rewrote CreateInstance to make it compatible with READ COMMITED transactions
author Alain Mazy <am@osimis.io>
date Thu, 30 Nov 2023 14:46:38 +0100
parents 3cdea26ece73
children dbf811b1bb43
files PostgreSQL/Plugins/CreateInstance.sql PostgreSQL/Plugins/PostgreSQLIndex.cpp
diffstat 2 files changed, 47 insertions(+), 71 deletions(-) [+]
line wrap: on
line diff
--- 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;
 
--- 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<uint64_t>(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<uint64_t>(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<PostgreSQLDatabase&>(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<uint64_t>(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;
   }