-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refactor(database): Refactor database connection management to avoid sqlite database is locked #1303
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
base: main
Are you sure you want to change the base?
Conversation
…t read/write separation This commit introduces a new `Connection` struct to abstract underlying database read/write connections. Multiple files were modified to replace the original `*gorm.DB` instances with the new `Connection` instances, and related function calls were adjusted to accommodate the new connection management approach. Key changes include: - Added `Connection` struct and its methods in `internal/model/connection.go` to implement read/write connection separation - Modified database initialization logic in `internal/bootstrap/db.go`, `internal/db/db.go` etc. to use new `Connection` instances instead of original `*gorm.DB` instances - Updated database operation functions in `internal/db/meta.go`, `internal/db/searchnode.go` and other files to properly use `Connection` instances for read/write operations - Added new files `internal/bootstrap/dbengine/mysql.go`, `internal/bootstrap/dbengine/postgres.go` and `internal/bootstrap/dbengine/sqlite.go` implementing connection creation logic for MySQL, PostgreSQL and SQLite respectively These changes improve code maintainability and extensibility while laying the foundation for future performance optimizations.
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.
Pull Request Overview
This PR refactors database connection management to optimize SQLite performance and resolve "database is locked" errors by implementing separate read and write connections based on SQLite best practices from "SQLite For Server" guidance.
- Introduces a new
Connectionstruct that abstracts read/write database access - Implements optimized SQLite connection management with separate read/write connections and performance pragmas
- Refactors all database operations to use the new connection abstraction pattern
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/model/connection.go | New Connection struct providing read/write database abstraction |
| internal/bootstrap/dbengine/*.go | Database engine implementations for SQLite, MySQL, and PostgreSQL with optimized connection settings |
| internal/bootstrap/db.go | Updated database initialization to use new connection management |
| internal/db/db.go | Refactored to use Connection struct instead of single gorm.DB instance |
| internal/db/*.go | Updated all database operations to use rwDb.R() for reads and rwDb.W() for writes |
| internal/search/db/init.go | Updated search indexing to use new connection pattern |
| internal/op/storage_test.go | Updated test to use new SQLite connection creation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
已确认并修复了Copilot查出的所有问题。AI查的很准确。
|
|
现在该PR的情况如何?是否仍有需要修改的代码或描述? |
|
什么情况下会出现数据库锁定? |
目前遇到的可能在构建索引的时候点击其他页面?不明。但文档中确实有对此事的记录:
另外的,我在点击存储的时候曾也触发过数据库锁定的情况。 |
|
所以为什么要把所有数据库都改了,改mysql和pgsql对代码结构有好处么? |
只是考虑到如果不统一的话,每个非SQLite的都需要写额外逻辑(if xxx == SQLite then XXXX ) 毕竟实际上使用的时候,对于SQLite无论如何都要明确区分是读DB或者写DB。 在这种情况下实际上其他SQL等于多了一层无用封装。毕竟他们的读写DB是同一个。 当然,有个更简单的方案是直接限制SQLite只允许一个连接,当然这会让目前本就性能不咋好的SQLite雪上加霜。 |
|
已修正新代码无法编译的问题。 另:如果不修改所有的数据库,有没有更好的推荐方案? |
根据
那就说明
所以不需要了。 |
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.
Pull Request Overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # internal/db/sharing.go
|
已更新最新代码并修复了冲突。考虑到数据库部分为公共部分,若没有问题申请尽快进行合并,否则只要新的代码修改了db或者增加了db,该PR就需要一直跟进、编译、申请审查、测试,过于费时费力。 |
|
(btw,我并没有合并权限) |
|
@PaienNate 根据团队成员的讨论,此PR必须经过完整测试才能进入合并流程。目前暂无法合并,你是否可以协助提供更多测试结果 |
考虑到我对项目熟悉程度不足,请问我如何提供协助? |
| sqlDB.SetMaxOpenConns(100) | ||
| sqlDB.SetMaxIdleConns(10) | ||
| sqlDB.SetConnMaxLifetime(0) |
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.
这里连接池大小我认为最好写到config里,下面pg也是
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.
收到,有机会会研究一下怎么从配置文件读取。


refactor(database): 基于SQLite For Server对数据库代码进行调优
Description / 描述
该PR旨在基于 https://kerkour.com/sqlite-for-servers 的方案,以解决sqlite下 database is locked的问题。
它参考了 https://github.com/bihe/monorepo/blob/477e534bd4c0814cdca73fea774b518148cebd3f/pkg/persistence/sqlite.go#L59 并做出部分修改,以适配该项目。它同时兼容了litestream对其进行备份(来源monorepo库,该库使用apache-2.0开源协议)。
Motivation and Context / 背景
在另外一个开源项目(sealdice-core)中由于使用SQLite时容易发生database is locked的情况,在调研后在对应项目予以修复。观察到OpenList有相同问题,故而移植代码核心方案到OpenList。
解决问题的核心:
How Has This Been Tested? / 测试
该方案在另外一个开源项目中测试过可行。 但不幸的是,我确实没有对我的代码进行令人满意的测试。我仅仅运行了storage_test并通过了测试,但我想这远远不够。
Checklist / 检查清单
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签(如无权限或需要的标签不存在,请在描述中说明,管理员将后续处理)。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。
我已相应更新了相关仓库(若适用)。