-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Optimize postgres - prepared statements in select #2182
Conversation
This PR may contain changes to database schema of one of the drivers. If you are introducing any changes to the schema, make sure the upgrade from the latest release to this change passes without any errors/issues. Please make sure the label |
You can find the image built from this PR at
Built from 22d53a8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Minor style comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good! Special thanks that this approach prevents sql injection attacks.
## but, on the other hand, the connection remains in "db.pqisBusy() == 1" for 100ms more. | ||
## I think this is because `nwaku` is single-threaded and it has to handle many connections (20) | ||
## simultaneously. Therefore, there is an underlying resource sharing (cpu) that makes this | ||
## to happen. Notice that the _Postgres_ database spawns one process per each connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate this great, explanatory comment here!!!
|
||
return ok(rows) | ||
|
||
method getMessages*(s: PostgresDriver, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come PostgresDriver reference named as "s"? :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comments! I'll review that in upcoming refactoring PRs :)
Description
The prepared statements is a mechanism that allows saving time for common queries.
We are applying that approach for the "select" queries, i.e. queries that come from the Store protocol requests. Notice that we were already applying this in "insert" operations.
With that, we enhance the performance of "select" queries. However, bear in mind that there is an underlying time consumption when performing concurrent queries. I've added a long comment about that in
dbconn.nim
.Changes
Issue
#1842