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

Replace mattn/sqlite with ncruces/sqlite (attempt two) #629

Open
wants to merge 57 commits into
base: master
Choose a base branch
from

Conversation

mtlynch
Copy link
Owner

@mtlynch mtlynch commented Jan 1, 2025

Current status (2025-01-24): Testing revealed a bunch of valid bugs in our SQLite queries that mattn/sqlite was papering over, so need to re-test after fixing those.

Test work items

  • - Pull migration 011 into its own PR
  • Investigate the index issue

Test with no litestream, no volume ✔️

Worked fine on fly.io

Test with fly volume, no litestream ❌

  • Fails with OOM after I try to read a 1.23 GB file
  • Updated to latest ncruces library, and now I get
2025-01-24T16:40:48.438 app[32871252ce9385] iad [info] 2025/01/24 16:40:48 /app/store/sqlite/entries.go:299: deleting entry FupVMDJnYD

2025-01-24T16:41:48.443 app[32871252ce9385] iad [info] 2025/01/24 16:41:48 /app/store/sqlite/entries.go:311: delete from entries_data table failed, aborting transaction: sqlite3: database is locked 
  • The same file uploads and deletes fine if I run locally

Test with litestream enabled and fly volume

Not started

Base automatically changed from entry-file to master January 1, 2025 20:43
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.

1 participant