** This report needs more confirmation through a proper test setup. **
Prosody can create more prepared statements than strictly necessary for some kinds of queries, which can lead to unnecessary resource consumption (i.e. garbage collecting idle prepared statements that could be reused).
0.10 has improved a number of SQL-related things, and I feel strongly that this fix should also go into the release, or at least some testing to assess the real-world impact.
MattJ
on
Changes
tags Priority-High
owner MattJ
MattJ
on
Changes
tags Milestone-0.10
Zash
on
Changes
tags Status-Accepted
Waqas
on
Patch being tested:
diff --git a/sql.lua b/sql2.lua
index 1574991..61d6af4 100644
--- a/sql.lua
+++ b/sql2.lua
@@ -175,7 +175,11 @@ function engine:execute_query(sql, ...)
sql = self:prepquery(sql);
local stmt = assert(self.conn:prepare(sql));
assert(stmt:execute(...));
- return stmt:rows();
+ local result = {};
+ for row in stmt:rows() do result[#result + 1] = row; end
+ stmt:close();
+ local i = 0;
+ return function() i=i+1; return result[i]; end;
end
function engine:execute_update(sql, ...)
sql = self:prepquery(sql);
Changes
ownerMattJ Waqas
Waqas
on
The above patch reads prepared statements eagerly and closes them explicitly. This works around the scenario where prepared statements are being created faster than the GC can clean them, resulting in hitting the database's prepared statement limit.
Zash
on
Doesn't that fix #391, while bumping statement reuse to a future branch?
Zash
on
Changes
tags Patch
Waqas
on
Zash, yes, as we had decided to simply solve that piece of the two related issues. Let's re-tag that one for 0.10, and push this one out.
MattJ
on
Dropping the priority on this issue in favour of #391 (which is what the above patch actually fixes)
Changes
tags Priority-Medium
MattJ
on
Changes
tagsPatch
Zash
on
This never made it into 0.10 and it hasn't come up as something to do for 0.11, so bumping milestone to 0.12.
Changes
tags Milestone-0.12
MattJ
on
Closing this as it hasn't been an issue anyone encountered for a while (probably due to the #391 fix). SQL optimization is a whole task in itself that we may tackle one day.
** This report needs more confirmation through a proper test setup. ** Prosody can create more prepared statements than strictly necessary for some kinds of queries, which can lead to unnecessary resource consumption (i.e. garbage collecting idle prepared statements that could be reused). 0.10 has improved a number of SQL-related things, and I feel strongly that this fix should also go into the release, or at least some testing to assess the real-world impact.
Patch being tested: diff --git a/sql.lua b/sql2.lua index 1574991..61d6af4 100644 --- a/sql.lua +++ b/sql2.lua @@ -175,7 +175,11 @@ function engine:execute_query(sql, ...) sql = self:prepquery(sql); local stmt = assert(self.conn:prepare(sql)); assert(stmt:execute(...)); - return stmt:rows(); + local result = {}; + for row in stmt:rows() do result[#result + 1] = row; end + stmt:close(); + local i = 0; + return function() i=i+1; return result[i]; end; end function engine:execute_update(sql, ...) sql = self:prepquery(sql);
ChangesMattJWaqasThe above patch reads prepared statements eagerly and closes them explicitly. This works around the scenario where prepared statements are being created faster than the GC can clean them, resulting in hitting the database's prepared statement limit.
Doesn't that fix #391, while bumping statement reuse to a future branch?
Zash, yes, as we had decided to simply solve that piece of the two related issues. Let's re-tag that one for 0.10, and push this one out.
Dropping the priority on this issue in favour of #391 (which is what the above patch actually fixes)
ChangesPatchThis never made it into 0.10 and it hasn't come up as something to do for 0.11, so bumping milestone to 0.12.
ChangesClosing this as it hasn't been an issue anyone encountered for a while (probably due to the #391 fix). SQL optimization is a whole task in itself that we may tackle one day.
Changes