Skip to content

feat: auto enable wal for sqlite#1633

Merged
looplj merged 1 commit into
unstablefrom
dev-tmp
May 10, 2026
Merged

feat: auto enable wal for sqlite#1633
looplj merged 1 commit into
unstablefrom
dev-tmp

Conversation

@looplj

@looplj looplj commented May 10, 2026

Copy link
Copy Markdown
Owner

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request enables Write-Ahead Logging (WAL) mode by default for SQLite databases to improve concurrency. It introduces a disable_sqlite_auto_wal configuration option and a helper function to automatically append the WAL pragma to the DSN if it is not already present. Feedback was provided to make the DSN check case-insensitive to prevent duplicate pragmas when user-provided strings use different casing.

Comment thread internal/server/db/ent.go
}
switch dialect {
case "sqlite3", "sqlite":
if !strings.Contains(dsn, "journal_mode") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The check for journal_mode is case-sensitive. Since SQLite pragmas and connection string parameters are often treated as case-insensitive by drivers (e.g., JOURNAL_MODE=WAL), this function might incorrectly append a duplicate journal_mode pragma if the user provided one with different casing. Using a case-insensitive check is more robust.

Suggested change
if !strings.Contains(dsn, "journal_mode") {
if !strings.Contains(strings.ToLower(dsn), "journal_mode") {

@greptile-apps

greptile-apps Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR auto-enables WAL (Write-Ahead Logging) mode for SQLite connections by injecting _pragma=journal_mode(WAL) into the DSN at startup, and adds a disable_sqlite_auto_wal escape hatch for users who want to opt out. The install scripts are also updated to bake WAL directly into the generated DSN for new deployments.

  • ensureSQLiteWAL is applied to both the master and read-replica DSNs; it detects an existing journal_mode pragma before appending to avoid duplication.
  • A new DisableSQLiteAutoWAL bool config field (with Viper default, YAML/JSON tags, and example config entry) lets operators opt out without touching the DSN.
  • llm/transformer/ollama/outbound.go contains an unrelated struct-tag alignment fix bundled into this PR.

Confidence Score: 5/5

Safe to merge; the WAL injection is guarded correctly and the opt-out flag is wired end-to-end.

The core logic is a pure string-manipulation helper that modifies only the DSN before handing it to the existing openDB function — no schema changes, no migration changes. All existing code paths are preserved, and the new flag defaults to false so behaviour is unchanged for users who do not configure it.

internal/server/db/ent.go — the journal-mode detection misses the _journal= shorthand, though this is an edge case not present in the project's own DSN patterns.

Important Files Changed

Filename Overview
internal/server/db/ent.go Adds ensureSQLiteWAL helper that injects WAL mode into SQLite DSNs; also applies it to the read-replica DSN. Detection of existing journal-mode settings misses the _journal= shorthand.
internal/server/db/config.go Adds DisableSQLiteAutoWAL bool field; struct tag alignment is broken relative to surrounding fields (pre-existing thread).
conf/conf.go Registers db.disable_sqlite_auto_wal default (false) with Viper; straightforward and correct.
deploy/install.sh Adds _pragma=journal_mode(WAL) to the generated SQLite DSN so new installs get WAL even without the auto-injection path.
deploy/install.ps1 Same WAL DSN update as install.sh, mirrored for the Windows installer.
llm/transformer/ollama/outbound.go Formatting-only: aligns struct tags in ChatMessage. No logic change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[NewEntClient called] --> B[ensureSQLiteWAL\nmaster DSN]
    B --> C{DisableSQLiteAutoWAL?}
    C -- true --> D[return DSN unchanged]
    C -- false --> E{dialect == sqlite3\nor sqlite?}
    E -- no --> D
    E -- yes --> F{DSN contains\n'journal_mode'?}
    F -- yes --> D
    F -- no --> G{DSN contains '?'}
    G -- yes --> H[append &_pragma=journal_mode WAL]
    G -- no --> I[append ?_pragma=journal_mode WAL]
    H --> J[openDB with WAL DSN]
    I --> J
    D --> J
    J --> K{ReadReplica.DSN set?}
    K -- yes --> L[ensureSQLiteWAL\nreplica DSN]
    L --> M[openDB replica]
    M --> N[newRouterDriver\nmaster + replica]
    K -- no --> O[entsql.OpenDB master only]
    N --> P[ent.NewClient]
    O --> P
Loading

Reviews (2): Last reviewed commit: "feat: auto enable wal for sqlite" | Re-trigger Greptile

Comment thread internal/server/db/ent.go Outdated
Comment thread internal/server/db/ent.go Outdated
Comment thread internal/server/db/config.go
@looplj looplj merged commit 61e00d3 into unstable May 10, 2026
4 checks passed
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