diff NOTES @ 469:302f3c2b1c34

merge pg-transactions -> mainline
author Alain Mazy <am@osimis.io>
date Mon, 05 Feb 2024 09:48:11 +0100
parents 0a8b34e3a337
children bf4b9c7cf338
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/NOTES	Mon Feb 05 09:48:11 2024 +0100
@@ -0,0 +1,230 @@
+Resources:
+*********
+- PG transaction modes explained: https://www.postgresql.org/files/developer/concurrency.pdf
+- Isolation level explained (PG + MySQL): https://amirsoleimani.medium.com/understanding-database-isolation-level-via-examples-mysql-and-postgres-a86b5502d404
+- Message queuing in PG: https://www.crunchydata.com/blog/message-queuing-using-native-postgresql
+
+
+Create and delete instances Internals:
+*************************************
+
+isNewInstance = CreateInstance(...)
+
+if (!isNewInstance && overwriteInstances)
+  DeleteResource(instance)
+       -> ClearDeletedFiles(manager);
+            DELETE FROM DeletedFiles  ------> this is not a temporary table in postgres but it is supposed to be empty before and after each transaction -> it is empty when taking a snapshot of the DB in READ COMMITTED mode!!!
+          ClearDeletedResources(manager);
+            DELETE FROM DeletedResources  ------> this is not a temporary table in postgres but it is supposed to be empty before and after each transaction !!!
+
+            DELETE FROM RemainingAncestor  ------> this is not a temporary table in postgres but it is supposed to be empty before and after each transaction !!!
+            DELETE FROM Resources WHERE internalId=..
+               -> cascades delete the MainDicomTags, the Metadata and the AttachedFiles
+                  -> this triggers AttachedFileDeletedFunc
+                         INSERT INTO DeletedFiles VALUES
+                            (old.uuid, old.filetype, old.compressedSize,
+                            old.uncompressedSize, old.compressionType,
+                            old.uncompressedHash, old.compressedHash);
+                        RETURN NULL;
+               -> this triggers a SQL trigger: ResourceDeletedFunc
+                        INSERT INTO DeletedResources VALUES (old.resourceType, old.publicId);
+                        IF EXISTS (SELECT 1 FROM Resources WHERE parentId = old.parentId) THEN
+                            -- 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
+                            -- Delete a parent resource when its unique child is deleted 
+                            DELETE FROM Resources WHERE internalId = old.parentId;
+                        END IF;
+
+            SELECT * FROM RemainingAncestor
+              -> SignalRemainingAncestor()  // There is at most 1 remaining ancestor
+                 -> ServerIndex::TransactionContext::SignalRemainingAncestor()
+                    -> stores remainingType and remainingPublicId (this is used in StatelessDatabaseOperations::DeleteResource to build the Rest Response of /delete 
+                                                                   and to update the LastUpdate of all parent (only when deleted from /delete))
+
+          SignalDeletedFiles(output, manager);
+            SELECT * FROM DeletedFiles
+              -> SignalDeletedAttachment()
+                 -> ServerIndex::TransactionContext::SignalAttachmentDeleted()
+                    -> pendingFilesToRemove_.push_back(FileToRemove(info))  (files are deleted in CommitFilesToRemove in the ServerIndex::TransactionContext::Commit)
+
+          SignalDeletedResources(output, manager);
+            SELECT resourceType, publicId FROM DeletedResources
+              -> SignalDeletedResource()
+                 -> Emit DeletedResource event (lua)
+
+
+  if (!CreateInstance(...))
+    Error: "No new instance while overwriting; this should not happen"
+
+if isNewInstance -> LogChange
+if isNewSeries -> LogChange
+....
+
+Sample SQL code that you can execute in DBeaver to test new functions/procedures:
+
+CreateInstance
+************************************************************************
+
+CREATE OR REPLACE FUNCTION CreateInstance(
+  IN patient_public_id TEXT,
+  IN study_public_id TEXT,
+  IN series_public_id TEXT,
+  IN instance_public_id TEXT,
+  OUT is_new_patient BIGINT,
+  OUT is_new_study BIGINT,
+  OUT is_new_series BIGINT,
+  OUT is_new_instance BIGINT,
+  OUT patient_internal_id BIGINT,
+  OUT study_internal_id BIGINT,
+  OUT series_internal_id BIGINT,
+  OUT instance_internal_id BIGINT) AS $body$
+
+BEGIN
+	is_new_patient := 1;
+	is_new_study := 1;
+	is_new_series := 1;
+	is_new_instance := 1;
+
+	
+	BEGIN
+        INSERT INTO "resources" VALUES (DEFAULT, 0, patient_public_id, NULL);
+    EXCEPTION
+        WHEN unique_violation THEN
+            is_new_patient := 0;
+    END;
+    SELECT internalid INTO patient_internal_id FROM "resources" WHERE publicId = patient_public_id AND resourcetype = 0 FOR UPDATE;  -- also locks the resource and its parent to prevent from deletion while we complete this transaction
+
+	BEGIN
+        INSERT INTO "resources" VALUES (DEFAULT, 1, study_public_id, patient_internal_id);
+    EXCEPTION
+        WHEN unique_violation THEN
+            is_new_study := 0;
+    END;
+    SELECT internalid INTO study_internal_id FROM "resources" WHERE publicId = study_public_id AND resourcetype = 1 FOR UPDATE;  -- also locks the resource and its parent to prevent from deletion while we complete this transaction
+
+	BEGIN
+	    INSERT INTO "resources" VALUES (DEFAULT, 2, series_public_id, study_internal_id);
+    EXCEPTION
+        WHEN unique_violation THEN
+            is_new_series := 0;
+    END;
+	SELECT internalid INTO series_internal_id FROM "resources" WHERE publicId = series_public_id AND resourcetype = 2 FOR UPDATE;  -- also locks the resource and its parent to prevent from deletion while we complete this transaction
+
+  	BEGIN
+		INSERT INTO "resources" VALUES (DEFAULT, 3, instance_public_id, series_internal_id);
+    EXCEPTION
+        WHEN unique_violation THEN
+            is_new_instance := 0;
+    END;
+    SELECT internalid INTO instance_internal_id FROM "resources" WHERE publicId = instance_public_id AND resourcetype = 3 FOR UPDATE;  -- also locks the resource and its parent to prevent from deletion while we complete this transaction
+
+    IF is_new_instance > 0 THEN
+        -- Move the patient to the end of the recycling order.
+        PERFORM PatientAddedOrUpdated(patient_internal_id, 1);
+    END IF;  
+END;
+
+$body$ 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.patient_internal_id, result.is_new_patient;
+    RAISE NOTICE 'Value studyInternalId: %, is_new: %', result.study_internal_id, result.is_new_study;
+    RAISE NOTICE 'Value seriesInternalId: %, is_new: %', result.series_internal_id, result.is_new_series;
+    RAISE NOTICE 'Value instanceInternalId: %, is_new: %', result.instance_internal_id, result.is_new_instance;
+    RAISE NOTICE '--------------';
+
+    SELECT * INTO result from CreateInstance('patient1', 'study1', 'series1', 'instance2');
+
+    RAISE NOTICE 'Value patientInternalId: %, is_new: %', result.patient_internal_id, result.is_new_patient;
+    RAISE NOTICE 'Value studyInternalId: %, is_new: %', result.study_internal_id, result.is_new_study;
+    RAISE NOTICE 'Value seriesInternalId: %, is_new: %', result.series_internal_id, result.is_new_series;
+    RAISE NOTICE 'Value instanceInternalId: %, is_new: %', result.instance_internal_id, result.is_new_instance;
+    RAISE NOTICE '--------------';
+
+    SELECT * INTO result from CreateInstance('patient1', 'study1', 'series1', 'instance2');
+
+    RAISE NOTICE 'Value patientInternalId: %, is_new: %', result.patient_internal_id, result.is_new_patient;
+    RAISE NOTICE 'Value studyInternalId: %, is_new: %', result.study_internal_id, result.is_new_study;
+    RAISE NOTICE 'Value seriesInternalId: %, is_new: %', result.series_internal_id, result.is_new_series;
+    RAISE NOTICE 'Value instanceInternalId: %, is_new: %', result.instance_internal_id, result.is_new_instance;
+    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, 10 connections
+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 (read-committed mode) : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 15.744 s
+                                                       test_concurrent_anonymize_same_study deletion took: 18.8 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 (read-committed mode) : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 9.514 s
+
+Orthanc 1.12.1 + PG 5.1 (serializable mode)          : test_upload_delete_same_study_from_multiple_threads with 5 workers and 3x repeat (10x): 23.016 s
+Orthanc mainline + PG mainline (read-committed mode) : test_upload_delete_same_study_from_multiple_threads with 5 workers and 3x repeat (10x): 7.129 s 
+
+Orthanc mainline + PG mainline (read-committed mode) : test_upload_multiple_studies_from_multiple_threads with 10 workers and 25 files and repeat 3x: 6.454 s
+
+With Docker with 10 connections SQL:
+osimis/orthanc:24.1.2 : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 18.956 s  FAIL !!!
+                        test_concurrent_anonymize_same_study deletion took: NA
+osimis/orthanc:current: test_concurrent_anonymize_same_study with 4 workers and 10x repeat:  6.867 s
+                        test_concurrent_anonymize_same_study deletion took: 9.095 s
+
+osimis/orthanc:24.1.2 : test_concurrent_uploads_same_study with 20 workers and 1x repeat:  9.822 s 
+osimis/orthanc:current: test_concurrent_uploads_same_study with 20 workers and 1x repeat: 16.027 s up to 38s !  (slower but the test is not representative of a real life scenario !!!!!)
+
+osimis/orthanc:24.1.2 : test_upload_delete_same_study_from_multiple_threads with 5 workers and 3x repeat (10x): 12.966 s
+osimis/orthanc:current: test_upload_delete_same_study_from_multiple_threads with 5 workers and 3x repeat (10x):  4.196 s
+
+osimis/orthanc:24.1.2 : test_upload_multiple_studies_from_multiple_threads with 10 workers and 25 files and repeat 3x: 8.957 s
+osimis/orthanc:current: test_upload_multiple_studies_from_multiple_threads with 10 workers and 25 files and repeat 3x: 2.671 s
+
+Testing the connecions (note: Orthanc and PG server running on the same server)
+10 connections : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 15.744 s
+1  connection  : test_concurrent_anonymize_same_study with 4 workers and 10x repeat: 21.341 s
+10 connections : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 6.57 s
+1  connection  : test_concurrent_uploads_same_study with 20 workers and 1x repeat: 10.223 s
+10 connections : test_upload_delete_same_study_from_multiple_threads with 5 workers and 3x repeat (10x): 7.129 s 
+1  connection  : test_upload_delete_same_study_from_multiple_threads with 5 workers and 3x repeat (10x): 11.172 s 
+
+
+TODO:
+- have a separate "thread" to UpdateStatistics ?
+
+- check https://discourse.orthanc-server.org/t/image-insert-are-too-slow-databse-performance-too-poor-when-using-mysql-mariadb/3820
+
+DONE:
+- implement a downgrade script ?  And test it in PotgresUpgrades integ tests
+- test the transfer plugin
+- perf tests: upload generated data (different studies)
+- In Docker images, re-enable MySQL & ODBC plugins + tests
+- reenable PatientRecyclingOrder
+- force the create/update DB transaction to be serializable (not needed: this is handled by POSTGRESQL_LOCK_DATABASE_SETUP)
+- PatientAddedFunc contains an IF  (check if other IF/THEN/ELSE pattern remains)
+- validate upgrade DB from previous Orthanc and from scratch
+- check minimal version of PG (9.5 - 9.6 ? for create index if not exists): seems to work with 9.5 cfr PotgresUpgrades integ tests
+- test events generation StableSeries .... (count the NewSeries, NewInstances event and make sure they match the numb)