changeset 435:326f8304daa1 pg-transactions

new creating temporary tables inside functions
author Alain Mazy <am@osimis.io>
date Thu, 14 Dec 2023 09:25:45 +0100
parents 23c7af6f671a
children f16faa1fdc46
files Framework/Common/DatabaseManager.cpp Framework/Common/DatabaseManager.h Framework/Common/ResultBase.cpp Framework/PostgreSQL/PostgreSQLResult.cpp NOTES PostgreSQL/Plugins/PostgreSQLIndex.cpp PostgreSQL/Plugins/ResourceDeletedFunc.sql
diffstat 7 files changed, 92 insertions(+), 58 deletions(-) [+]
line wrap: on
line diff
--- 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) :
--- 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);
     };
 
 
--- 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();
--- 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);
     }
--- 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 ?
--- 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);
-
   }
 
 
--- 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;