Cherry-Picking more commits from main into dev #1581
Conversation
…y is possible around MigrateSessionTaskStore.TryRemove, so don't spin indefinitely
…ly gets a clean folder
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks two improvements from the main branch into dev:
-
Test isolation improvement (
TestUtils.cs):UnitTestWorkingDir()is simplified by removing thecategoryandincludeGuidparameters. Instead, it now automatically incorporates test arguments as a hash code, ensuring different parameterized test runs of the same method get separate working directories without requiring callers to pass extra arguments. -
Dispose refactoring (
MigrateSessionTaskStore.cs): TheDispose()method is updated so that session disposal happens outside the write lock. The flag_disposedis set while holding the lock to prevent concurrent add/remove operations from racing, and then sessions are disposed after the lock is released — this is safe because all other methods check_disposedearly under the lock before accessing sessions.
Changes:
- Simplified
UnitTestWorkingDir()inTestUtils.csto use argument-based hash codes for parameterized test separation instead of optionalcategory/includeGuidparameters. - Refactored
MigrateSessionTaskStore.Dispose()inMigrateSessionTaskStore.csto set the disposed flag under a lock, then dispose sessions outside the lock.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
test/Garnet.test/TestUtils.cs |
Removes category/includeGuid params; adds argument hashing for test directory isolation |
libs/cluster/Server/Migration/MigrateSessionTaskStore.cs |
Refactors Dispose() to release the write lock before iterating/disposing sessions; adds /// <inheritdoc/> tag |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.