Skip to content

Default SQLite connections to WAL - #405

Merged
JanJakes merged 6 commits into
trunkfrom
wal
Jun 19, 2026
Merged

JanJakes merged 6 commits into
trunkfrom
wal

Conversation

@JanJakes

@JanJakes JanJakes commented May 26, 2026 •

Copy link
Copy Markdown
Member

What changed

Journal mode:

  • Default new SQLite connections to journal_mode = WAL.
  • Skip defaulting to WAL when unavailable (e.g., on network filesystems).

Synchronous flag:

  • Apply synchronous = NORMAL when WAL is used.
  • Otherwise, use the SQLite default.

Other changes:

  • Wire the SQLITE_JOURNAL_MODE override through the WordPress plugin connection paths.
  • Accept journal_mode and synchronous in the WP_PDO_MySQL_On_SQLite driver options.
  • Add connection setup tests for the defaults, overrides, invalid values, and the WAL fallback.

Why

WAL improves read/write concurrency for SQLite-backed WordPress installs, and synchronous = NORMAL avoids frequent sync to the main database. In WAL mode, NORMAL is safe. From the SQLite docs:

WAL mode is safe from corruption with synchronous=NORMAL, and probably DELETE mode is safe too on modern filesystems. WAL mode is always consistent with synchronous=NORMAL, but WAL mode does lose durability. A transaction committed in WAL mode with synchronous=NORMAL might roll back following a power loss or system crash. Transactions are durable across application crashes regardless of the synchronous setting or journal mode.

The synchronous=NORMAL setting provides the best balance between performance and safety for most applications running in WAL mode. You lose durability across power lose with synchronous NORMAL in WAL mode, but that is not important for most applications. Transactions are still atomic, consistent, and isolated, which are the most important characteristics in most use cases.

Benchmark

A local benchmark ran a WordPress-like 90% read / 10% write workload against the same database file with 4, 8, and 16 concurrent workers, comparing the trunk defaults with WAL + NORMAL (median of 3 runs, PHP 8.5.5, SQLite 3.53.0).

Workers Config Ops/s p50 p95 p99
4 default 4,985 0.21 ms 3.94 ms 10.2 ms
4 WAL + NORMAL 15,982 (3.2×) 0.19 ms 0.73 ms 1.2 ms
8 default 5,249 0.24 ms 4.22 ms 22.6 ms
8 WAL + NORMAL 25,371 (4.8×) 0.20 ms 1.02 ms 1.5 ms
16 default 5,226 0.25 ms 10.25 ms 59.0 ms
16 WAL + NORMAL 30,300 (5.8×) 0.22 ms 1.43 ms 4.0 ms

This was run on an M4 MacBook Pro Max.

@JanJakes
JanJakes force-pushed the wal branch 7 times, most recently from 52b2201 to e5f3c64 Compare June 12, 2026 13:15
JanJakes added 4 commits June 12, 2026 15:18
Configure new SQLite connections to use WAL by default and apply
synchronous=NORMAL when WAL is the effective journal mode. Keep the
explicit SQLITE_JOURNAL_MODE override available for WordPress plugin
connections, including initial installation.
WAL can fail to engage in some environments, e.g., on network filesystems
or when the WAL sidecar files cannot be created. In that case, setting the
journal mode throws, which would newly fail connections that worked before
WAL became the default. Keep the database's current journal mode when the
WAL default fails, but let explicitly configured modes surface the error.

