Skip to content

fix: default sqlite journal mode to WAL#37758

Closed
silverwind wants to merge 1 commit into
go-gitea:mainfrom
silverwind:fix-parallel-issue-test-flake
Closed

fix: default sqlite journal mode to WAL#37758
silverwind wants to merge 1 commit into
go-gitea:mainfrom
silverwind:fix-parallel-issue-test-flake

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented May 18, 2026

DELETE journal mode serializes readers against writers on the same file. Concurrent issue creation plus the async indexer reading in the background can exceed the 5s busy timeout and return SQLITE_BUSY. WAL lets readers and one writer coexist.

Also fix testAPICreateIssueParallel: wg.Done() lived inside the subtest, so a t.FailNow skipped it and hung wg.Wait until the job timeout (the symptom in https://github.com/go-gitea/gitea/actions/runs/26009967009/job/76448592797). Move it to a defer on the outer goroutine. The obsolete 100ms sleep workaround for the old mattn unlock_notify bug is removed — modernc isn't affected.

Drawbacks of WAL as default for existing installs

  • backups via cp gitea.db need to include -wal/-shm or use sqlite3 .backup
  • WAL needs mmap, so NFS/SMB-hosted DBs are unsupported
  • once opened in WAL the DB persists in WAL; rollback needs PRAGMA journal_mode=DELETE

The example config notes when to opt back to DELETE.


This PR was written with the help of Claude Opus 4.7

DELETE journal mode serializes every read against every write on the
same file, so concurrent issue creation with the async issue indexer
reading in the background can exceed the 5s busy timeout and return
SQLITE_BUSY. WAL lets readers and one writer coexist, removing the
contention.

Also fix `testAPICreateIssueParallel`: `wg.Done()` lived inside the
subtest closure, so a `t.FailNow` from `MakeRequest` skipped it and
hung `wg.Wait` until the 50min job timeout. Moving it to a deferred
call on the outer goroutine makes any future flake fail fast. The
obsolete `time.Sleep(100ms)` workaround for the old mattn cgo
`unlock_notify` bug is gone too.

Drawbacks of WAL as default for existing installs:
- backups via `cp gitea.db` no longer capture consistent state;
  must include `-wal`/`-shm` siblings or use `sqlite3 .backup`
- WAL needs `mmap`, so NFS/SMB-hosted databases are unsupported
- once a DB is opened in WAL it persists; rollback needs an
  explicit `PRAGMA journal_mode=DELETE`

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 18, 2026
@github-actions github-actions Bot added the docs-update-needed The document needs to be updated synchronously label May 18, 2026
@bircni
Copy link
Copy Markdown
Member

bircni commented May 18, 2026

Is it compatible between wal and non wal?

@lunny
Copy link
Copy Markdown
Member

lunny commented May 18, 2026

But if user switch to non-wal, the problem is still there.

Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. default mode shouldn't be changed if there is no strong requirement from end users
  2. the comments and Sleep should be kept because the mattn driver is still there
  3. the test case should use "assert" but not "require" in the go routine

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented May 18, 2026

Alternatively, we could just make only the integration tests use to WAL for stability.

My impression is that WAL is safe and a perf improvement in all scenarios, the more busy the database is, the more improvement.

@wxiaoguang
Copy link
Copy Markdown
Contributor

My impression is that WAL is safe and a perf improvement in all scenarios, the more busy the database is, the more improvement.

Show me that WAL really reduces the CI time or at least it reduces the time for the go routines.

@silverwind
Copy link
Copy Markdown
Member Author

Original motivivation here was to fix the test flake seen in https://github.com/go-gitea/gitea/actions/runs/26009967009/job/76448592797, the change here makes it not hang anymore. Using WAL should stabilize the test, if we stay on DELETE, another fix needs to be found.

@wxiaoguang
Copy link
Copy Markdown
Contributor

the change here makes it not hang anymore.

It is just your hallucination


Original motivivation here was to fix the test flake seen in https://github.com/go-gitea/gitea/actions/runs/26009967009/job/76448592797,

The root cause of "hang" is the incorrect "require" usages in the testing helper functions.

-> chore: fix tests #37760

@silverwind
Copy link
Copy Markdown
Member Author

Keep it for now.

@silverwind silverwind closed this May 18, 2026
@silverwind silverwind deleted the fix-parallel-issue-test-flake branch May 18, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-update-needed The document needs to be updated synchronously lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants