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.

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

[feature] Tune sqlite pragmas #1349

Merged
merged 8 commits into from
Jan 17, 2023
Merged

[feature] Tune sqlite pragmas #1349

merged 8 commits into from
Jan 17, 2023

Conversation

tsmethurst
Copy link
Contributor

@tsmethurst tsmethurst commented Jan 16, 2023

Description

If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements.

If this is a documentation change, please briefly describe what you've changed and why.

This pull request implements sqlite pragma tuning, based on #1247 but using the current config structure (since the other method has trouble reading environment variables for nested config values).

When sqlite is used, then the following pragmas will be used by default:

# String. SQLite journaling mode.
# SQLite only -- unused otherwise.
# See: https://www.sqlite.org/pragma.html#pragma_journal_mode
db-sqlite-journal-mode: "WAL"

# String. SQLite synchronous mode.
# SQLite only -- unused otherwise.
# See: https://www.sqlite.org/pragma.html#pragma_synchronous
db-sqlite-synchronous: "NORMAL"

# Byte size. SQlite cache size.
# SQLite only -- unused otherwise.
# See: https://www.sqlite.org/pragma.html#pragma_cache_size
db-sqlite-cache-size: "64MiB"

The PR also disables cache=shared when not running using an in-memory database. It's kept for in-memory for testing purposes.

I'm testing the PR on goblin.technology currently. It seems mostly ok, but i am getting some locked errors every couple minutes:

Jan 16 13:47:40 goblin.technology gotosocial[505318]: timestamp="16/01/2023 13:47:40.327" func=concurrency.(*WorkerPool).Queue.func1 level=ERROR type=worker.Worker[*media.ProcessingMedia] error="error loading media redacted: database is locked (5) (SQLITE_BUSY)" msg="message processing error"
Jan 16 13:54:02 goblin.technology gotosocial[505318]: timestamp="16/01/2023 13:54:02.676" func=concurrency.(*WorkerPool).Queue.func1 level=ERROR type=worker.Worker[messages.FromFederator] error="unexpected error: GetRemoteStatus: error populating status fields: populateStatusFields: error populating status mentions: populateStatusMentions: error creating mention: database is locked (5) (SQLITE_BUSY)" msg="message processing error"
Jan 16 13:58:19 goblin.technology gotosocial[505318]: timestamp="16/01/2023 13:58:19.197" func=concurrency.(*WorkerPool).Queue.func1 level=ERROR type=worker.Worker[messages.FromFederator] error="unexpected error: GetRemoteStatus: error populating status fields: populateStatusFields: error populating status mentions: populateStatusMentions: error creating mention: database is locked (5) (SQLITE_BUSY)" msg="message processing error"
Jan 16 13:59:38 goblin.technology gotosocial[505318]: timestamp="16/01/2023 13:59:38.835" func=concurrency.(*WorkerPool).Queue.func1 level=ERROR type=worker.Worker[messages.FromFederator] error="unexpected error: GetRemoteStatus: couldn't get status author: database error during dereferencing: GetRemoteAccount: error updating remoteAccount: database is locked (5) (SQLITE_BUSY)" msg="message processing error"

For more info on the above errors, see https://www.sqlite.org/wal.html#sometimes_queries_return_sqlite_busy_in_wal_mode

I believe they're being caused by messages being delivered to more than one account at a time, and running multiple concurrent processes to update the same entry. We will probably need to add some kind of locking here to fix this before merging this PR.... @NyaaaWhatsUpDoc did you also run into this problem when testing, or did it not show on a single-user instance?

closes #1335

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • 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.

@tsmethurst tsmethurst marked this pull request as draft January 16, 2023 14:03
@NyaaaWhatsUpDoc
Copy link
Member

NyaaaWhatsUpDoc commented Jan 17, 2023

regarding the DB locked errors, my solution has been setting the busy_timeout parameter. It accepts values in milliseconds, so I'd have a configuration parameter of type time.Duration that you then divide the value of by time.Millisecond. Then set the pragma with strconv.FormatInt() (uses int64 so no, albeit small, risk of value truncation)

@tsmethurst tsmethurst marked this pull request as ready for review January 17, 2023 12:28
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc merged commit 627b8ee into main Jan 17, 2023
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc deleted the sqlite branch January 17, 2023 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] Support WAL mode for sqlite databases.
2 participants