Revert "opt: reduce sqlite "database is locked" error (#1009)"#1053
Revert "opt: reduce sqlite "database is locked" error (#1009)"#1053looplj merged 1 commit intorelease/v0.9.xfrom
Conversation
This reverts commit f0e32b2.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request reverts a prior optimization aimed at reducing "database is locked" errors in SQLite by limiting database connections. The revert restores the connection settings to their default behavior, suggesting the previous change may not have been effective or introduced other issues. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reverts a change that was intended to prevent "database is locked" errors with SQLite by limiting the connection pool size to 1. While this limitation can impact performance by serializing database access, removing it without an alternative concurrency control mechanism (like enabling WAL mode) will likely cause the locking errors to reappear under concurrent write scenarios. This could lead to application instability. It would be beneficial to understand the reasoning behind this revert and what alternative strategy, if any, is being employed to handle SQLite concurrency.
I am having trouble creating individual review comments. Click here to see my feedback.
internal/server/db/ent.go (49-52)
Removing this connection limit for SQLite re-introduces a high risk of database is locked errors during concurrent write operations. These lines correctly serialized database access, which is a standard way to prevent this issue in SQLite when not using WAL mode.
If this revert is for performance reasons, a safer alternative would be to ensure WAL (Write-Ahead Logging) mode is enabled for all SQLite database connections (e.g., by adding _journal=WAL to the DSN or executing PRAGMA journal_mode=WAL; on connection). Without such a guarantee, removing this safeguard is likely to cause runtime failures.
|
close #1046 |
1 participant
This reverts commit f0e32b2.