sqldb+docs: fix user pragma options for native SQL store, add mobile docs#10590
sqldb+docs: fix user pragma options for native SQL store, add mobile docs#10590ziggie1984 wants to merge 2 commits intolightningnetwork:masterfrom
Conversation
The SqliteConfig already supports user-configurable pragma options via the `db.sqlite.pragmaoptions` config flag, and the kvdb sqlite backend correctly applies them. However, the native SQL store (sqldb) was ignoring cfg.PragmaOptions when constructing the DSN, meaning user pragma settings only applied to the KV store but not to the native SQL tables. This is particularly important for mobile (Android/iOS) deployments where `temp_store=memory` must be set to avoid SQLITE_IOERR_GETTEMPPATH (error 6410) failures caused by the app sandbox preventing access to the system temporary directory.
Summary of ChangesHello @ziggie1984, 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 enhances LND's database handling by ensuring user-defined SQLite pragma options are consistently applied across all database backends, including the native SQL store. Concurrently, it introduces crucial documentation for mobile developers, explaining how to configure SQLite to avoid common 'disk I/O error' issues on Android and iOS devices, thereby improving the reliability of LND on these platforms. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
lightninglabs-deploy
added
the
severity-high
![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 correctly extends the user-configurable pragma options to the native SQL store, which was previously only available for the kvdb backend. The accompanying documentation for mobile-specific SQLite considerations is clear and will be very helpful for developers on those platforms. I have one minor suggestion to improve a comment to better align with the project's style guide.
| // Then we add any user specified pragma options. Note that these can | ||
| // be of the form: "key=value", "key(N)" or "key". |
There was a problem hiding this comment.
This comment explains what the code does, but the LND style guide prefers comments that explain the intention or why. A more descriptive comment would be helpful here to explain why we are adding user-specified pragma options, providing more context to future developers.
| // Then we add any user specified pragma options. Note that these can | |
| // be of the form: "key=value", "key(N)" or "key". | |
| // In addition to default pragmas, add any user-specified pragma options | |
| // to allow for platform-specific customizations (e.g. for mobile). |
References
- In-body comments should explain the intention of the code, not just what it does. The current comment explains what the code is doing, but not why it's important to add user-specified pragma options. (link)
Document that `temp_store=memory` is required when running LND on Android/iOS via gomobile. Without it, SQLite fails with SQLITE_IOERR_GETTEMPPATH (error 6410) because the app sandbox prevents access to the system temporary directory.
ziggie1984
force-pushed
the
fix-sqlite-temp-store
branch
from
2cd855e to
1ad66ef
Compare
🟠 PR Severity: HIGH
🟠 High (1 file)
🟢 Low (1 file)
AnalysisThis PR is classified as HIGH severity because it modifies While the diff is small (6 lines in the sqldb package), any changes to database-related code require review by a knowledgeable engineer to ensure correctness and avoid potential data integrity issues. The documentation changes in To override, add a |
|
closing in favour of #10565 |
2 participants
Summary
sqldb: pass user pragma options to native SQL store — The
db.sqlite.pragmaoptionsconfig flag was only being applied to the kvdb SQLite backend but not to the native SQL store (sqldb). This meant user-specified pragma options liketemp_store=memoryhad no effect on the native SQL tables.docs: add mobile SQLite considerations — Documents that
temp_store=memoryis required when running LND on Android/iOS via gomobile, explaining the root cause (SQLITE_IOERR_GETTEMPPATH, error code 6410).Fixes #10558
Context
On Android (and iOS), the app sandbox prevents SQLite from accessing the system temporary directory. When neutrino's block header and compact filter header sync run concurrently and trigger more complex database transactions, SQLite attempts to write temp files and fails with
SQLITE_IOERR_GETTEMPPATH(error 6410). This causes all retry attempts to exhaust, resulting indb tx retries exceededand sync getting permanently stuck around block ~123,000.The fix for the reporter's issue is to add the following to their LND config:
Since neutrino only uses the kvdb backend (not native SQL), this config option already works today without this PR. However, this PR ensures the pragma options are also correctly passed through to the native SQL store for completeness, and adds documentation to help future mobile developers avoid this issue.
Test plan
temp_store=memorytemp_store=memoryset via configgo build ./...passes for the sqldb module