changeset 431:7c1fe5d6c12c pg-transactions

PG: IncrementGlobalProperty
author Alain Mazy <am@osimis.io>
date Thu, 07 Dec 2023 12:13:43 +0100
parents f1f3c5554283
children 8b7c1c423367
files Framework/Plugins/DatabaseBackendAdapterV4.cpp Framework/Plugins/IDatabaseBackend.h Framework/Plugins/IndexBackend.cpp Framework/Plugins/IndexBackend.h NOTES PostgreSQL/Plugins/PostgreSQLIndex.cpp PostgreSQL/Plugins/PostgreSQLIndex.h PostgreSQL/Plugins/PrepareIndex.sql
diffstat 8 files changed, 225 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Plugins/DatabaseBackendAdapterV4.cpp	Thu Nov 30 14:50:09 2023 +0100
+++ b/Framework/Plugins/DatabaseBackendAdapterV4.cpp	Thu Dec 07 12:13:43 2023 +0100
@@ -431,6 +431,7 @@
         response.mutable_get_system_information()->set_supports_flush_to_disk(false);
         response.mutable_get_system_information()->set_supports_revisions(accessor.GetBackend().HasRevisionsSupport());
         response.mutable_get_system_information()->set_supports_labels(accessor.GetBackend().HasLabelsSupport());
+        response.mutable_get_system_information()->set_supports_increment_global_property(accessor.GetBackend().HasAtomicIncrementGlobalProperty());
         break;
       }
 
@@ -941,7 +942,16 @@
         
         break;
       }
-      
+
+      case Orthanc::DatabasePluginMessages::OPERATION_INCREMENT_GLOBAL_PROPERTY:
+      {
+        int64_t value = backend.IncrementGlobalProperty(manager, request.increment_global_property().server_id().c_str(),
+                                                        request.increment_global_property().property(),
+                                                        request.increment_global_property().increment());
+        response.mutable_increment_global_property()->set_new_value(value);
+        break;
+      }
+
       case Orthanc::DatabasePluginMessages::OPERATION_LOOKUP_METADATA:
       {
         std::string value;
--- a/Framework/Plugins/IDatabaseBackend.h	Thu Nov 30 14:50:09 2023 +0100
+++ b/Framework/Plugins/IDatabaseBackend.h	Thu Dec 07 12:13:43 2023 +0100
@@ -348,5 +348,12 @@
     // New in Orthanc 1.12.0
     virtual void ListAllLabels(std::list<std::string>& target,
                                DatabaseManager& manager) = 0;
+
+    virtual bool HasAtomicIncrementGlobalProperty() = 0;
+
+    virtual int64_t IncrementGlobalProperty(DatabaseManager& manager,
+                                            const char* serverIdentifier,
+                                            int32_t property,
+                                            int64_t increment) = 0;
   };
 }
--- a/Framework/Plugins/IndexBackend.cpp	Thu Nov 30 14:50:09 2023 +0100
+++ b/Framework/Plugins/IndexBackend.cpp	Thu Dec 07 12:13:43 2023 +0100
@@ -1204,6 +1204,18 @@
     }
   }
 
