-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix: reduce number of iops on postgresql engines #3289
Conversation
fixes ory#3137 - The SQL update here would potentially update a lot of rows, which did not need updating. In some DB engines, this would not be an issue, because the redundant writes are ignored. But on PostgreSQL engines, it is another story; here it would actually carry out the writes, leading to a potentially high number of redundant iops when the engine is vaccuming outdated records. With this change, the SQL update will only affect the rows which is not in the desired state already.
Codecov Report
@@ Coverage Diff @@
## master #3289 +/- ##
==========================================
- Coverage 78.42% 76.91% -1.52%
==========================================
Files 123 123
Lines 12518 8918 -3600
==========================================
- Hits 9817 6859 -2958
+ Misses 2061 1631 -430
+ Partials 640 428 -212
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
I do not understand why Codecov thinks that my change will reduce coverage by 1,51% |
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.
Nice find!
@aeneasr Thank you for reviewing and accepting my PR. |
We will release Hydra 2.0 at the summit |
Hello @lunar-smh |
Optimise SQL update to avoid redundant writes, which may lead to a high number of iops on postgresql engines.
This change will not change the behaviour of the code. It is only an optimisation of the update, so Postgresql does not need to spend unneccessary resources on vaccuming. I assume that the actual update is going to perform slightly better as well.
fixes #3137
Related issue(s)
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further Comments