Mercurial > hg > orthanc-databases
changeset 636:a79bfa4910c9
Fixed high memory usage due to caching of too many prepared SQL statement when using since & limit
| author | Alain Mazy <am@orthanc.team> |
|---|---|
| date | Mon, 03 Feb 2025 18:52:22 +0100 |
| parents | fa94274d15c3 |
| children | 77b9684c57b4 |
| files | Framework/Common/DatabaseManager.cpp Framework/Common/DatabaseManager.h Framework/Common/Dictionary.cpp Framework/Common/Dictionary.h Framework/Common/Query.cpp Framework/Common/Query.h Framework/Plugins/ISqlLookupFormatter.h Framework/Plugins/IndexBackend.cpp PostgreSQL/NEWS |
| diffstat | 9 files changed, 128 insertions(+), 18 deletions(-) [+] |
line wrap: on
line diff
--- a/Framework/Common/DatabaseManager.cpp Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Common/DatabaseManager.cpp Mon Feb 03 18:52:22 2025 +0100 @@ -550,18 +550,35 @@ } } - DatabaseManager::CachedStatement::CachedStatement(const StatementId& statementId, DatabaseManager& manager, const std::string& sql) : StatementBase(manager), statementId_(statementId) { + Query::Parameters emptyParameters; + Setup(sql, emptyParameters); + } + + + DatabaseManager::CachedStatement::CachedStatement(const StatementId& statementId, + DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes) : + StatementBase(manager), + statementId_(statementId) + { + Setup(sql, parametersTypes); + } + + void DatabaseManager::CachedStatement::Setup(const std::string& sql, + const Query::Parameters& parametersTypes) + { statement_ = GetManager().LookupCachedStatement(statementId_); if (statement_ == NULL) { - SetQuery(new Query(sql)); + SetQuery(new Query(sql, parametersTypes)); } else { @@ -628,7 +645,17 @@ const std::string& sql) : StatementBase(manager) { - SetQuery(new Query(sql)); + Query::Parameters emptyParameters; + SetQuery(new Query(sql, emptyParameters)); + } + + + DatabaseManager::StandaloneStatement::StandaloneStatement(DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes) : + StatementBase(manager) + { + SetQuery(new Query(sql, parametersTypes)); }
--- a/Framework/Common/DatabaseManager.h Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Common/DatabaseManager.h Mon Feb 03 18:52:22 2025 +0100 @@ -212,11 +212,19 @@ StatementId statementId_; IPrecompiledStatement* statement_; + void Setup(const std::string& sql, + const Query::Parameters& parametersTypes); + public: CachedStatement(const StatementId& statementId, DatabaseManager& manager, const std::string& sql); + CachedStatement(const StatementId& statementId, + DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes); + void Execute() { Dictionary parameters; @@ -247,6 +255,10 @@ StandaloneStatement(DatabaseManager& manager, const std::string& sql); + StandaloneStatement(DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes); + virtual ~StandaloneStatement(); void Execute()
--- a/Framework/Common/Dictionary.cpp Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Common/Dictionary.cpp Mon Feb 03 18:52:22 2025 +0100 @@ -155,4 +155,15 @@ return *found->second; } } + + void Dictionary::GetParametersType(Query::Parameters& target) const + { + target.clear(); + + for (Values::const_iterator it = values_.begin(); + it != values_.end(); ++it) + { + target[it->first] = it->second->GetType(); + } + } }
--- a/Framework/Common/Dictionary.h Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Common/Dictionary.h Mon Feb 03 18:52:22 2025 +0100 @@ -27,6 +27,7 @@ #include <map> #include <stdint.h> +#include "Query.h" namespace OrthancDatabases { @@ -74,5 +75,7 @@ void SetNullValue(const std::string& key); const IValue& GetValue(const std::string& key) const; + + void GetParametersType(Query::Parameters& target) const; }; }
--- a/Framework/Common/Query.cpp Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Common/Query.cpp Mon Feb 03 18:52:22 2025 +0100 @@ -56,7 +56,7 @@ }; - void Query::Setup(const std::string& sql) + void Query::Setup(const std::string& sql, const Query::Parameters& parametersTypes) { boost::regex regex("\\$\\{(.*?)\\}"); @@ -76,7 +76,17 @@ parameter = parameter.substr(2, parameter.size() - 3); tokens_.push_back(new Token(true, parameter)); - parameters_[parameter] = ValueType_Utf8String; + + // when we already know the type of the parameter, set it directly to the right value + Query::Parameters::const_iterator itp = parametersTypes.find(parameter); + if (itp != parametersTypes.end()) + { + parameters_[parameter] = itp->second; + } + else + { + parameters_[parameter] = ValueType_Utf8String; + } last = it->second; @@ -93,7 +103,15 @@ Query::Query(const std::string& sql) : readOnly_(false) { - Setup(sql); + Query::Parameters emptyParameters; + Setup(sql, emptyParameters); + } + + Query::Query(const std::string& sql, + const Query::Parameters& parametersTypes) : + readOnly_(false) + { + Setup(sql, parametersTypes); } @@ -101,7 +119,16 @@ bool readOnly) : readOnly_(readOnly) { - Setup(sql); + Query::Parameters emptyParameters; + Setup(sql, emptyParameters); + } + + Query::Query(const std::string& sql, + bool readOnly, + const Query::Parameters& parametersTypes) : + readOnly_(readOnly) + { + Setup(sql, parametersTypes); }
--- a/Framework/Common/Query.h Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Common/Query.h Mon Feb 03 18:52:22 2025 +0100 @@ -48,8 +48,8 @@ ValueType type) = 0; }; + typedef std::map<std::string, ValueType> Parameters; private: - typedef std::map<std::string, ValueType> Parameters; class Token; @@ -57,14 +57,19 @@ Parameters parameters_; bool readOnly_; - void Setup(const std::string& sql); + void Setup(const std::string& sql, const Query::Parameters& parametersTypes); public: explicit Query(const std::string& sql); + explicit Query(const std::string& sql, const Parameters& parametersType); Query(const std::string& sql, bool isReadOnly); + Query(const std::string& sql, + bool isReadOnly, + const Parameters& parametersType); + ~Query(); bool IsReadOnly() const
--- a/Framework/Plugins/ISqlLookupFormatter.h Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Plugins/ISqlLookupFormatter.h Mon Feb 03 18:52:22 2025 +0100 @@ -56,6 +56,8 @@ virtual std::string GenerateParameter(const std::string& value) = 0; + virtual std::string GenerateParameter(const int64_t& value) = 0; + virtual std::string FormatResourceType(Orthanc::ResourceType level) = 0; virtual std::string FormatWildcardEscape() = 0;
--- a/Framework/Plugins/IndexBackend.cpp Mon Feb 03 14:40:42 2025 +0100 +++ b/Framework/Plugins/IndexBackend.cpp Mon Feb 03 18:52:22 2025 +0100 @@ -2180,6 +2180,16 @@ return "${" + key + "}"; } + virtual std::string GenerateParameter(const int64_t& value) + { + const std::string key = FormatParameter(count_); + + count_ ++; + dictionary_.SetIntegerValue(key, value); + + return "${" + key + "}"; + } + virtual std::string FormatResourceType(Orthanc::ResourceType level) { return boost::lexical_cast<std::string>(MessagesToolbox::ConvertToPlainC(level)); @@ -2229,11 +2239,13 @@ { if (count > 0 || since > 0) { - sql += " OFFSET " + boost::lexical_cast<std::string>(since) + " ROWS "; + std::string parameterSince = GenerateParameter(since); + sql += " OFFSET " + parameterSince + " ROWS "; } if (count > 0) { - sql += " FETCH NEXT " + boost::lexical_cast<std::string>(count) + " ROWS ONLY "; + std::string parameterCount = GenerateParameter(count); + sql += " FETCH NEXT " + parameterCount + " ROWS ONLY "; } }; break; case Dialect_SQLite: @@ -2241,26 +2253,32 @@ { if (count > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(count); + std::string parameterCount = GenerateParameter(count); + sql += " LIMIT " + parameterCount; } if (since > 0) { - sql += " OFFSET " + boost::lexical_cast<std::string>(since); + std::string parameterSince = GenerateParameter(since); + sql += " OFFSET " + parameterSince; } }; break; case Dialect_MySQL: { if (count > 0 && since > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(since) + ", " + boost::lexical_cast<std::string>(count); + std::string parameterCount = GenerateParameter(count); + std::string parameterSince = GenerateParameter(since); + sql += " LIMIT " + parameterSince + ", " + parameterCount; } else if (count > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(count); + std::string parameterCount = GenerateParameter(count); + sql += " LIMIT " + parameterCount; } else if (since > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(since) + ", 18446744073709551615"; // max uint64 value when you don't want any limit + std::string parameterSince = GenerateParameter(since); + sql += " LIMIT " + parameterSince + ", 18446744073709551615"; // max uint64 value when you don't want any limit } }; break; default: @@ -4035,13 +4053,16 @@ sql += " ORDER BY c0_queryId, c2_rowNumber"; // this is really important to make sure that the Lookup query is the first one to provide results since we use it to create the responses element ! std::unique_ptr<DatabaseManager::StatementBase> statement; + + Query::Parameters parametersTypes; + formatter.GetDictionary().GetParametersType(parametersTypes); if (manager.GetDialect() == Dialect_MySQL) { // TODO: investigate why "complex" cached statement do not seem to work properly in MySQL - statement.reset(new DatabaseManager::StandaloneStatement(manager, sql)); + statement.reset(new DatabaseManager::StandaloneStatement(manager, sql, parametersTypes)); } else { - statement.reset(new DatabaseManager::CachedStatement(STATEMENT_FROM_HERE_DYNAMIC(sql), manager, sql)); + statement.reset(new DatabaseManager::CachedStatement(STATEMENT_FROM_HERE_DYNAMIC(sql), manager, sql, parametersTypes)); } statement->Execute(formatter.GetDictionary());