-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Suggest increasing busy_timeout to prevent errors #7945
Conversation
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 PR! However I don't think we need it after #7927. For future PRs, please read the contribution guidelines regarding how to write commit messages.
docs/sqlite.md
Outdated
@@ -39,9 +39,12 @@ journal_mode=wal | |||
busy_timeout=5000 // Overried with the db.sqlite.busytimeout option. | |||
``` | |||
|
|||
Default `busy_timeout` unit is milliseconds. A 5s timeout could lead to `SQLITE_BUSY` errors, which can be prevented by setting a longer timeout. |
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.
This line should be wrapped. Also I think this is no longer true after #7927
So what is missing, is that I didn't provide an extended description for the edit other than the commit title, and I didn't wrap the text in the md? From my understanding, the new PR you cited will only prevent SQLite timeouts from causing issues, rather than prevent them entirely, am I wrong? I'd rather prefer my db backend doesn't hit timeout errors at all. |
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.
So what is missing, is that I didn't provide an extended description for the edit other than the commit title, and I didn't wrap the text in the md?
You need to add a prefix as instructed here,
the commit header message should begin with the prefix of the modified package. For example, if a commit was made to modify the lnwallet package, it should start with lnwallet: .
So just simply put a prefix docs: suggest...
On a side note, why does a simple md edit fail some of the tests?
We have test flakes and we are in the process of fixing them.
From my understanding, the new PR you cited will only prevent SQLite timeouts from causing issues, rather than prevent them entirely, am I wrong? I'd rather prefer my db backend doesn't hit timeout errors at all.
It will catch the SQLITE_BUSY
error and retry the transaction up to 10 times. Maybe we could document this behavior as well?
That being said, still feels weird to see the SQLITE_BUSY
error as lnd
only has one database connection.
wrapped md line about timeout, and fixed commit subject.
Closing and starting a new PR that follows the suggestions above |
Change Description
Update of SQLite doc to explain how
busy_timeout
can be tweaked to reduceSQLITE_BUSY
errors.Steps to Test
Nothing special, this follows up with #7869
Pull Request Checklist
Testing
Code Style and Documentation
[skip ci]
in the commit message for small changes.📝 Please see our Contribution Guidelines for further guidance.