+  bool IndexBackend::HasAtomicIncrementGlobalProperty()
+  {
+    return false; // currently only implemented in Postgres
+  }
+
+  int64_t IndexBackend::IncrementGlobalProperty(DatabaseManager& manager,
+                                                const char* serverIdentifier,
+                                                int32_t property,
+                                                int64_t increment)
+  {
+    throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented);
+  }
     
   void IndexBackend::LookupIdentifier(std::list<int64_t>& target /*out*/,
                                       DatabaseManager& manager,
--- a/Framework/Plugins/IndexBackend.h	Thu Nov 30 14:50:09 2023 +0100
+++ b/Framework/Plugins/IndexBackend.h	Thu Dec 07 12:13:43 2023 +0100
@@ -396,6 +396,14 @@
     virtual void ListAllLabels(std::list<std::string>& target,
                                DatabaseManager& manager) ORTHANC_OVERRIDE;
     
+    virtual bool HasAtomicIncrementGlobalProperty() ORTHANC_OVERRIDE;
+
+    virtual int64_t IncrementGlobalProperty(DatabaseManager& manager,
+                                            const char* serverIdentifier,
+                                            int32_t property,
+                                            int64_t increment) ORTHANC_OVERRIDE;
+
+
     /**
      * "maxDatabaseRetries" is to handle
      * "OrthancPluginErrorCode_DatabaseCannotSerialize" if there is a
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/NOTES	Thu Dec 07 12:13:43 2023 +0100
@@ -0,0 +1,120 @@
+Resources:
+*********
+- PG transaction modes explained: https://www.postgresql.org/files/developer/concurrency.pdf
+- Isoolation level explained (PG + MySQL): https://amirsoleimani.medium.com/understanding-database-isolation-level-via-examples-mysql-and-postgres-a86b5502d404
+
+
+Sample SQL code that you can execute in DBeaver to test new functions/procedures:
+
+CreateInstance
+************************************************************************
+
+CREATE OR replace FUNCTION CreateInstance(
+  IN patient TEXT,
+  IN study TEXT,
+  IN series TEXT,
+  IN instance TEXT,
+  OUT isNewPatient BIGINT,
+  OUT isNewStudy BIGINT,
+  OUT isNewSeries BIGINT,
+  OUT isNewInstance BIGINT,
+  OUT patientKey BIGINT,
+  OUT studyKey BIGINT,
+  OUT seriesKey BIGINT,
+  OUT instanceKey BIGINT)
+AS $$ 
+begin
+	isNewPatient := 1;
+	isNewStudy := 1;
+	isNewSeries := 1;
+	isNewInstance := 1;
+
+	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;
+
+	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;
+
+	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;
+
+  	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;
+   
+END;
+$$ LANGUAGE plpgsql;
+
+DO $$ 
+DECLARE 
+    result record;
+begin
+	delete from "resources";
+
+    SELECT * INTO result from CreateInstance('patient1', 'study1', 'series1', 'instance1');
+
+    RAISE NOTICE 'Value patientInternalId: %, is_new: %', result.patientKey, result.isNewPatient;
+    RAISE NOTICE 'Value studyInternalId: %, is_new: %', result.studyKey, result.isNewStudy;
+    RAISE NOTICE 'Value seriesInternalId: %, is_new: %', result.seriesKey, result.isNewSeries;
+    RAISE NOTICE 'Value instanceInternalId: %, is_new: %', result.instanceKey, result.isNewInstance;
+    RAISE NOTICE '--------------';
+
+    SELECT * INTO result from CreateInstance('patient1', 'study1', 'series1', 'instance2');
+
+    RAISE NOTICE 'Value patientInternalId: %, is_new: %', result.patientKey, result.isNewPatient;
+    RAISE NOTICE 'Value studyInternalId: %, is_new: %', result.studyKey, result.isNewStudy;
+    RAISE NOTICE 'Value seriesInternalId: %, is_new: %', result.seriesKey, result.isNewSeries;
+    RAISE NOTICE 'Value instanceInternalId: %, is_new: %', result.instanceKey, result.isNewInstance;
+    RAISE NOTICE '--------------';
+
+    SELECT * INTO result from CreateInstance('patient1', 'study1', 'series1', 'instance2');
+
+    RAISE NOTICE 'Value patientInternalId: %, is_new: %', result.patientKey, result.isNewPatient;
+    RAISE NOTICE 'Value studyInternalId: %, is_new: %', result.studyKey, result.isNewStudy;
+    RAISE NOTICE 'Value seriesInternalId: %, is_new: %', result.seriesKey, result.isNewSeries;
+    RAISE NOTICE 'Value instanceInternalId: %, is_new: %', result.instanceKey, result.isNewInstance;
+    RAISE NOTICE '--------------';
+
+END $$;
+
+-- \set patient_key 'patient_key'
+-- SELECT CreateInstance('patient', 'study', 'series', 'instance', patient_key) ;
+
+-- drop function CreateInstance
+-- select * from "resources";
+-- delete from "resources";
+-- INSERT INTO "resources" VALUES (DEFAULT, 0, 'patient', NULL)
+
+
+
+************************************************************************
+
+In debug, no verbose logs
+Orthanc 1.12.1 + PG 5.1 (serializable mode)           : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 51.703 s
+Orthanc mainline + PG maineline (serializable mode)   : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 51.913 s
+Orthanc mainline + PG maineline (read-committed mode) : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 24.754 s
+
+Orthanc 1.12.1 + PG 5.1 (serializable mode)           : test_concurrent_uploads_same_study with 40 workers and 1x repeat: 55.932 s
+Orthanc mainline + PG maineline (serializable mode)   : fails: No new instance while overwriting; this should not happen
+Orthanc mainline + PG maineline (read-committed mode) : 
+
+TODO:
--- a/PostgreSQL/Plugins/PostgreSQLIndex.cpp	Thu Nov 30 14:50:09 2023 +0100
+++ b/PostgreSQL/Plugins/PostgreSQLIndex.cpp	Thu Dec 07 12:13:43 2023 +0100
@@ -398,6 +398,60 @@
     return result;
   }
 
+  int64_t PostgreSQLIndex::IncrementGlobalProperty(DatabaseManager& manager,
+                                                   const char* serverIdentifier,
+                                                   int32_t property,
+                                                   int64_t increment)
+  {
+    if (serverIdentifier == NULL)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
+    }
+    else
+    {
+      if (strlen(serverIdentifier) == 0)
+      {
+        DatabaseManager::CachedStatement statement(
+          STATEMENT_FROM_HERE, manager,
+          "INSERT INTO GlobalProperties (property, value) VALUES(${property}, ${increment}) "
+          "  ON CONFLICT (property) DO UPDATE SET value = CAST(GlobalProperties.value AS BIGINT) + ${increment}"
+          " RETURNING CAST(value AS BIGINT)");
+
+        statement.SetParameterType("property", ValueType_Integer64);
+        statement.SetParameterType("increment", ValueType_Integer64);
+
+        Dictionary args;
+        args.SetIntegerValue("property", property);
+        args.SetIntegerValue("increment", increment);
+        
+        statement.Execute(args);
+
+        return statement.ReadInteger64(0);
+      }
+      else
+      {
+        DatabaseManager::CachedStatement statement(
+          STATEMENT_FROM_HERE, manager,
+          "INSERT INTO ServerProperties (server, property, value) VALUES(${server}, ${property}, ${increment}) "
+          "  ON CONFLICT (server, property) DO UPDATE SET value = CAST(ServerProperties.value AS BIGINT) + ${increment}"
+          " RETURNING CAST(value AS BIGINT)");
+
+        statement.SetParameterType("server", ValueType_Utf8String);
+        statement.SetParameterType("property", ValueType_Integer64);
+        statement.SetParameterType("increment", ValueType_Integer64);
+
+        Dictionary args;
+        args.SetUtf8Value("server", serverIdentifier);
+        args.SetIntegerValue("property", property);
+        args.SetIntegerValue("increment", increment);
+        
+        statement.Execute(args);
+
+        return statement.ReadInteger64(0);
+      }
+    }
+  }
+
 
 #if ORTHANC_PLUGINS_HAS_DATABASE_CONSTRAINT == 1
   void PostgreSQLIndex::CreateInstance(OrthancPluginCreateInstanceResult& result,
--- a/PostgreSQL/Plugins/PostgreSQLIndex.h	Thu Nov 30 14:50:09 2023 +0100
+++ b/PostgreSQL/Plugins/PostgreSQLIndex.h	Thu Dec 07 12:13:43 2023 +0100
@@ -90,5 +90,16 @@
     {
       return true;
     }
+
+    virtual bool HasAtomicIncrementGlobalProperty() ORTHANC_OVERRIDE
+    {
+      return true;
+    }
+
+    virtual int64_t IncrementGlobalProperty(DatabaseManager& manager,
+                                            const char* serverIdentifier,
+                                            int32_t property,
+                                            int64_t increment) ORTHANC_OVERRIDE;
+
   };
 }
--- a/PostgreSQL/Plugins/PrepareIndex.sql	Thu Nov 30 14:50:09 2023 +0100
+++ b/PostgreSQL/Plugins/PrepareIndex.sql	Thu Dec 07 12:13:43 2023 +0100
@@ -134,7 +134,8 @@
   
   -- http://stackoverflow.com/a/11299968/881731
   IF EXISTS (SELECT 1 FROM Resources WHERE parentId = old.parentId) THEN
-    -- Signal that the deleted resource has a remaining parent
+    -- Signal that the deleted resource has a remaining parent 
+    -- (a parent that must not be deleted but whose LastUpdate must be updated)
     INSERT INTO RemainingAncestor
       SELECT resourceType, publicId FROM Resources WHERE internalId = old.parentId;
   ELSE