Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[performance] retry db queries on busy errors #2025

Merged
merged 16 commits into from
Jul 25, 2023
Merged

[performance] retry db queries on busy errors #2025

merged 16 commits into from
Jul 25, 2023

Conversation

NyaaaWhatsUpDoc
Copy link
Member

Description

  • rename DBConn to WrappedDB to better follow "sql" package name conventions
  • wrap bun.DB to perform retry on returned busy-like-errors (SQLite only, for now(?))
  • removes unnecessary db.Error type
  • makes db.ErrNoEntries simply a ptr to sqlite.ErrNoRows since that is dialect agnostic

Checklist

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

@NyaaaWhatsUpDoc
catch SQLITE_BUSY errors, wrap bun.DB to use our own busy retrier, re…
50e32cc 
…move unnecessary db.Error type

Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
remove dead code
4de119b 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
remove more dead code, add missing error arguments
8990d72 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
update sqlite to use maxOpenConns()
ff3eece 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
add uncommitted changes
0db84d6 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
use direct calls-through for the ConnIface to make sure we don't doub…
6f192d8 
…le query hook

Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
expose underlying bun.DB better
0eda66c 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
retry on the correct busy error
8310b9e 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
use longer possible maxRetries for db retry-backoff
5ca2324 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
remove the note regarding max-open-conns only applying to postgres
a0f1d77 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
improved code commenting
ab465cf 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
remove unnecessary infof call (just use info)
9c1e925 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
rename DBConn to WrappedDB to better follow sql package name conventions
a9aa2ed 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
update test error string checks
8882cd7 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc marked this pull request as ready for review July 24, 2023 12:58
@NyaaaWhatsUpDoc
shush linter
daf243c 
Signed-off-by: kim <[email protected]>
@NyaaaWhatsUpDoc
Copy link
Member Author

this closes #1407

tsmethurst
tsmethurst reviewed Jul 25, 2023
View reviewed changes
Copy link
Contributor

@tsmethurst tsmethurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! A nice neat solution!

@tsmethurst tsmethurst merged commit 5f3e095 into superseriousbusiness:main Jul 25, 2023
@tsmethurst tsmethurst mentioned this pull request Jul 25, 2023
Closed
@S0yKaf S0yKaf mentioned this pull request Sep 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsmethurst tsmethurst tsmethurst left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NyaaaWhatsUpDoc @tsmethurst