From c38e088a3c3662b0df26d00bed3c8918ddf447bf Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 14:00:29 -0400 Subject: [PATCH 01/14] Add a global shutdown function This helps tests on Windows work correctly as well as providing a more convenient way to ensure sync to filesystem in local mode --- engine.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/engine.go b/engine.go index 637841d..fc9a7be 100644 --- a/engine.go +++ b/engine.go @@ -34,6 +34,8 @@ import ( var ( // This ensures that we only have one instance of modusDB in this process. singleton atomic.Bool + // activeEngine tracks the current Engine instance for global access + activeEngine *Engine ErrSingletonOnly = errors.New("only one instance of modusDB can exist in a process") ErrEmptyDataDir = errors.New("data directory is required") @@ -41,9 +43,13 @@ var ( ErrNonExistentDB = errors.New("namespace does not exist") ) -// ResetSingleton resets the singleton state for testing purposes. -// This should ONLY be called during testing, typically in cleanup functions. -func ResetSingleton() { +// Shutdown closes the active Engine instance and resets the singleton state. +func Shutdown() { + if activeEngine != nil { + activeEngine.Close() + activeEngine = nil + } + // Reset the singleton state so a new engine can be created if needed singleton.Store(false) } @@ -105,7 +111,8 @@ func NewEngine(conf Config) (*Engine, error) { engine.logger.Error(err, "Failed to reset database") return nil, fmt.Errorf("error resetting db: %w", err) } - + // Store the engine as the active instance + activeEngine = engine x.UpdateHealthStatus(true) engine.db0 = &Namespace{id: 0, engine: engine} From d92d2d14108adc93579a0f2047fa9ecf3048b7a6 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 14:00:43 -0400 Subject: [PATCH 02/14] Update tests --- client_test.go | 6 +++--- util_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client_test.go b/client_test.go index 9b7184f..4a8efdd 100644 --- a/client_test.go +++ b/client_test.go @@ -110,8 +110,8 @@ func TestClientPool(t *testing.T) { }) } - // Reset singleton at the end of the test to ensure the next test can start fresh - mg.ResetSingleton() + // Shutdown at the end of the test to ensure the next test can start fresh + mg.Shutdown() } func TestClientPoolStress(t *testing.T) { @@ -205,6 +205,6 @@ func TestClientPoolStress(t *testing.T) { require.Greater(t, successCount, 0) }) - mg.ResetSingleton() + mg.Shutdown() } } diff --git a/util_test.go b/util_test.go index 958b802..169c961 100644 --- a/util_test.go +++ b/util_test.go @@ -45,8 +45,8 @@ func CreateTestClient(t *testing.T, uri string) (mg.Client, func()) { } client.Close() - // Reset the singleton state so the next test can create a new engine - mg.ResetSingleton() + // Properly shutdown the engine and reset the singleton state + mg.Shutdown() } return client, cleanup From 95018d803fe00c938dd56de65411d340064c9d89 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 14:12:10 -0400 Subject: [PATCH 03/14] Update changelog --- CHANGELOG.md | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f282fc3..ed9decf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,22 +2,9 @@ ## UNRELEASED -- feat: add readfrom json tag to support reverse edges - [#49](https://github.com/hypermodeinc/modusgraph/pull/49) +- feat: add a Shutdown function to close the active engine -- chore: Refactoring package management [#51](https://github.com/hypermodeinc/modusgraph/pull/51) - -- fix: alter schema on reverse edge after querying schema - [#55](https://github.com/hypermodeinc/modusgraph/pull/55) - -- feat: update interface to engine and namespace - [#57](https://github.com/hypermodeinc/modusgraph/pull/57) - -- chore: Update dgraph dependency [#62](https://github.com/hypermodeinc/modusgraph/pull/62) - -- fix: add context to api functions [#69](https://github.com/hypermodeinc/modusgraph/pull/69) - -## 2025-01-02 - Version 0.1.0 +## 2025-05-21 - Version 0.1.0 Baseline for the changelog. From 254cdfc195dff2e2bbe3a454affb1524d60ccb72 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 14:37:01 -0400 Subject: [PATCH 04/14] Explicitly close pstore files Needed for Windows --- engine.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/engine.go b/engine.go index fc9a7be..35e28ae 100644 --- a/engine.go +++ b/engine.go @@ -385,7 +385,7 @@ func (engine *Engine) LoadData(inCtx context.Context, dataDir string) error { return engine.db0.LoadData(inCtx, dataDir) } -// Close closes the modusDB instance. +// Close closes the modusGraph instance. func (engine *Engine) Close() { engine.mutex.Lock() defer engine.mutex.Unlock() @@ -395,11 +395,18 @@ func (engine *Engine) Close() { } if !singleton.CompareAndSwap(true, false) { - panic("modusDB instance was not properly opened") + panic("modusGraph instance was not properly opened") } engine.isOpen.Store(false) x.UpdateHealthStatus(false) + + // Close Badger DB explicitly to ensure all file handles are released + // This is especially important on Windows where file handles can remain locked + if worker.State.Pstore != nil { + worker.State.Pstore.Close() + } + posting.Cleanup() worker.State.Dispose() } From c55043549f00c7b25d3e6dc469129647d3c36033 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 14:41:01 -0400 Subject: [PATCH 05/14] Add write ahead log close --- engine.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/engine.go b/engine.go index 35e28ae..928ba10 100644 --- a/engine.go +++ b/engine.go @@ -406,6 +406,9 @@ func (engine *Engine) Close() { if worker.State.Pstore != nil { worker.State.Pstore.Close() } + if worker.State.WALstore != nil { + worker.State.WALstore.Close() + } posting.Cleanup() worker.State.Dispose() From 4f6f56e347b3470eeb7241ba8a86c40fa01b4f63 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 15:59:20 -0400 Subject: [PATCH 06/14] Add additional code to get Windows to give up the wal file --- engine.go | 33 +++++++++++++++++++++++++++++++++ shutdown_other.go | 14 ++++++++++++++ shutdown_windows.go | 33 +++++++++++++++++++++++++++++++++ test_helpers_windows.go | 40 ++++++++++++++++++++++++++++++++++++++++ util_test.go | 11 ++++++++++- 5 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 shutdown_other.go create mode 100644 shutdown_windows.go create mode 100644 test_helpers_windows.go diff --git a/engine.go b/engine.go index 928ba10..185216b 100644 --- a/engine.go +++ b/engine.go @@ -10,9 +10,11 @@ import ( "errors" "fmt" "path" + "runtime" "strconv" "sync" "sync/atomic" + "time" "github.com/dgraph-io/badger/v4" "github.com/dgraph-io/dgo/v240" @@ -49,6 +51,12 @@ func Shutdown() { activeEngine.Close() activeEngine = nil } + + // This calls the platform-specific cleanup function + // For Windows, it's defined in shutdown_windows.go + // For other platforms, it's a no-op + windowsCleanup() + // Reset the singleton state so a new engine can be created if needed singleton.Store(false) } @@ -401,15 +409,40 @@ func (engine *Engine) Close() { engine.isOpen.Store(false) x.UpdateHealthStatus(false) + // Ensure all pending operations are completed before shutdown + + // Close all connections and stop servers before closing the DB + if engine.server != nil { + engine.server.Stop() + engine.server = nil + } + if engine.listener != nil { + engine.listener.Close() + engine.listener = nil + } + // Close Badger DB explicitly to ensure all file handles are released // This is especially important on Windows where file handles can remain locked if worker.State.Pstore != nil { + fmt.Println("⚠️ Closing Pstore") + // Sync and try to force-close any open value log files + worker.State.Pstore.Sync() + worker.State.Pstore.RunValueLogGC(0.5) // Force garbage collection before closing worker.State.Pstore.Close() } + if worker.State.WALstore != nil { + fmt.Println("⚠️ Closing WAL store") + // Flush and sync all pending writes before closing + worker.State.WALstore.Sync() worker.State.WALstore.Close() } + // Force garbage collection to help release file handles on Windows + runtime.GC() + // Sleep a little to give the OS time to release file handles (Windows specific) + time.Sleep(100 * time.Millisecond) + posting.Cleanup() worker.State.Dispose() } diff --git a/shutdown_other.go b/shutdown_other.go new file mode 100644 index 0000000..f09644e --- /dev/null +++ b/shutdown_other.go @@ -0,0 +1,14 @@ +//go:build !windows +// +build !windows + +/* + * SPDX-FileCopyrightText: © Hypermode Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package modusgraph + +// windowsCleanup is a no-op on non-Windows platforms +func windowsCleanup() { + // No special cleanup needed on Unix-like systems +} diff --git a/shutdown_windows.go b/shutdown_windows.go new file mode 100644 index 0000000..5a66575 --- /dev/null +++ b/shutdown_windows.go @@ -0,0 +1,33 @@ +//go:build windows +// +build windows + +/* + * SPDX-FileCopyrightText: © Hypermode Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package modusgraph + +import ( + "fmt" + "runtime" + "time" +) + +// windowsCleanup performs Windows-specific cleanup operations to ensure +// file handles are properly released before the temporary directory is removed. +// This is necessary because Windows handles file locks differently than Unix systems. +func windowsCleanup() { + fmt.Println("⚠️ Performing Windows-specific cleanup") + + // Force multiple garbage collections to help release file handles + for i := 0; i < 3; i++ { + runtime.GC() + // Small pause between GCs to allow finalizers to run + time.Sleep(50 * time.Millisecond) + } + + // Give the OS some extra time to actually release the file handles + // This is particularly important for WAL files from Badger DB + time.Sleep(200 * time.Millisecond) +} diff --git a/test_helpers_windows.go b/test_helpers_windows.go new file mode 100644 index 0000000..a17ba14 --- /dev/null +++ b/test_helpers_windows.go @@ -0,0 +1,40 @@ +//go:build windows +// +build windows + +/* + * SPDX-FileCopyrightText: © Hypermode Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package modusgraph + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +// CreateTestDir creates a test directory that works around the Windows file locking issue +// by creating a temporary directory structure that won't be automatically cleaned up by Go's +// testing framework. Instead, we'll just mark it with a timestamp and let it be cleaned up +// by Windows eventually or by an external cleanup process. +func CreateTestDir(t *testing.T) string { + // Get the system temp directory + tempDir := os.TempDir() + + // Create a uniquely named directory that we won't try to delete + // Format: TestWin_TestName_UnixTimestamp + dirName := fmt.Sprintf("TestWin_%s_%d", t.Name(), os.Now().UnixNano()) + testDir := filepath.Join(tempDir, dirName) + + err := os.MkdirAll(testDir, 0755) + if err != nil { + t.Fatalf("Failed to create test directory: %v", err) + } + + // Log where this directory is for manual cleanup if needed + t.Logf("Created Windows-specific test directory: %s", testDir) + + return testDir +} diff --git a/util_test.go b/util_test.go index 169c961..7c86169 100644 --- a/util_test.go +++ b/util_test.go @@ -9,8 +9,10 @@ import ( "context" "log" "os" + "runtime" "strconv" "testing" + "time" "github.com/go-logr/stdr" mg "github.com/hypermodeinc/modusgraph" @@ -39,14 +41,21 @@ func CreateTestClient(t *testing.T, uri string) (mg.Client, func()) { require.NoError(t, err) cleanup := func() { + // First attempt to drop all data, but don't fail if this has an error err := client.DropAll(context.Background()) if err != nil { - t.Error(err) + t.Logf("Warning: DropAll failed: %v", err) } + + // Close the client client.Close() // Properly shutdown the engine and reset the singleton state mg.Shutdown() + + // On Windows, give the OS some time to release file handles + runtime.GC() + time.Sleep(100 * time.Millisecond) } return client, cleanup From 2760f5a8fc1c58ae392f187fd48c67255aa8c0cc Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 16:04:16 -0400 Subject: [PATCH 07/14] Revert "Add additional code to get Windows to give up the wal file" This reverts commit 4f6f56e347b3470eeb7241ba8a86c40fa01b4f63. --- engine.go | 33 --------------------------------- shutdown_other.go | 14 -------------- shutdown_windows.go | 33 --------------------------------- test_helpers_windows.go | 40 ---------------------------------------- util_test.go | 11 +---------- 5 files changed, 1 insertion(+), 130 deletions(-) delete mode 100644 shutdown_other.go delete mode 100644 shutdown_windows.go delete mode 100644 test_helpers_windows.go diff --git a/engine.go b/engine.go index 185216b..928ba10 100644 --- a/engine.go +++ b/engine.go @@ -10,11 +10,9 @@ import ( "errors" "fmt" "path" - "runtime" "strconv" "sync" "sync/atomic" - "time" "github.com/dgraph-io/badger/v4" "github.com/dgraph-io/dgo/v240" @@ -51,12 +49,6 @@ func Shutdown() { activeEngine.Close() activeEngine = nil } - - // This calls the platform-specific cleanup function - // For Windows, it's defined in shutdown_windows.go - // For other platforms, it's a no-op - windowsCleanup() - // Reset the singleton state so a new engine can be created if needed singleton.Store(false) } @@ -409,40 +401,15 @@ func (engine *Engine) Close() { engine.isOpen.Store(false) x.UpdateHealthStatus(false) - // Ensure all pending operations are completed before shutdown - - // Close all connections and stop servers before closing the DB - if engine.server != nil { - engine.server.Stop() - engine.server = nil - } - if engine.listener != nil { - engine.listener.Close() - engine.listener = nil - } - // Close Badger DB explicitly to ensure all file handles are released // This is especially important on Windows where file handles can remain locked if worker.State.Pstore != nil { - fmt.Println("⚠️ Closing Pstore") - // Sync and try to force-close any open value log files - worker.State.Pstore.Sync() - worker.State.Pstore.RunValueLogGC(0.5) // Force garbage collection before closing worker.State.Pstore.Close() } - if worker.State.WALstore != nil { - fmt.Println("⚠️ Closing WAL store") - // Flush and sync all pending writes before closing - worker.State.WALstore.Sync() worker.State.WALstore.Close() } - // Force garbage collection to help release file handles on Windows - runtime.GC() - // Sleep a little to give the OS time to release file handles (Windows specific) - time.Sleep(100 * time.Millisecond) - posting.Cleanup() worker.State.Dispose() } diff --git a/shutdown_other.go b/shutdown_other.go deleted file mode 100644 index f09644e..0000000 --- a/shutdown_other.go +++ /dev/null @@ -1,14 +0,0 @@ -//go:build !windows -// +build !windows - -/* - * SPDX-FileCopyrightText: © Hypermode Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package modusgraph - -// windowsCleanup is a no-op on non-Windows platforms -func windowsCleanup() { - // No special cleanup needed on Unix-like systems -} diff --git a/shutdown_windows.go b/shutdown_windows.go deleted file mode 100644 index 5a66575..0000000 --- a/shutdown_windows.go +++ /dev/null @@ -1,33 +0,0 @@ -//go:build windows -// +build windows - -/* - * SPDX-FileCopyrightText: © Hypermode Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package modusgraph - -import ( - "fmt" - "runtime" - "time" -) - -// windowsCleanup performs Windows-specific cleanup operations to ensure -// file handles are properly released before the temporary directory is removed. -// This is necessary because Windows handles file locks differently than Unix systems. -func windowsCleanup() { - fmt.Println("⚠️ Performing Windows-specific cleanup") - - // Force multiple garbage collections to help release file handles - for i := 0; i < 3; i++ { - runtime.GC() - // Small pause between GCs to allow finalizers to run - time.Sleep(50 * time.Millisecond) - } - - // Give the OS some extra time to actually release the file handles - // This is particularly important for WAL files from Badger DB - time.Sleep(200 * time.Millisecond) -} diff --git a/test_helpers_windows.go b/test_helpers_windows.go deleted file mode 100644 index a17ba14..0000000 --- a/test_helpers_windows.go +++ /dev/null @@ -1,40 +0,0 @@ -//go:build windows -// +build windows - -/* - * SPDX-FileCopyrightText: © Hypermode Inc. - * SPDX-License-Identifier: Apache-2.0 - */ - -package modusgraph - -import ( - "fmt" - "os" - "path/filepath" - "testing" -) - -// CreateTestDir creates a test directory that works around the Windows file locking issue -// by creating a temporary directory structure that won't be automatically cleaned up by Go's -// testing framework. Instead, we'll just mark it with a timestamp and let it be cleaned up -// by Windows eventually or by an external cleanup process. -func CreateTestDir(t *testing.T) string { - // Get the system temp directory - tempDir := os.TempDir() - - // Create a uniquely named directory that we won't try to delete - // Format: TestWin_TestName_UnixTimestamp - dirName := fmt.Sprintf("TestWin_%s_%d", t.Name(), os.Now().UnixNano()) - testDir := filepath.Join(tempDir, dirName) - - err := os.MkdirAll(testDir, 0755) - if err != nil { - t.Fatalf("Failed to create test directory: %v", err) - } - - // Log where this directory is for manual cleanup if needed - t.Logf("Created Windows-specific test directory: %s", testDir) - - return testDir -} diff --git a/util_test.go b/util_test.go index 7c86169..169c961 100644 --- a/util_test.go +++ b/util_test.go @@ -9,10 +9,8 @@ import ( "context" "log" "os" - "runtime" "strconv" "testing" - "time" "github.com/go-logr/stdr" mg "github.com/hypermodeinc/modusgraph" @@ -41,21 +39,14 @@ func CreateTestClient(t *testing.T, uri string) (mg.Client, func()) { require.NoError(t, err) cleanup := func() { - // First attempt to drop all data, but don't fail if this has an error err := client.DropAll(context.Background()) if err != nil { - t.Logf("Warning: DropAll failed: %v", err) + t.Error(err) } - - // Close the client client.Close() // Properly shutdown the engine and reset the singleton state mg.Shutdown() - - // On Windows, give the OS some time to release file handles - runtime.GC() - time.Sleep(100 * time.Millisecond) } return client, cleanup From 20f86b003a0635159f20b5391d7eba588e3d4d5c Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 16:09:02 -0400 Subject: [PATCH 08/14] Simplify the closing process --- engine.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/engine.go b/engine.go index 928ba10..44af7ee 100644 --- a/engine.go +++ b/engine.go @@ -10,9 +10,11 @@ import ( "errors" "fmt" "path" + "runtime" "strconv" "sync" "sync/atomic" + "time" "github.com/dgraph-io/badger/v4" "github.com/dgraph-io/dgo/v240" @@ -412,6 +414,11 @@ func (engine *Engine) Close() { posting.Cleanup() worker.State.Dispose() + + if runtime.GOOS == "windows" { + runtime.GC() + time.Sleep(200 * time.Millisecond) + } } func (ns *Engine) reset() error { From 64d1ec945aab6ceeff657093143e84375a312fdf Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 16:25:25 -0400 Subject: [PATCH 09/14] Attempt to actually close files --- engine.go | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 5 deletions(-) diff --git a/engine.go b/engine.go index 44af7ee..8148585 100644 --- a/engine.go +++ b/engine.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "path" + "reflect" "runtime" "strconv" "sync" @@ -405,11 +406,17 @@ func (engine *Engine) Close() { // Close Badger DB explicitly to ensure all file handles are released // This is especially important on Windows where file handles can remain locked - if worker.State.Pstore != nil { - worker.State.Pstore.Close() - } - if worker.State.WALstore != nil { - worker.State.WALstore.Close() + if runtime.GOOS == "windows" { + if worker.State.Pstore != nil { + worker.State.Pstore.Close() + } + if worker.State.WALstore != nil { + // Regular close call that only does sync + worker.State.WALstore.Close() + // Force close all file descriptors in WALstore + // This is needed especially on Windows where file handles may remain locked + forceCloseWAL(worker.State.WALstore) + } } posting.Cleanup() @@ -421,6 +428,70 @@ func (engine *Engine) Close() { } } +// forceCloseWAL uses reflection to access the WALstore's internal file handles and explicitly close them. +// This is necessary because Dgraph's WALstore.Close() only syncs but doesn't actually close file descriptors. +func forceCloseWAL(walStore interface{}) { + // Use reflection to access the internal DiskStorage + v := reflect.ValueOf(walStore) + if v.Kind() != reflect.Ptr || v.IsNil() { + return + } + + // Try to access the diskStorage field + ds := v.Elem().FieldByName("diskStorage") + if !ds.IsValid() { + return + } + + // Access the meta and wal fields + meta := ds.Elem().FieldByName("meta") + wal := ds.Elem().FieldByName("wal") + + // Close meta file if available + if meta.IsValid() && !meta.IsNil() { + // Close Fd field + fd := meta.Elem().FieldByName("Fd") + if fd.IsValid() && !fd.IsNil() { + closeMethod := fd.MethodByName("Close") + if closeMethod.IsValid() { + closeMethod.Call(nil) + } + } + } + + // Close wal files if available + if wal.IsValid() && !wal.IsNil() { + // Close current file + current := wal.Elem().FieldByName("current") + if current.IsValid() && !current.IsNil() { + fd := current.Elem().FieldByName("Fd") + if fd.IsValid() && !fd.IsNil() { + closeMethod := fd.MethodByName("Close") + if closeMethod.IsValid() { + closeMethod.Call(nil) + } + } + } + + // Close all files in the files slice + files := wal.Elem().FieldByName("files") + if files.IsValid() && files.Kind() == reflect.Slice { + for i := 0; i < files.Len(); i++ { + file := files.Index(i) + if file.IsValid() && !file.IsNil() { + fd := file.Elem().FieldByName("Fd") + if fd.IsValid() && !fd.IsNil() { + closeMethod := fd.MethodByName("Close") + if closeMethod.IsValid() { + closeMethod.Call(nil) + } + } + } + } + } + } +} + func (ns *Engine) reset() error { z, restart, err := newZero() if err != nil { From a3873989eaeb46cd771f5713991c9c8c64fa2392 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 16:36:28 -0400 Subject: [PATCH 10/14] Testing --- admin_test.go | 7 ++++- engine.go | 82 ++++----------------------------------------------- 2 files changed, 12 insertions(+), 77 deletions(-) diff --git a/admin_test.go b/admin_test.go index d8c25ef..d6bb301 100644 --- a/admin_test.go +++ b/admin_test.go @@ -23,7 +23,8 @@ func TestDropData(t *testing.T) { }{ { name: "DropDataWithFileURI", - uri: "file://" + t.TempDir(), + //uri: "file://" + t.TempDir(), + uri: "file://", }, { name: "DropDataWithDgraphURI", @@ -39,6 +40,10 @@ func TestDropData(t *testing.T) { return } + if tc.uri == "file://" { + tc.uri = "file://" + t.TempDir() + } + client, cleanup := CreateTestClient(t, tc.uri) defer cleanup() diff --git a/engine.go b/engine.go index 8148585..58bb827 100644 --- a/engine.go +++ b/engine.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "path" - "reflect" "runtime" "strconv" "sync" @@ -406,17 +405,12 @@ func (engine *Engine) Close() { // Close Badger DB explicitly to ensure all file handles are released // This is especially important on Windows where file handles can remain locked - if runtime.GOOS == "windows" { - if worker.State.Pstore != nil { - worker.State.Pstore.Close() - } - if worker.State.WALstore != nil { - // Regular close call that only does sync - worker.State.WALstore.Close() - // Force close all file descriptors in WALstore - // This is needed especially on Windows where file handles may remain locked - forceCloseWAL(worker.State.WALstore) - } + if worker.State.Pstore != nil { + worker.State.Pstore.Close() + } + if worker.State.WALstore != nil { + // Regular close call that only does sync + worker.State.WALstore.Close() } posting.Cleanup() @@ -428,70 +422,6 @@ func (engine *Engine) Close() { } } -// forceCloseWAL uses reflection to access the WALstore's internal file handles and explicitly close them. -// This is necessary because Dgraph's WALstore.Close() only syncs but doesn't actually close file descriptors. -func forceCloseWAL(walStore interface{}) { - // Use reflection to access the internal DiskStorage - v := reflect.ValueOf(walStore) - if v.Kind() != reflect.Ptr || v.IsNil() { - return - } - - // Try to access the diskStorage field - ds := v.Elem().FieldByName("diskStorage") - if !ds.IsValid() { - return - } - - // Access the meta and wal fields - meta := ds.Elem().FieldByName("meta") - wal := ds.Elem().FieldByName("wal") - - // Close meta file if available - if meta.IsValid() && !meta.IsNil() { - // Close Fd field - fd := meta.Elem().FieldByName("Fd") - if fd.IsValid() && !fd.IsNil() { - closeMethod := fd.MethodByName("Close") - if closeMethod.IsValid() { - closeMethod.Call(nil) - } - } - } - - // Close wal files if available - if wal.IsValid() && !wal.IsNil() { - // Close current file - current := wal.Elem().FieldByName("current") - if current.IsValid() && !current.IsNil() { - fd := current.Elem().FieldByName("Fd") - if fd.IsValid() && !fd.IsNil() { - closeMethod := fd.MethodByName("Close") - if closeMethod.IsValid() { - closeMethod.Call(nil) - } - } - } - - // Close all files in the files slice - files := wal.Elem().FieldByName("files") - if files.IsValid() && files.Kind() == reflect.Slice { - for i := 0; i < files.Len(); i++ { - file := files.Index(i) - if file.IsValid() && !file.IsNil() { - fd := file.Elem().FieldByName("Fd") - if fd.IsValid() && !fd.IsNil() { - closeMethod := fd.MethodByName("Close") - if closeMethod.IsValid() { - closeMethod.Call(nil) - } - } - } - } - } - } -} - func (ns *Engine) reset() error { z, restart, err := newZero() if err != nil { From 31b0eceb21dd8e8309e4d55254c6306bec734275 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 16:38:56 -0400 Subject: [PATCH 11/14] Testing --- admin_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin_test.go b/admin_test.go index d6bb301..ff0a77b 100644 --- a/admin_test.go +++ b/admin_test.go @@ -45,7 +45,7 @@ func TestDropData(t *testing.T) { } client, cleanup := CreateTestClient(t, tc.uri) - defer cleanup() + t.Cleanup(cleanup) entity := TestEntity{ Name: "Test Entity", From 2336e6bfd905d08e0a8869d788e1cc6918ac70e3 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 17:33:27 -0400 Subject: [PATCH 12/14] Add a windows-specific temp dir generator --- admin_test.go | 9 ++------- engine.go | 31 ++++++++++--------------------- util_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/admin_test.go b/admin_test.go index ff0a77b..9117cf1 100644 --- a/admin_test.go +++ b/admin_test.go @@ -23,8 +23,7 @@ func TestDropData(t *testing.T) { }{ { name: "DropDataWithFileURI", - //uri: "file://" + t.TempDir(), - uri: "file://", + uri: "file://" + GetTempDir(t), }, { name: "DropDataWithDgraphURI", @@ -40,12 +39,8 @@ func TestDropData(t *testing.T) { return } - if tc.uri == "file://" { - tc.uri = "file://" + t.TempDir() - } - client, cleanup := CreateTestClient(t, tc.uri) - t.Cleanup(cleanup) + defer cleanup() entity := TestEntity{ Name: "Test Entity", diff --git a/engine.go b/engine.go index 58bb827..41c5589 100644 --- a/engine.go +++ b/engine.go @@ -45,16 +45,6 @@ var ( ErrNonExistentDB = errors.New("namespace does not exist") ) -// Shutdown closes the active Engine instance and resets the singleton state. -func Shutdown() { - if activeEngine != nil { - activeEngine.Close() - activeEngine = nil - } - // Reset the singleton state so a new engine can be created if needed - singleton.Store(false) -} - // Engine is an instance of modusDB. // For now, we only support one instance of modusDB per process. type Engine struct { @@ -123,6 +113,16 @@ func NewEngine(conf Config) (*Engine, error) { return engine, nil } +// Shutdown closes the active Engine instance and resets the singleton state. +func Shutdown() { + if activeEngine != nil { + activeEngine.Close() + activeEngine = nil + } + // Reset the singleton state so a new engine can be created if needed + singleton.Store(false) +} + func (engine *Engine) GetClient() (*dgo.Dgraph, error) { engine.logger.V(2).Info("Getting Dgraph client from engine") client, err := createDgraphClient(context.Background(), engine.listener) @@ -402,17 +402,6 @@ func (engine *Engine) Close() { engine.isOpen.Store(false) x.UpdateHealthStatus(false) - - // Close Badger DB explicitly to ensure all file handles are released - // This is especially important on Windows where file handles can remain locked - if worker.State.Pstore != nil { - worker.State.Pstore.Close() - } - if worker.State.WALstore != nil { - // Regular close call that only does sync - worker.State.WALstore.Close() - } - posting.Cleanup() worker.State.Dispose() diff --git a/util_test.go b/util_test.go index 169c961..5ebcd17 100644 --- a/util_test.go +++ b/util_test.go @@ -9,8 +9,12 @@ import ( "context" "log" "os" + "path/filepath" + "runtime" "strconv" + "strings" "testing" + "time" "github.com/go-logr/stdr" mg "github.com/hypermodeinc/modusgraph" @@ -52,6 +56,46 @@ func CreateTestClient(t *testing.T, uri string) (mg.Client, func()) { return client, cleanup } +func GetTempDir(t *testing.T) string { + if runtime.GOOS == "windows" { + // Get the system temp directory + baseDir := os.TempDir() + + // Create a unique directory name using the test name + testName := t.Name() + // Clean the test name to make it filesystem-safe + testName = strings.ReplaceAll(testName, "/", "_") + testName = strings.ReplaceAll(testName, "\\", "_") + testName = strings.ReplaceAll(testName, ":", "_") + + // Create a directory path that includes the test name + tempDir := filepath.Join(baseDir, "modusgraph_test_"+testName) + + // Ensure the directory exists + err := os.MkdirAll(tempDir, 0755) + if err != nil { + // Fall back to standard temp directory if we can't create our own + t.Logf("Failed to create temp directory %s: %v, falling back to standard temp dir", tempDir, err) + return os.TempDir() + } + + // Register cleanup function with the test + t.Cleanup(func() { + // Force GC to help release any open file handles + runtime.GC() + time.Sleep(200 * time.Millisecond) + + // Try to remove the directory + if err := os.RemoveAll(tempDir); err != nil { + t.Logf("Warning: failed to remove temp directory %s: %v", tempDir, err) + } + }) + + return tempDir + } + return t.TempDir() +} + // SetupTestEnv configures the environment variables for tests. // This is particularly useful when debugging tests in an IDE. func SetupTestEnv(logLevel int) { From 12f8fb00824481af819cda78af0b6e541e778906 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 17:54:56 -0400 Subject: [PATCH 13/14] Utilize the new testing temp folder function --- admin_test.go | 4 ++-- client_test.go | 4 ++-- delete_test.go | 2 +- insert_test.go | 6 +++--- query_test.go | 6 +++--- unit_test/api_test.go | 17 ----------------- update_test.go | 2 +- util_test.go | 14 ++++---------- 8 files changed, 16 insertions(+), 39 deletions(-) diff --git a/admin_test.go b/admin_test.go index 9117cf1..4a91486 100644 --- a/admin_test.go +++ b/admin_test.go @@ -81,7 +81,7 @@ func TestDropAll(t *testing.T) { }{ { name: "DropAllWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "DropAllWithDgraphURI", @@ -159,7 +159,7 @@ func TestCreateSchema(t *testing.T) { }{ { name: "CreateSchemaWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "CreateSchemaWithDgraphURI", diff --git a/client_test.go b/client_test.go index 4a8efdd..93ea47c 100644 --- a/client_test.go +++ b/client_test.go @@ -25,7 +25,7 @@ func TestClientPool(t *testing.T) { }{ { name: "ClientPoolWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "ClientPoolWithDgraphURI", @@ -122,7 +122,7 @@ func TestClientPoolStress(t *testing.T) { }{ { name: "ClientPoolStressWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "ClientPoolStressWithDgraphURI", diff --git a/delete_test.go b/delete_test.go index 8212bc3..c204f66 100644 --- a/delete_test.go +++ b/delete_test.go @@ -24,7 +24,7 @@ func TestClientDelete(t *testing.T) { }{ { name: "DeleteWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "DeleteWithDgraphURI", diff --git a/insert_test.go b/insert_test.go index 4009ddd..557045c 100644 --- a/insert_test.go +++ b/insert_test.go @@ -35,7 +35,7 @@ func TestClientInsert(t *testing.T) { }{ { name: "InsertWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "InsertWithDgraphURI", @@ -95,7 +95,7 @@ func TestClientInsertMultipleEntities(t *testing.T) { }{ { name: "InsertMultipleWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "InsertMultipleWithDgraphURI", @@ -157,7 +157,7 @@ func TestDepthQuery(t *testing.T) { }{ { name: "InsertWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "InsertWithDgraphURI", diff --git a/query_test.go b/query_test.go index 2292f80..660f404 100644 --- a/query_test.go +++ b/query_test.go @@ -26,7 +26,7 @@ func TestClientSimpleGet(t *testing.T) { }{ { name: "GetWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "GetWithDgraphURI", @@ -86,7 +86,7 @@ func TestClientQuery(t *testing.T) { }{ { name: "QueryWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "QueryWithDgraphURI", @@ -268,7 +268,7 @@ func TestVectorSimilaritySearch(t *testing.T) { }{ { name: "VectorSimilaritySearchWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, /* { diff --git a/unit_test/api_test.go b/unit_test/api_test.go index 3147358..8cf7cdb 100644 --- a/unit_test/api_test.go +++ b/unit_test/api_test.go @@ -1102,20 +1102,3 @@ func TestMultiPolygon(t *testing.T) { require.Equal(t, "Jane Doe", geomStruct.Name) require.Equal(t, multiPolygon.Coordinates, geomStruct.MultiArea.Coordinates) } - -func TestUserStore(t *testing.T) { - ctx := context.Background() - engine, err := modusgraph.NewEngine(modusgraph.NewDefaultConfig("./foo")) - require.NoError(t, err) - defer engine.Close() - - user := User{ - Name: "John Doe", - Age: 30, - } - gid, user, err := modusgraph.Create(ctx, engine, user) - require.NoError(t, err) - require.NotZero(t, gid) - require.Equal(t, "John Doe", user.Name) - require.Equal(t, 30, user.Age) -} diff --git a/update_test.go b/update_test.go index be7291b..9e762bb 100644 --- a/update_test.go +++ b/update_test.go @@ -23,7 +23,7 @@ func TestClientUpdate(t *testing.T) { }{ { name: "UpdateWithFileURI", - uri: "file://" + t.TempDir(), + uri: "file://" + GetTempDir(t), }, { name: "UpdateWithDgraphURI", diff --git a/util_test.go b/util_test.go index 5ebcd17..b22b8c8 100644 --- a/util_test.go +++ b/util_test.go @@ -56,36 +56,30 @@ func CreateTestClient(t *testing.T, uri string) (mg.Client, func()) { return client, cleanup } +// GetTempDir returns a temporary directory for testing purposes. +// It creates a unique directory for each test and registers a cleanup function to remove it. +// On Windows, it uses the standard temp directory and creates a unique directory for each test. +// On other platforms, it uses the standard toolchain TempDir function. func GetTempDir(t *testing.T) string { if runtime.GOOS == "windows" { - // Get the system temp directory baseDir := os.TempDir() - - // Create a unique directory name using the test name testName := t.Name() - // Clean the test name to make it filesystem-safe testName = strings.ReplaceAll(testName, "/", "_") testName = strings.ReplaceAll(testName, "\\", "_") testName = strings.ReplaceAll(testName, ":", "_") - // Create a directory path that includes the test name tempDir := filepath.Join(baseDir, "modusgraph_test_"+testName) - // Ensure the directory exists err := os.MkdirAll(tempDir, 0755) if err != nil { - // Fall back to standard temp directory if we can't create our own t.Logf("Failed to create temp directory %s: %v, falling back to standard temp dir", tempDir, err) return os.TempDir() } - // Register cleanup function with the test t.Cleanup(func() { - // Force GC to help release any open file handles runtime.GC() time.Sleep(200 * time.Millisecond) - // Try to remove the directory if err := os.RemoveAll(tempDir); err != nil { t.Logf("Warning: failed to remove temp directory %s: %v", tempDir, err) } From 801b20baa4b4f469944f34bc56387ae038d8ae33 Mon Sep 17 00:00:00 2001 From: Matthew McNeely Date: Tue, 27 May 2025 18:19:45 -0400 Subject: [PATCH 14/14] Update README --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index a8b3419..fc96ca4 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,18 @@ Version 2.0. See the [LICENSE](./LICENSE) file for a complete copy of the licens questions about modus licensing, or need an alternate license or other arrangement, please contact us at . +## Windows Users + +modusGraph (and its dependencies) are designed to work on POSIX-compliant operating systems, and are +not guaranteed to work on Windows. + +Tests at the top level folder (`go test .`) on Windows are maintained to pass, but other tests in +subfolders may not work as expected. + +Temporary folders created during tests may not be cleaned up properly on Windows. Users should +periodically clean up these folders. The temporary folders are created in the Windows temp +directory, `C:\Users\\AppData\Local\Temp\modusgraph_test*`. + ## Acknowledgements modusGraph builds heavily upon packages from the open source projects of