Also include the SQLite connection setup in the installation error
handling, so a connection failure dies gracefully during WordPress
installation instead of causing a fatal error.
PRAGMA synchronous accepts integers from 0 to 3 as equivalents of the
keyword values, and constants like SQLITE_SYNCHRONOUS are likely to be
defined as integers. Map integer values to the corresponding keywords
instead of silently ignoring them.
The WAL default applies to all WP_SQLite_Connection consumers, but only
the WordPress plugin paths supported overriding it. Accept journal_mode
and synchronous in the WP_PDO_MySQL_On_SQLite driver options so the
PDO API consumers can configure them as well.
@JanJakes
JanJakes marked this pull request as ready for review June 12, 2026 13:37
@JanJakes
JanJakes requested a review from adamziel June 12, 2026 13:37
// Otherwise, use SQLite's default value.
$synchronous = $options['synchronous'] ?? null;
if ( null === $synchronous && 'WAL' === $effective_journal_mode ) {
$synchronous = self::DEFAULT_SQLITE_WAL_SYNCHRONOUS;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is super confusing. It's SYNCHRONOUS but it's NORMAL. Let's just stick to SQLite terminology and maybe even let go of constants:

Suggested change
$synchronous = self::DEFAULT_SQLITE_WAL_SYNCHRONOUS;
$synchronous = 'NORMAL';

Or even go super simple:

Suggested change
$synchronous = self::DEFAULT_SQLITE_WAL_SYNCHRONOUS;
$this->query( 'PRAGMA synchronous = NORMAL' );

The comment above explains what we're doing, I assume it's because the logic is so opaque here. I'd love to, instead, understand why we're doing this. Document the NORMAL mode inline and explain why we're defaulting to it when WAL is enabled and not otherwise and what is SQLite default value?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@adamziel Good point, thanks! Improved in a15ecdb.

Drop the single-use DEFAULT_SQLITE_JOURNAL_MODE and
DEFAULT_SQLITE_WAL_SYNCHRONOUS constants in favor of inline literals, and
replace the synchronous comment with one that explains why we default to
NORMAL under WAL: SQLite's default is FULL (an fsync per commit), while
NORMAL avoids that cost and stays corruption-safe in WAL mode, a guarantee
that does not hold for rollback journal modes.
Comment on lines +162 to 164
if ( in_array( $synchronous, self::SQLITE_SYNCHRONOUS_SETTINGS, true ) ) {
$this->query( 'PRAGMA synchronous = ' . $synchronous );
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if ( in_array( $synchronous, self::SQLITE_SYNCHRONOUS_SETTINGS, true ) ) {
$this->query( 'PRAGMA synchronous = ' . $synchronous );
}
if ( in_array( $synchronous, self::SQLITE_SYNCHRONOUS_SETTINGS, true ) ) {
$this->query( 'PRAGMA synchronous = ' . $synchronous );
} else if ( isset( $synchronous ) ) {
throw new PDOException( sprintf( '%s is not a valid `synchronous` PRAGMA value. ' ) );
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's not accept invalid option values.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ditto for the journal mode, let's not skip that silently.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@adamziel Fixed in 8de3d1a. I used InvalidArgumentException as in the case of missing path, but not sure which is better—vs PDOException.

Previously, an invalid explicitly provided journal mode or synchronous
value was silently ignored, leaving the connection on a different setting
than requested. Throw an InvalidArgumentException instead, consistent with
the existing validation of the "path" option, so misconfiguration surfaces
rather than passing unnoticed.
@JanJakes
JanJakes merged commit 299d62d into trunk Jun 19, 2026
23 checks passed
@JanJakes
JanJakes deleted the wal branch June 19, 2026 18:05
@JanJakes JanJakes mentioned this pull request Jun 19, 2026
JanJakes added a commit that referenced this pull request Jun 19, 2026
## Release `3.0.0-rc.5`

Version bump and changelog update for release `3.0.0-rc.5`.

**Changelog draft:**
* Default SQLite connections to WAL
([#405](#405))

**Full changelog:**
v3.0.0-rc.4...release/v3.0.0-rc.5

## Next steps

1. **Review** the changes in this pull request.
2. **Push** any additional edits to this branch (`release/v3.0.0-rc.5`).
3. **Merge** this pull request to complete the release.

Merging will automatically build the plugin ZIP and create a [GitHub
release](https://github.com/WordPress/sqlite-database-integration/releases).

> [!NOTE]
> This is a **pre-release**. It will not be deployed to
[WordPress.org](https://wordpress.org/plugins/sqlite-database-integration/).
@adamziel adamziel mentioned this pull request Jul 1, 2026
@JanJakes JanJakes added this to the Release 3.0 milestone Aug 12, 2026
wojtekn added a commit to Automattic/studio that referenced this pull request Sep 23, 2026
## Related issues

- Fixes #4843

## How AI was used in this PR

Claude Code traced the report through the bundled SQLite driver and
Studio's subprocess paths, and wrote the change. I reviewed it and
pushed back on an earlier, larger version — see the scope note below.

## Proposed Changes

#4819 fixed `Cannot escape data without an active database connection`
by converting a site's SQLite database out of WAL journal mode at site
start, but gated that to the Playground runtime on the reasoning that
Native PHP "runs the platform's own SQLite against real OS locks."

That gate leaves the default configuration unprotected:
`getSiteRuntime()` returns Native PHP when a site has no explicit
runtime, so **most sites never run the conversion**. Measured on trunk,
same site, same database — the runtime alone decides:

| Runtime | `journal_mode` after start |
| --- | --- |
| Playground (`sandbox`) | `delete` |
| Native PHP (default) | `wal` |

Real OS locks avoid PHP-WASM's emulated-lock problem, but they do not
make WAL safe here, because the contention is between *processes*:
Studio opens one site's database from several at once — an export spawns
a WP-CLI process per table while the site server keeps serving — and
those connections contend for WAL's `-shm` shared-memory index. Past the
driver's 10s busy timeout the connection fails, and the driver swallows
it into the misleading fatal above. That matches the report's
observation that retried pushes failed at a *different* internal step
each time: per-connection contention, not one stuck lock.

This removes the runtime gate so both runtimes get the conversion that
#4819 already proved out.

### Scope note for reviewers

An earlier version of this PR also pinned `SQLITE_JOURNAL_MODE=DELETE`
in `wp-config.php` so subprocesses could never re-enter WAL. **That was
dropped.** WAL is the upstream default for good reason —
WordPress/sqlite-database-integration#405 measured 3.2×–5.8× higher
throughput and a p99 drop from 59ms to 4ms under concurrent load — and
pinning DELETE globally would forfeit that on every platform to address
a problem only reported on Windows.

So this PR deliberately does **not** settle whether Studio should use
WAL. It only makes the existing behavior consistent across runtimes. The
underlying question — whether Studio's multi-process access pattern is
better fixed by serializing DB access, raising the busy timeout, or a
Windows-scoped change — is worth answering upstream, where
WordPress/sqlite-database-integration#443 (`SET` no longer taking a
write lock, opened from STU-1821) fixed the last contention bug of this
family.

⚠️ The fatal in #4843 is Windows-specific and I could not reproduce it
on macOS. This removes the mechanism on the runtime that was missing it;
**confirming the crash is gone needs a Windows push.**

## Testing Instructions

Check a site's mode (read-only, safe while running):

```
node -e "const{DatabaseSync}=require('node:sqlite');const d=new DatabaseSync(process.argv[1],{readOnly:true});console.log(d.prepare('PRAGMA journal_mode').get().journal_mode);d.close();" ~/Studio/<site>/wp-content/database/.ht.sqlite
```

1. On `trunk`, start a **native** (default) site and confirm it reads
`wal`, with `.ht.sqlite-wal` / `.ht.sqlite-shm` present.
2. On this branch, `npm run cli:build`, stop the site, then start it
again.
3. It should now read `delete`, with both sidecar files gone.
4. Repeat with `--runtime sandbox` to confirm Playground behavior is
unchanged.

Verified on macOS against one site switched between both runtimes:
Playground `delete` (unchanged), native `wal` → `delete`. Forcing a
stopped database to WAL and starting it confirmed the conversion is what
moves it.

Note when testing in dev mode: stop the site first (the conversion
intentionally swallows errors if the database is locked, so a running
site silently shows no change), quit any running `npm start` (it uses
the same dev CLI build), and let `npm run cli:build` finish before
launching — the CLI daemon loads its bundle at startup.

## Pre-merge Checklist

- [x] Have you checked for TypeScript, React or other console errors?

Lint, `npm run typecheck`, and the affected unit tests pass. The test
asserting the native runtime was skipped is inverted to assert the
conversion now runs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

2 participants