Conversation
benbjohnson
requested changes
Feb 23, 2026
Owner
benbjohnson
left a comment
There was a problem hiding this comment.
I'm okay with setting parameters in the URL but I think the PRAGMA part is overkill. These should all be set at initialization time and PRAGMAs can be used anytime.
661885e to
6929314
Compare
PR Build Metrics
Binary Size
Dependency ChangesAdded:
Removed:
govulncheck OutputBuild Info
History (4 previous)
🤖 Updated on each push. |
Replace the requirement for setenv() (not thread-safe on Linux) with a Go config registry that allows per-database VFS configuration at runtime. Library users who don't know config values at process startup can now safely configure each database connection without calling os.Setenv. Key changes: - Add VFSConfig registry (SetVFSConfig/GetVFSConfig/DeleteVFSConfig) keyed by database name with RWMutex protection - openMainDB() checks registry and creates per-connection ReplicaClient when config specifies a replica_url - Per-connection clients are cleaned up on VFSFile.Close() - LitestreamVFSRegister() no longer requires LITESTREAM_REPLICA_URL env var, enabling per-database config via registry - Add GoLitestreamConfigure C export for Python/C callers - Add PRAGMAs: litestream_poll_interval (R/W), litestream_cache_size (R/W), litestream_hydration_enabled (R/O), litestream_log_level (R/W), litestream_replica_url (R/O) - Protect PollInterval/CacheSize PRAGMA access with mutex Fixes #1150
Address Ben's review feedback: PRAGMAs are overkill for config that should be set at initialization time. Also fix issues found during code review. Changes: - Remove all new PRAGMAs (poll_interval, cache_size, hydration_enabled, log_level, replica_url) - Return defensive copies from SetVFSConfig/GetVFSConfig to prevent concurrent mutation of shared config - Add nil client check in openMainDB to fail early with clear error - Close per-connection client on f.Open() failure to prevent leaks - Revert unnecessary mutex addition in monitorReplicaClient - Add TestVFSConfig_CopyOnSetAndGet and TestVFS_NilClientReturnsError
6929314 to
5f029df
Compare
Expose SQLite URI parameters to the Litestream VFS via an internal sqlite3vfs fork, then parse supported VFS settings from database URIs. This lets loadable-extension users set options such as poll_interval without PRAGMA support.
Collaborator
Author
|
Updated for re-review:
Latest commit is |
Close per-connection replica clients if Init fails so URI-based connections do not leak backend resources.
Replace the internal sqlite3vfs fork with the upstream module import and a temporary replace to the URI-support branch from psanford/sqlite3vfs#18. Add an end-to-end URI replica_url test and keep test fault injection active for FilenameOpener-based opens.
benbjohnson
approved these changes
Apr 21, 2026
Remove the temporary replace directive and import the corylanou/sqlite3vfs fork directly until upstream URI filename support is merged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge Blocker
This PR currently depends directly on
github.com/corylanou/sqlite3vfs v0.0.0-20260421183553-1656bf6ea7f7, from branchlitestream-uri-filename-supportin the fork. This is intentional for review/CI and avoids anyreplacedirective.Do not merge this PR until psanford/sqlite3vfs#18 is merged upstream and this dependency/import path is switched back to
github.com/psanford/sqlite3vfs.Description
Add thread-safe VFS configuration for per-database runtime settings and URI parameters. This removes the need for loadable-extension users to rely on process-wide
setenv()after startup, and lets callers configure options such asreplica_urlandpoll_intervaldirectly in the SQLite database URI.Key changes:
vfs_config.go):SetVFSConfig/GetVFSConfig/DeleteVFSConfigkeyed by database name, protected bysync.RWMutexwith defensive copies on set/get to prevent concurrent mutation.github.com/corylanou/sqlite3vfsdirectly from branchlitestream-uri-filename-supportuntil add URI parameter support via optional FilenameOpener interface psanford/sqlite3vfs#18 is available upstream. There is intentionally noreplacedirective.internal/sqlite3vfspackage from this PR.GoLitestreamConfigure()and URI config now share the sameVFSConfig.Set()validation path.openMainDB()creates a per-connectionReplicaClientwhen config specifiesreplica_url; clients are cleaned up onVFSFile.Close()and onOpen()failure.LitestreamVFSRegister()no longer requiresLITESTREAM_REPLICA_URL, enabling per-database config via registry or URI parameters.Motivation and Context
setenv()is not thread-safe on Linux. Library users who don't know VFS config values at process startup cannot safely callos.Setenvlater. This PR adds registry and URI-based configuration so callers can configure each database connection without mutating process-wide environment variables.Fixes #1150
Fixes #1231
How Has This Been Tested?
go test -tags vfs -count=1 .go test -tags vfs -count=1 ./cmd/litestream-vfsgo test github.com/corylanou/sqlite3vfsgo vet -tags vfs . ./cmd/litestream-vfsgo build -tags "SQLITE3VFS_LOADABLE_EXT vfs" ./cmd/litestream-vfsgo-imports-repo,go-vet-repo-mod,go-staticcheck-repo-modTypes of changes
Checklist
go fmt,go vet)