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✅ All clear — no issues detected
Binary Size
Dependency ChangesNo dependency changes. govulncheck OutputBuild Info
History (1 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
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.
Description
Add a thread-safe Go config registry for the VFS extension, eliminating the need for
setenv()which is not thread-safe on Linux. Library users who don't know VFS config values at process startup can now safely configure each database connection at runtime via the registry API without callingos.Setenv.Key changes:
vfs_config.go):SetVFSConfig/GetVFSConfig/DeleteVFSConfigkeyed by database name, protected bysync.RWMutexwith defensive copies on set/get to prevent concurrent mutationopenMainDB()checks registry and creates a per-connectionReplicaClientwhen config specifiesreplica_url; cleaned up onVFSFile.Close()and onOpen()failureLitestreamVFSRegister()no longer requiresLITESTREAM_REPLICA_URL, enabling per-database config via registryGoLitestreamConfigure()for Python/C callers to set per-database config before opening VFS connectionsopenMainDB()returns a clear error if no replica client is available from either the VFS default or per-database configMotivation 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. The current VFS extension requires all config via env vars set before registration. URI query parameters are not viable without upstream changes topsanford/sqlite3vfs(follow-up issue).Fixes #1150
How Has This Been Tested?
go test -tags vfs -race— 7 new tests pass with race detector:TestVFSConfig_SetGet— basic set/get/deleteTestVFSConfig_ConcurrentAccess— 100 goroutines, race detectorTestVFSConfig_OverridesDefaults— config values override VFS defaultsTestVFSConfig_NilOptionalFields— nil field handlingTestVFSConfig_CopyOnSetAndGet— defensive copies prevent shared mutationTestVFSConfig_PerConnectionOverrides— per-db config applied inopenMainDB()TestVFS_NilClientReturnsError— clear error when no client configuredgo build -tags "SQLITE3VFS_LOADABLE_EXT vfs" ./cmd/litestream-vfs— loadable extension buildsTestVFSFile_PendingIndexRacerace onmain)Types of changes
Checklist
go fmt,go vet)go test ./...)