# HG changeset patch # User Sebastien Jodogne # Date 1557388331 -7200 # Node ID 3266785d5627dc2fb13ba46a6941e9c2132ca1a2 # Parent e26690365c2501ab54bd92f9a33acb1475436e94 cleaning up PostgreSQL locks with constants diff -r e26690365c25 -r 3266785d5627 PostgreSQL/Plugins/PostgreSQLDefinitions.h --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/PostgreSQL/Plugins/PostgreSQLDefinitions.h Thu May 09 09:52:11 2019 +0200 @@ -0,0 +1,49 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2019 Osimis S.A., Belgium + * + * This program is free software: you can redistribute it and/or + * modify it under the terms of the GNU Affero General Public License + * as published by the Free Software Foundation, either version 3 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + **/ + + +#pragma once + +#include + + +/** + * This advisory lock is used if the "Lock" option is set to "true", + * in order to prevent the execution of two PostgreSQL index plugins + * on the same database. + **/ +static const int32_t POSTGRESQL_LOCK_INDEX = 42; + + +/** + * This advisory lock is used if the "Lock" option is set to "true", + * in order to prevent the execution of two PostgreSQL storage area + * plugins on the same database. + **/ +static const int32_t POSTGRESQL_LOCK_STORAGE = 43; + + +/** + * Transient advisory lock to protect the setup of the database, + * because concurrent statements like "CREATE TABLE" are not protected + * by transactions. + * https://groups.google.com/d/msg/orthanc-users/yV3LSTh_TjI/h3PRApJFBAAJ + **/ +static const int32_t POSTGRESQL_LOCK_DATABASE_SETUP = 44; diff -r e26690365c25 -r 3266785d5627 PostgreSQL/Plugins/PostgreSQLIndex.cpp --- a/PostgreSQL/Plugins/PostgreSQLIndex.cpp Wed May 08 21:09:18 2019 +0200 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.cpp Thu May 09 09:52:11 2019 +0200 @@ -24,6 +24,7 @@ #include "../../Framework/Plugins/GlobalProperties.h" #include "../../Framework/PostgreSQL/PostgreSQLDatabase.h" #include "../../Framework/PostgreSQL/PostgreSQLTransaction.h" +#include "PostgreSQLDefinitions.h" #include // Auto-generated file @@ -71,192 +72,188 @@ if (parameters_.HasLock()) { - db->AdvisoryLock(42 /* some arbitrary constant */); - } - - /** - * Try and acquire a transient advisory lock to protect the setup - * of the database, because concurrent statements like "CREATE - * TABLE" are not protected by transactions. - * https://groups.google.com/d/msg/orthanc-users/yV3LSTh_TjI/h3PRApJFBAAJ - **/ - PostgreSQLDatabase::TransientAdvisoryLock lock(*db, 44 /* some arbitrary constant */); - - if (clearAll_) - { - db->ClearAll(); - } - - { - PostgreSQLTransaction t(*db); - - if (!db->DoesTableExist("Resources")) - { - std::string query; - - Orthanc::EmbeddedResources::GetFileResource - (query, Orthanc::EmbeddedResources::POSTGRESQL_PREPARE_INDEX); - db->Execute(query); - - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion, expectedVersion); - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, 1); - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasTrigramIndex, 0); - } - - if (!db->DoesTableExist("Resources")) - { - LOG(ERROR) << "Corrupted PostgreSQL database"; - throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); - } - - int version = 0; - if (!LookupGlobalIntegerProperty(version, *db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion) || - version != 6) - { - LOG(ERROR) << "PostgreSQL plugin is incompatible with database schema version: " << version; - throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); - } - - int revision; - if (!LookupGlobalIntegerProperty(revision, *db, t, Orthanc::GlobalProperty_DatabasePatchLevel)) - { - revision = 1; - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision); - } - - if (revision != 1) - { - LOG(ERROR) << "PostgreSQL plugin is incompatible with database schema revision: " << revision; - throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); - } - - t.Commit(); + db->AdvisoryLock(POSTGRESQL_LOCK_INDEX); } { - PostgreSQLTransaction t(*db); + PostgreSQLDatabase::TransientAdvisoryLock lock(*db, POSTGRESQL_LOCK_DATABASE_SETUP); - int hasTrigram = 0; - if (!LookupGlobalIntegerProperty(hasTrigram, *db, t, - Orthanc::GlobalProperty_HasTrigramIndex) || - hasTrigram != 1) + if (clearAll_) + { + db->ClearAll(); + } + { - /** - * Apply fix for performance issue (speed up wildcard search - * by using GIN trigrams). This implements the patch suggested - * in issue #47, BUT we also keep the original - * "DicomIdentifiersIndexValues", as it leads to better - * performance for "strict" searches (i.e. searches involving - * no wildcard). - * https://www.postgresql.org/docs/current/static/pgtrgm.html - * https://bitbucket.org/sjodogne/orthanc/issues/47/index-improvements-for-pg-plugin - **/ - try + PostgreSQLTransaction t(*db); + + if (!db->DoesTableExist("Resources")) + { + std::string query; + + Orthanc::EmbeddedResources::GetFileResource + (query, Orthanc::EmbeddedResources::POSTGRESQL_PREPARE_INDEX); + db->Execute(query); + + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion, expectedVersion); + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, 1); + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasTrigramIndex, 0); + } + + if (!db->DoesTableExist("Resources")) { - // We've observed 9 minutes on DB with 100000 studies - LOG(WARNING) << "Trying to enable trigram matching on the PostgreSQL database " - << "to speed up wildcard searches. This may take several minutes"; + LOG(ERROR) << "Corrupted PostgreSQL database"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); + } - db->Execute( - "CREATE EXTENSION IF NOT EXISTS pg_trgm; " - "CREATE INDEX DicomIdentifiersIndexValues2 ON DicomIdentifiers USING gin(value gin_trgm_ops);"); - - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasTrigramIndex, 1); - LOG(WARNING) << "Trigram index has been created"; + int version = 0; + if (!LookupGlobalIntegerProperty(version, *db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion) || + version != 6) + { + LOG(ERROR) << "PostgreSQL plugin is incompatible with database schema version: " << version; + throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); + } - t.Commit(); - } - catch (Orthanc::OrthancException&) + int revision; + if (!LookupGlobalIntegerProperty(revision, *db, t, Orthanc::GlobalProperty_DatabasePatchLevel)) { - LOG(WARNING) << "Performance warning: Your PostgreSQL server does " - << "not support trigram matching"; - LOG(WARNING) << "-> Consider installing the \"pg_trgm\" extension on the " - << "PostgreSQL server, e.g. on Debian: sudo apt install postgresql-contrib"; + revision = 1; + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision); } - } - else - { + + if (revision != 1) + { + LOG(ERROR) << "PostgreSQL plugin is incompatible with database schema revision: " << revision; + throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); + } + t.Commit(); } - } - { - PostgreSQLTransaction t(*db); + { + PostgreSQLTransaction t(*db); - int property = 0; - if (!LookupGlobalIntegerProperty(property, *db, t, - Orthanc::GlobalProperty_HasCreateInstance) || - property != 2) - { - LOG(INFO) << "Installing the CreateInstance extension"; + int hasTrigram = 0; + if (!LookupGlobalIntegerProperty(hasTrigram, *db, t, + Orthanc::GlobalProperty_HasTrigramIndex) || + hasTrigram != 1) + { + /** + * Apply fix for performance issue (speed up wildcard search + * by using GIN trigrams). This implements the patch suggested + * in issue #47, BUT we also keep the original + * "DicomIdentifiersIndexValues", as it leads to better + * performance for "strict" searches (i.e. searches involving + * no wildcard). + * https://www.postgresql.org/docs/current/static/pgtrgm.html + * https://bitbucket.org/sjodogne/orthanc/issues/47/index-improvements-for-pg-plugin + **/ + try + { + // We've observed 9 minutes on DB with 100000 studies + LOG(WARNING) << "Trying to enable trigram matching on the PostgreSQL database " + << "to speed up wildcard searches. This may take several minutes"; + + db->Execute( + "CREATE EXTENSION IF NOT EXISTS pg_trgm; " + "CREATE INDEX DicomIdentifiersIndexValues2 ON DicomIdentifiers USING gin(value gin_trgm_ops);"); + + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasTrigramIndex, 1); + LOG(WARNING) << "Trigram index has been created"; - if (property == 1) + t.Commit(); + } + catch (Orthanc::OrthancException&) + { + LOG(WARNING) << "Performance warning: Your PostgreSQL server does " + << "not support trigram matching"; + LOG(WARNING) << "-> Consider installing the \"pg_trgm\" extension on the " + << "PostgreSQL server, e.g. on Debian: sudo apt install postgresql-contrib"; + } + } + else { - // Drop older, experimental versions of this extension - db->Execute("DROP FUNCTION CreateInstance(" - "IN patient TEXT, IN study TEXT, IN series TEXT, in instance TEXT)"); + t.Commit(); } + } + + { + PostgreSQLTransaction t(*db); + + int property = 0; + if (!LookupGlobalIntegerProperty(property, *db, t, + Orthanc::GlobalProperty_HasCreateInstance) || + property != 2) + { + LOG(INFO) << "Installing the CreateInstance extension"; + + if (property == 1) + { + // Drop older, experimental versions of this extension + db->Execute("DROP FUNCTION CreateInstance(" + "IN patient TEXT, IN study TEXT, IN series TEXT, in instance TEXT)"); + } - std::string query; - Orthanc::EmbeddedResources::GetFileResource - (query, Orthanc::EmbeddedResources::POSTGRESQL_CREATE_INSTANCE); - db->Execute(query); + std::string query; + Orthanc::EmbeddedResources::GetFileResource + (query, Orthanc::EmbeddedResources::POSTGRESQL_CREATE_INSTANCE); + db->Execute(query); - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasCreateInstance, 2); - } + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasCreateInstance, 2); + } - if (!LookupGlobalIntegerProperty(property, *db, t, - Orthanc::GlobalProperty_GetTotalSizeIsFast) || - property != 1) - { - LOG(INFO) << "Installing the FastTotalSize extension"; + if (!LookupGlobalIntegerProperty(property, *db, t, + Orthanc::GlobalProperty_GetTotalSizeIsFast) || + property != 1) + { + LOG(INFO) << "Installing the FastTotalSize extension"; - std::string query; - Orthanc::EmbeddedResources::GetFileResource - (query, Orthanc::EmbeddedResources::POSTGRESQL_FAST_TOTAL_SIZE); - db->Execute(query); + std::string query; + Orthanc::EmbeddedResources::GetFileResource + (query, Orthanc::EmbeddedResources::POSTGRESQL_FAST_TOTAL_SIZE); + db->Execute(query); - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_GetTotalSizeIsFast, 1); - } + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_GetTotalSizeIsFast, 1); + } - // Installing this extension requires the "GlobalIntegers" table - // created by the "FastTotalSize" extension - property = 0; - if (!LookupGlobalIntegerProperty(property, *db, t, - Orthanc::GlobalProperty_HasFastCountResources) || - property != 1) - { - LOG(INFO) << "Installing the FastCountResources extension"; + // Installing this extension requires the "GlobalIntegers" table + // created by the "FastTotalSize" extension + property = 0; + if (!LookupGlobalIntegerProperty(property, *db, t, + Orthanc::GlobalProperty_HasFastCountResources) || + property != 1) + { + LOG(INFO) << "Installing the FastCountResources extension"; - std::string query; - Orthanc::EmbeddedResources::GetFileResource - (query, Orthanc::EmbeddedResources::POSTGRESQL_FAST_COUNT_RESOURCES); - db->Execute(query); + std::string query; + Orthanc::EmbeddedResources::GetFileResource + (query, Orthanc::EmbeddedResources::POSTGRESQL_FAST_COUNT_RESOURCES); + db->Execute(query); - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasFastCountResources, 1); - } + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_HasFastCountResources, 1); + } - // Installing this extension requires the "GlobalIntegers" table - // created by the "GetLastChangeIndex" extension - property = 0; - if (!LookupGlobalIntegerProperty(property, *db, t, - Orthanc::GlobalProperty_GetLastChangeIndex) || - property != 1) - { - LOG(INFO) << "Installing the GetLastChangeIndex extension"; + // Installing this extension requires the "GlobalIntegers" table + // created by the "GetLastChangeIndex" extension + property = 0; + if (!LookupGlobalIntegerProperty(property, *db, t, + Orthanc::GlobalProperty_GetLastChangeIndex) || + property != 1) + { + LOG(INFO) << "Installing the GetLastChangeIndex extension"; - std::string query; - Orthanc::EmbeddedResources::GetFileResource - (query, Orthanc::EmbeddedResources::POSTGRESQL_GET_LAST_CHANGE_INDEX); - db->Execute(query); + std::string query; + Orthanc::EmbeddedResources::GetFileResource + (query, Orthanc::EmbeddedResources::POSTGRESQL_GET_LAST_CHANGE_INDEX); + db->Execute(query); - SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_GetLastChangeIndex, 1); + SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_GetLastChangeIndex, 1); + } + + t.Commit(); } - - t.Commit(); } return db.release(); diff -r e26690365c25 -r 3266785d5627 PostgreSQL/Plugins/PostgreSQLStorageArea.cpp --- a/PostgreSQL/Plugins/PostgreSQLStorageArea.cpp Wed May 08 21:09:18 2019 +0200 +++ b/PostgreSQL/Plugins/PostgreSQLStorageArea.cpp Thu May 09 09:52:11 2019 +0200 @@ -20,6 +20,7 @@ #include "PostgreSQLStorageArea.h" +#include "PostgreSQLDefinitions.h" #include "../../Framework/PostgreSQL/PostgreSQLTransaction.h" @@ -37,38 +38,34 @@ if (parameters_.HasLock()) { - db->AdvisoryLock(43 /* some arbitrary constant */); - } - - /** - * Try and acquire a transient advisory lock to protect the setup - * of the database, because concurrent statements like "CREATE - * TABLE" are not protected by transactions. - * https://groups.google.com/d/msg/orthanc-users/yV3LSTh_TjI/h3PRApJFBAAJ - **/ - PostgreSQLDatabase::TransientAdvisoryLock lock(*db, 44 /* some arbitrary constant */); - - if (clearAll_) - { - db->ClearAll(); + db->AdvisoryLock(POSTGRESQL_LOCK_STORAGE); } { - PostgreSQLTransaction t(*db); + PostgreSQLDatabase::TransientAdvisoryLock lock(*db, POSTGRESQL_LOCK_DATABASE_SETUP); - if (!db->DoesTableExist("StorageArea")) + if (clearAll_) { - db->Execute("CREATE TABLE IF NOT EXISTS StorageArea(" - "uuid VARCHAR NOT NULL PRIMARY KEY," - "content OID NOT NULL," - "type INTEGER NOT NULL)"); - - // Automatically remove the large objects associated with the table - db->Execute("CREATE OR REPLACE RULE StorageAreaDelete AS ON DELETE " - "TO StorageArea DO SELECT lo_unlink(old.content);"); + db->ClearAll(); } - t.Commit(); + { + PostgreSQLTransaction t(*db); + + if (!db->DoesTableExist("StorageArea")) + { + db->Execute("CREATE TABLE IF NOT EXISTS StorageArea(" + "uuid VARCHAR NOT NULL PRIMARY KEY," + "content OID NOT NULL," + "type INTEGER NOT NULL)"); + + // Automatically remove the large objects associated with the table + db->Execute("CREATE OR REPLACE RULE StorageAreaDelete AS ON DELETE " + "TO StorageArea DO SELECT lo_unlink(old.content);"); + } + + t.Commit(); + } } return db.release();