Conversation
|
I've assigned @tankyleo as a reviewer! |
|
Oops, looks like the new |
tankyleo
left a comment
tankyleo
left a comment
There was a problem hiding this comment.
Thanks again sorry for the delay, feel free to rebase.
So I can `POSTGRES_ENDPOINT='postgresql://%2Frun%2Fuser%2F1000' cargo test` against my user-local, socket-activated dev postgres.
Without this change, we would re-parse and re-plan each query, potentially multiple times per request (!). Instead, maintain a small prepared statement cache per connection. We'll lazily prepare a query statement the first time we execute it, then use the prepared statement thereafter. This change reduces latency by ~20-60% on my small benchmark suite, but is particularly impactful on batch conditional updates, where we were previously re-parsing and re-planning the put query for each item in the batch. This change improved throughput there by ~1.5x (~4.8k/s -> 12.3k/s).
ba56010 to
b4c596c
Compare
|
|
||
| let statement = client.prepare(query).await?; | ||
| self.statements.insert(query, statement.clone()); | ||
| Ok(statement) |
There was a problem hiding this comment.
Reusing the statement here can cause postgres to choose and reuse a generic plan that can perform poorly with varying parameters on our list queries.
From this documentation: https://www.postgresql.org/docs/16/sql-prepare.html
"A prepared statement can be executed with either a generic plan or a custom plan. A generic plan is the same across all executions, while a custom plan is generated for a specific execution using the parameter values given in that call. Use of a generic plan avoids planning overhead, but in some situations a custom plan will be much more efficient to execute because the planner can make use of knowledge of the parameter values. (Of course, if the prepared statement has no parameters, then this is moot and a generic plan is always used.)
By default (that is, when plan_cache_mode is set to auto), the server will automatically choose whether to use a generic or custom plan for a prepared statement that has parameters. The current rule for this is that the first five executions are done with custom plans and the average estimated cost of those plans is calculated. Then a generic plan is created and its estimated cost is compared to the average custom-plan cost. Subsequent executions use the generic plan if its cost is not so much higher than the average custom-plan cost as to make repeated replanning seem preferable."
Let me know what you think.
3 participants
Without this change, we would re-parse and re-plan each query, potentially multiple times per request (!). For simple queries, this appears to add ~100-200 us overhead. More for more complex queries.
Instead, maintain a small prepared statement cache per connection. We'll lazily prepare a query statement the first time we execute it, then use the prepared statement thereafter.
This change reduces latency by ~20-60% on my small benchmark suite, but is particularly impactful on batch conditional updates, where we were previously re-parsing and re-planning the
putquery for each item in the batch. This change improved throughput there by ~2.5x (~4.8k/s -> 12.3k/s).Using const
&'static strqueries is really more a stylistic preference on my part. I think it adds a nice roadblock to prevent people from accidentally addinglet stmt = format!("..", untrusted_user_input)-> SQL injection.Migrations and other admin queries are still uncached, since they're usually only executed once.