diff --git a/internal/config/loader.go b/internal/config/loader.go index 9b2d2397d..3ceda2a4a 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "time" "github.com/smart-mcp-proxy/mcpproxy-go/internal/secret" @@ -432,6 +433,19 @@ func atomicWriteFile(path string, data []byte, perm os.FileMode) error { return atomicWriteFileMode(path, data, perm, false) } +// skipWriteSync drops the fsync from atomicWriteFileMode. Only tests set it, +// through DisableWriteSyncForTesting; production writes always sync. +var skipWriteSync atomic.Bool + +// DisableWriteSyncForTesting makes config file writes skip the fsync before +// the atomic rename for the rest of the process. The written bytes are still +// visible to every reader in the process; only crash durability is lost, which +// no unit test exercises. Call it from TestMain in packages whose tests write +// configs heavily: FlushFileBuffers on the Windows CI runner costs tens of +// milliseconds per write and dominated internal/runtime's test time. Never +// call it from production code. +func DisableWriteSyncForTesting() { skipWriteSync.Store(true) } + // atomicWriteFileMode is atomicWriteFile; with exactMode the temp file is // chmod-ed to perm before the rename, so the result carries perm itself and // not perm filtered by the process umask (which OpenFile applies). @@ -473,8 +487,10 @@ func atomicWriteFileMode(path string, data []byte, perm os.FileMode, exactMode b } // Fsync to ensure data is on disk - if err := tmpFile.Sync(); err != nil { - return fmt.Errorf("failed to sync temp file: %w", err) + if !skipWriteSync.Load() { + if err := tmpFile.Sync(); err != nil { + return fmt.Errorf("failed to sync temp file: %w", err) + } } // Close temp file diff --git a/internal/runtime/main_test.go b/internal/runtime/main_test.go new file mode 100644 index 000000000..95580a4bd --- /dev/null +++ b/internal/runtime/main_test.go @@ -0,0 +1,29 @@ +package runtime + +import ( + "os" + "testing" + + "go.etcd.io/bbolt" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" +) + +// TestMain turns off fsync for config writes and bbolt commits. Nearly every +// test here builds a Runtime, which writes the config file, commits to +// config.db and creates a Bleve index; on the windows-latest runner those +// flushes cost tens of milliseconds each and the package took ~470s against +// go test's default 10m timeout (TestConfigCommit_CommitPathsConverge alone +// ran 92s). Skipping the flush changes nothing a test can observe in-process +// (writes stay visible to every reader); only crash durability is lost, and no +// test here simulates a crash. +// +// bbolt.DefaultOptions is what Bleve's scorch index copies for its root.bolt; +// storage opens config.db with its own options, hence the separate switch. +func TestMain(m *testing.M) { + config.DisableWriteSyncForTesting() + storage.DisableSyncForTesting() + bbolt.DefaultOptions.NoSync = true + os.Exit(m.Run()) +} diff --git a/internal/storage/bbolt.go b/internal/storage/bbolt.go index 1842fdbc8..44786d622 100644 --- a/internal/storage/bbolt.go +++ b/internal/storage/bbolt.go @@ -9,6 +9,7 @@ import ( "io" "os" "path/filepath" + "sync/atomic" "time" "go.etcd.io/bbolt" @@ -36,6 +37,19 @@ type BoltDB struct { logger *zap.SugaredLogger } +// noSync opens databases with bbolt's NoSync. Only tests set it, through +// DisableSyncForTesting; production databases always sync their commits. +var noSync atomic.Bool + +// DisableSyncForTesting makes every database opened afterwards in this +// process skip the fsync on commit (bbolt Options.NoSync). Committed data is +// still visible to every reader of the open database; only crash durability is +// lost, which no unit test exercises. Call it from TestMain in packages whose +// tests open many databases or commit heavily: an fsync per commit on the +// Windows CI runner dominated internal/runtime's test time. Never call it from +// production code. +func DisableSyncForTesting() { noSync.Store(true) } + // NewBoltDB creates a new BoltDB instance func NewBoltDB(dataDir string, logger *zap.SugaredLogger) (*BoltDB, error) { dbPath := filepath.Join(dataDir, "config.db") @@ -45,6 +59,7 @@ func NewBoltDB(dataDir string, logger *zap.SugaredLogger) (*BoltDB, error) { // owner-only (0600). db, err := bbolt.Open(dbPath, 0600, &bbolt.Options{ Timeout: 10 * time.Second, + NoSync: noSync.Load(), }) if err != nil { logger.Warnf("Failed to open database on first attempt: %v", err) diff --git a/internal/storage/main_test.go b/internal/storage/main_test.go new file mode 100644 index 000000000..61dc03395 --- /dev/null +++ b/internal/storage/main_test.go @@ -0,0 +1,15 @@ +package storage + +import ( + "os" + "testing" +) + +// TestMain opens every test database with bbolt NoSync: an fsync per commit +// made this package take ~150s on the windows-latest runner. Committed data +// stays visible to every reader of the open database; only crash durability is +// lost, and no test here simulates a crash. See DisableSyncForTesting. +func TestMain(m *testing.M) { + DisableSyncForTesting() + os.Exit(m.Run()) +}