From ea9074422fdf464975e8c9aade394a7cbf9b7a06 Mon Sep 17 00:00:00 2001 From: Badrish Chandramouli Date: Fri, 14 Aug 2026 15:47:43 -0700 Subject: [PATCH 1/3] Fix root causes behind recurring Garnet .NET CI flakes Triaged the last 60 "Garnet .NET CI" runs on main (39 failed, 68 failed-job logs) and bucketed every failure by root cause. CI runners have 2 cores, so each failure was reproduced locally under `taskset -c 0-1`; that is why these do not reproduce on high-core dev machines. Every change below addresses the underlying defect rather than relaxing a timeout, adding a retry, or weakening an assertion. Product fixes ------------- Vector Set contexts are allocated independently per node and index creation is not AOF-logged, so on slot migration the destination reserved a context from its own free list and stamped it into the AOF migrate records. Replay adopted that context verbatim, guarding only on IsMigrating and never on IsInUse. ContextMetadata.MarkInUse only asserted, which is compiled out in Release - the configuration the failing CI job runs - so two Vector Sets silently shared one namespace and destroyed each other's data. Migrated records are now remapped onto a context that is free locally, and the mapping is held for the duration of the migration. Element records are provably transmitted, applied, and acknowledged before the index key is sent (Task.WhenAll barrier in MigrateSessionSlots.CreateAndRunMigrateTasksAsync), so the mapping can be released when the index key arrives. A/B over 10 iterations: 12 context collisions, 748 records destroyed and 5 exceptions before, none after. A Vector Set's index record is written before the context metadata that reserves its context, and that metadata is only flushed once index creation succeeds. A checkpoint taken between the two captures a live index record whose context is not marked in use, so the free list hands the same context to the next Vector Set created and the two silently share a namespace - the same corruption class as the migration defect above. ReconcileRecoveredState now restores the reservation for every recovered index whose context is free. The hash slot the reservation needs is computed in RecoveredVectorSetIndexKey, which widens recoveredIndexes from byte to ushort to carry it. VectorSetRecoveredContextReservationTests covers this: with the fix the new Vector Set gets its own context, and without it recovery reports the context as free and the next VADD is handed the identical context, failing the test. The migration remap above is in-memory state keyed on contextMetadatas, and two paths rebuild that array underneath it: FLUSHDB/FLUSHALL through FlushGuard.Dispose, and recovery through ReconcileRecoveredState. Neither cleared the remap, so a cached entry could steer the remaining records of a migration into a context that had since been freed and handed to another Vector Set. Both paths now clear it. Recovery already treats a context still marked migrating as a failed migration and marks it for cleanup, so forcing the retried migration to re-resolve is the intended behaviour rather than only the safe one. Test fixes ---------- A thread parked in ExceptionInjectionHelper.ResetAndWaitAsync is only released by EnableException, but the cleanup a test runs on its way out is DisableException. Any test that leaves between a waiter arriving and being re-enabled - an assertion failing, or a wait for the arrival timing out on a slow 2-core runner - therefore strands that waiter permanently. The waiter is a server thread holding a pooled network buffer, so LimitedFixedBufferPool.Dispose spins forever on a reference that is never returned and the entire test process hangs: the job produces no results at all rather than one failing test, which is why these runs show up in CI as a bare timeout with no reportable failure. The cluster call sites already bound this with WaitAsync(timeout, token); the four Vector Set call sites use the unbounded synchronous ResetAndWait. GarnetServer now suspends parking for the duration of InternalDispose, which releases anyone already parked and stops anyone new from parking. Covering arrivals matters because disposal closes listeners before it drains handlers, so a request already in flight can reach a still-armed injection point after shutdown has begun and strand itself there; releasing only the waiters that were already parked leaves the identical hang one moment later. The suspension is a count rather than a flag so concurrent shutdowns are independent, and it is unwound in a finally so it cannot leak into a later test sharing this process-wide state - a leaked suspension would silently stop every subsequent injection point from pausing anything. No timeout is introduced. Both the helper methods and their call site are [Conditional("DEBUG")] and compile away in Release. ExceptionInjectionShutdownTests covers all three properties, and each fails when the corresponding behaviour is removed: disposal completes while a waiter is parked, a caller arriving after shutdown began does not park, and parking still pauses normally once the shutdown that suspended it has finished. StackExchange.Redis 2.12.8 pools SimpleResultBox in a [ThreadStatic] field and sets the exception outside the lock that ActivateContinuations pulses. When a server error faults the box before the caller reaches Monitor.Wait, the caller skips the wait and recycles the box while the reader thread still has a PulseAll outstanding. The next synchronous call on that thread consumes the stale pulse and returns with neither result nor exception, shifting every subsequent reply on that thread by one - a permanent lag that survives new multiplexers and new servers, because the box is thread-static and NUnit runs a fixture on one thread. The CI logs show this exactly: LuaScriptTests.Issue1079 loses its exception and Issue1235 later receives Issue1079's error, and ScriptLoadErrors pairs with Struct the same way, with counts matching 1:1. RespVectorSetTests.VADDErrors fails identically. Adds TestUtils.ThrowsRedisException, which runs the failing call on a dedicated thread so the poisoned box never lands on the shared test thread, and applies it to the two fixtures CI proves are affected. Garnet's wire output was confirmed byte-exact with a command/reply balance counter, so this corrects the client-side defect without hiding any server behaviour. DatabaseManagerBase.TakeCheckpointAsync runs compaction inline, and with the compactionMaxSegments: 1 used by VectorSetOverwriteTests the computed compactLength is zero, so untilAddress equals readOnlyAddress and every SAVE compacts the entire log. SETAsync was the only test in the fixture invoking the ~120 MB helper twice against one server; splitting the KEEPTTL case into its own test takes it from 13 s to 2 s. The 3-minute AsyncTimeout added previously is left in place as defence in depth. DisposeCluster disposed nodes in a bare loop, so one node throwing or stalling left the rest of the cluster alive and holding its ports; the next four tests then failed with "Failed to connect within 30 seconds" even though the test that actually broke had passed. Every node's dispose is now started before any of them is waited on, and each is then waited on under a bounded share of one budget. Starting them inside the wait loop would leave a single node that exhausts the budget with every later node still undisposed, which is the same abandoned port by another route. Each dispose gets a dedicated thread rather than a pool thread, because on a two-core runner the pool injects threads slowly enough that a queued dispose could otherwise sit unstarted behind the ones already blocked. Task.Wait(TimeSpan) rethrows a faulted dispose as an AggregateException rather than returning, so it is caught per node; without that the throw escaped mid-loop and abandoned every remaining node, which is the cascade this loop exists to prevent. A standalone probe confirmed it: 1 of 3 healthy nodes disposed before, 3 of 3 after. The wait is bounded only by the shared budget, with no per-node cap, so a merely slow node cannot fail a teardown that the pre-existing outer guard would have allowed. CLUSTER MEET returns before gossip propagates, and MIGRATE and SETSLOT are issued against the source node, so the source must know the target or it replies "ERR Unknown endpoint". Adds the missing convergence waits in ClusterMigrateTests, including one that waited in the wrong direction and one that had no wait at all. Slot re-assignment likewise settles asynchronously after CLUSTER FAILOVER FORCE, so ClusterDivergentReplicasTest now waits for the new primary to observe its own slot ownership before writing to it. Verification ------------ All runs pinned to 2 cores with `taskset -c 0-1`, Debug, net8.0: Garnet.test.vectorset 400/400 Garnet.test.scripting 619 passed, 30 skipped, 0 failed Garnet.test.cluster.vectorsets 84/84 Garnet.test.cluster.repl.vectorsets 7/7 Garnet.test.cluster 156/156 Garnet.test.cluster.replication 107/107 Garnet.test.cluster.migrate 56/56 Build is clean with 0 warnings, and `dotnet format` reports no changes for both Garnet.slnx and Tsavorite.slnx. Superseded upstream during rebase --------------------------------- Fixes from the original version of this branch were landed independently on main and have been dropped in favour of the upstream versions: - The missing VADDSetFlagsArg branch in HandleVectorSetAddReplication, and the drain that has to precede it, are both in #2080. That version is a superset: it also reads the index back and fails loudly rather than silently applying flags to a Vector Set that is not there. - UpgradeReplicasAsync racing a promoted primary that is still recovering is fixed in main with WaitForFailoverCompleted, which polls the failover state machine directly instead of inferring readiness from slot coverage. The WaitForAllSlotsServedAsync helper written for that call site is dropped with it rather than left unused. Deliberately unchanged ---------------------- ClusterSRAddReplicaAfterPrimaryCheckpoint and RespMemoryWriterOverflowTests.SortedSetAsync did not reproduce (3/3 and 6/6 locally), so no speculative change was made rather than risk masking a cause that has not been identified. Vector Set cleanup reclaims a whole context but decides a context is abandoned from the single key QueueCleanups happened to observe during compaction, so a context that lives on under a different key - after RENAME, or after being handed to the next Vector Set created - can be marked while live. Fixing that requires an allocation generation on the context so a stale request can be recognised, which is a design change to context ownership rather than a CI issue, and is being handled separately. Compensating guards were prototyped here and removed: they cannot cover the window in which a context is reserved before its index record is written, and detecting the damage after the fact only converts silent data loss into a different symptom. ClusterVectorSetTests.VectorSetMigrateManyBySlot: poll on any failed read ------------------------------------------------------------------------- This test failed three times across recent CI runs (windows net8.0 Debug and windows net10.0 Debug) with "Expected: 3, But was: 1". ClusterTestUtils.Execute catches every exception and returns the message as a single-element reply, so a failed read is indistinguishable from data by type. The retry loop that reads the migrated keys back from the new replica treated a reply as transient only when its text began with "Key has MOVED to ", and hit a hard ClassicAssert.AreEqual(3, length) for anything else - on the very first poll, before the replica had converged. Probing that window directly (reading the new replica immediately after MigrateSlots) shows three distinct transient replies, only the first of which the old guard recognised: "Key has MOVED to Endpoint ... but CommandFlags.NoRedirect was specified" "CLUSTERDOWN Hash slot not served" a nil reply, which casts to a null array and throws NullReferenceException Replaying the original guard in that window fails 2/2 tests with exactly the CI error; the new guard passes 5 runs of 2/2. A successful VSIM ... WITHSCORES WITHATTRIBS hit is always three elements, so a null or single-element reply always means the read did not land. The loop now treats all of them as "poll again", captures the last response for the failure message, and no longer asserts inside the loop - a caught NUnit assertion is still recorded against the test result even when the next poll would resolve it, which the single-slot variant of this test already documents. The element/score/attribute assertions are unchanged; they now run once after the loop, so a genuinely wrong reply still fails with the full NUnit diff, and a persistent error still fails with the response text instead of a bare length mismatch. The polling window matches the 15s the single-slot variant already uses for the same convergence event, and the previously absent sleep between polls stops the loop from spinning on the two cores it shares with the nodes it is waiting for. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 30f31d5b-9e53-4111-91a3-e16c94510a77 --- .../Testing/ExceptionInjectionHelper.cs | 56 +++- libs/host/GarnetServer.cs | 40 ++- .../Vector/VectorManager.ContextMetadata.cs | 4 + .../Resp/Vector/VectorManager.Replication.cs | 145 ++++++++--- libs/server/Resp/Vector/VectorManager.cs | 26 +- .../ClusterMigrateTests.cs | 11 + .../ClusterReplicationBaseTests.cs | 6 +- .../VectorSets/ClusterVectorSetTests.cs | 79 ++++-- .../Garnet.test.cluster/ClusterTestContext.cs | 98 ++++++- .../Garnet.test.scripting/LuaScriptTests.cs | 246 +++++++++--------- .../ExceptionInjectionShutdownTests.cs | 152 +++++++++++ .../RespVectorSetTests.cs | 110 ++++---- .../VectorSetOverwriteTests.cs | 15 +- ...ctorSetRecoveredContextReservationTests.cs | 137 ++++++++++ test/standalone/Garnet.test/TestUtils.cs | 58 +++++ 15 files changed, 915 insertions(+), 268 deletions(-) create mode 100644 test/standalone/Garnet.test.vectorset/ExceptionInjectionShutdownTests.cs create mode 100644 test/standalone/Garnet.test.vectorset/VectorSetRecoveredContextReservationTests.cs diff --git a/libs/common/Testing/ExceptionInjectionHelper.cs b/libs/common/Testing/ExceptionInjectionHelper.cs index 7498934a7bc..94a0affeba7 100644 --- a/libs/common/Testing/ExceptionInjectionHelper.cs +++ b/libs/common/Testing/ExceptionInjectionHelper.cs @@ -17,6 +17,12 @@ public static class ExceptionInjectionHelper static object @lock = new(); static TaskCompletionSource update = new(TaskCreationOptions.RunContinuationsAsynchronously); + /// + /// Non-zero while at least one scope is open. While it is non-zero + /// refuses to park, because nothing is left to signal it. + /// + static int parkingSuspensions; + /// /// Array of exception injection types /// @@ -128,6 +134,51 @@ public static bool TriggerCondition(ExceptionInjectionType exceptionType) #endif } + /// + /// Stops from parking, and releases anyone already parked. + /// + /// A parked waiter is only released by , but the cleanup a test runs on + /// its way out is . A test that leaves between a waiter arriving and + /// being re-enabled - an assertion failing, or a wait for the arrival timing out - therefore strands + /// that waiter permanently. The waiter is a server thread holding a pooled network buffer, so + /// LimitedFixedBufferPool.Dispose then spins forever waiting for a reference that is never + /// returned and the whole test process hangs rather than one test failing. + /// + /// Suspension covers arrivals as well as waiters already parked, so a request that reaches an + /// injection point after its owner has begun shutting down cannot re-create the same hang. + /// + /// Every call must be paired with . The count keeps concurrent shutdowns + /// independent, and ending every scope is what stops a suspension leaking into a later test sharing + /// this process-wide state. + /// + [Conditional("DEBUG")] + public static void SuspendParking() + { + TaskCompletionSource release; + + lock (@lock) + { + parkingSuspensions++; + release = update; + update = new(TaskCreationOptions.RunContinuationsAsynchronously); + } + + _ = release.TrySetResult(true); + } + + /// + /// Ends a scope. Parking resumes once every scope has ended. + /// + [Conditional("DEBUG")] + public static void ResumeParking() + { + lock (@lock) + { + Debug.Assert(parkingSuspensions > 0, "ResumeParking without a matching SuspendParking"); + parkingSuspensions--; + } + } + /// /// Wait on set condition /// @@ -149,7 +200,10 @@ public static async Task ResetAndWaitAsync(ExceptionInjectionType exceptionType) Task task; lock (@lock) { - if (IsEnabled(exceptionType)) + // Parking is suspended while a server is shutting down, because whoever armed this + // injection point is gone and will never re-enable it. Reading it under the lock is + // what makes a suspension raised at any point before here take effect. + if (IsEnabled(exceptionType) || parkingSuspensions > 0) break; task = update.Task; } diff --git a/libs/host/GarnetServer.cs b/libs/host/GarnetServer.cs index 8b872b40c73..b4fe319d3a1 100644 --- a/libs/host/GarnetServer.cs +++ b/libs/host/GarnetServer.cs @@ -564,23 +564,35 @@ public void Dispose(bool deleteDir = true) private void InternalDispose() { - // Phase 1: Stop listening on all servers to free ports immediately. - for (var i = 0; i < servers.Length; i++) - servers[i]?.Close(); + // A thread parked in a test injection point holds a pooled network buffer, and the drain below + // waits for every such buffer to come back. Suspending covers both waiters already parked and + // requests that reach an injection point while this runs, so shutdown cannot be blocked by an + // injection point whose owner is already gone. Compiled out in Release. + ExceptionInjectionHelper.SuspendParking(); + try + { + // Phase 1: Stop listening on all servers to free ports immediately. + for (var i = 0; i < servers.Length; i++) + servers[i]?.Close(); - // Phase 2: Drain active handlers and clean up remaining resources. - for (var i = 0; i < servers.Length; i++) - servers[i]?.Dispose(); + // Phase 2: Drain active handlers and clean up remaining resources. + for (var i = 0; i < servers.Length; i++) + servers[i]?.Dispose(); - // Phase 3: Dispose the provider (storage engine shutdown — may take time). - Provider?.Dispose(); + // Phase 3: Dispose the provider (storage engine shutdown — may take time). + Provider?.Dispose(); - subscribeBroker?.Dispose(); - storeEpoch?.Dispose(); - pubSubEpoch?.Dispose(); - opts.AuthSettings?.Dispose(); - if (disposeLoggerFactory) - loggerFactory?.Dispose(); + subscribeBroker?.Dispose(); + storeEpoch?.Dispose(); + pubSubEpoch?.Dispose(); + opts.AuthSettings?.Dispose(); + if (disposeLoggerFactory) + loggerFactory?.Dispose(); + } + finally + { + ExceptionInjectionHelper.ResumeParking(); + } } private static void DeleteDirectory(string path) diff --git a/libs/server/Resp/Vector/VectorManager.ContextMetadata.cs b/libs/server/Resp/Vector/VectorManager.ContextMetadata.cs index 91f85f88d32..af33eaeda27 100644 --- a/libs/server/Resp/Vector/VectorManager.ContextMetadata.cs +++ b/libs/server/Resp/Vector/VectorManager.ContextMetadata.cs @@ -448,6 +448,10 @@ public readonly void Dispose() manager.recoveredIndexes.Clear(); manager.recoveredMetadata.Clear(); + // Migration remappings name contexts in the array just replaced, so a surviving entry would + // steer the rest of that migration into a context this flush has already handed back + manager.ClearMigratedContextRemap(); + // Allow Vector Set operations again manager.vectorSetLocks.ReleaseLock(lockToken); diff --git a/libs/server/Resp/Vector/VectorManager.Replication.cs b/libs/server/Resp/Vector/VectorManager.Replication.cs index afcd435f146..7ceb513e83e 100644 --- a/libs/server/Resp/Vector/VectorManager.Replication.cs +++ b/libs/server/Resp/Vector/VectorManager.Replication.cs @@ -4,6 +4,7 @@ using System; using System.Buffers; using System.Buffers.Binary; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Runtime.InteropServices; @@ -165,6 +166,87 @@ internal void ReplicateVectorSetSetAttribute(ReadOnlySpan key, ReadOnlySpa } } + /// + /// Maps a context chosen by a PRIMARY for an in-flight migration onto the context this node actually + /// stores that migrated Vector Set under. + /// + /// Guarded by lock (this), like the rest of the context metadata. + /// + private readonly Dictionary migratedContextRemap = new(); + + /// + /// Discard every in-flight migration remapping. + /// + /// Callers must already hold lock (this), which is what guards the map. + /// + private void ClearMigratedContextRemap() + { + Debug.Assert(Monitor.IsEntered(this), "Migration remap is guarded by lock (this)"); + + migratedContextRemap.Clear(); + } + + /// + /// Determine which context migrated data should be written to on this node. + /// + /// Contexts are assigned independently by each node, so a context that is free on the PRIMARY which chose it + /// can still be occupied here - most commonly by a deleted Vector Set whose element data has not finished being + /// cleaned up. Storing the migrated data there anyway would leave two Vector Sets sharing a context, corrupting + /// both, so in that case the migration is steered onto a context that is free locally. + /// + /// The mapping is remembered so that every record belonging to a migration resolves identically, and is + /// discarded once the index key for that migration arrives. + /// + private ulong ResolveMigratedContext(StorageSession currentSession, ulong migratedContext) + { + ulong localContext; + + lock (this) + { + if (migratedContextRemap.TryGetValue(migratedContext, out localContext)) + { + return localContext; + } + + var (contextIndex, contextValue) = ContextMetadata.DecomposeContext(migratedContext); + + // A context past the end of the local metadata has never been allocated on this node, + // so it cannot be adopted as-is + var contextKnownLocally = contextIndex < contextMetadatas.Length; + + if (contextKnownLocally && contextMetadatas[contextIndex].IsMigrating(contextIndex != 0, contextValue)) + { + // Already reserved for a migration, which is how a migration that was interrupted and resumed looks + migratedContextRemap[migratedContext] = migratedContext; + + return migratedContext; + } + + if (!contextKnownLocally || contextMetadatas[contextIndex].IsInUse(contextIndex != 0, contextValue)) + { + localContext = NextVectorSetContext(ushort.MaxValue); + + (contextIndex, contextValue) = ContextMetadata.DecomposeContext(localContext); + } + else + { + localContext = migratedContext; + + contextMetadatas[contextIndex].MarkInUse(contextIndex != 0, contextValue, ushort.MaxValue); + } + + contextMetadatas[contextIndex].MarkMigrating(contextIndex != 0, contextValue); + + migratedContextRemap[migratedContext] = localContext; + + _ = dirtyContextMetadatas.Add(contextIndex); + } + + UpdateContextMetadata(ref currentSession.vectorBasicContext); + + return localContext; + } + /// /// Vector Set adds are phrased as reads (once the index is created), so they require special handling. /// @@ -194,28 +276,21 @@ internal void HandleVectorSetAddReplication( ulong ns = BinaryPrimitives.ReadUInt32LittleEndian(elementNsBytes); // REPLICAs wouldn't have seen a reservation message, so allocate this on demand - var (contextIndex, contextValue) = ContextMetadata.DecomposeContext(ns & ~(ContextStep - 1)); - - var needsUpdate = false; - lock (this) - { - if (!contextMetadatas[contextIndex].IsMigrating(contextIndex != 0, contextValue)) - { - contextMetadatas[contextIndex].MarkInUse(contextIndex != 0, contextValue, ushort.MaxValue); - contextMetadatas[contextIndex].MarkMigrating(contextIndex != 0, contextValue); + var migratedContext = ns & ~(ContextStep - 1); + var localContext = ResolveMigratedContext(currentSession, migratedContext); - _ = dirtyContextMetadatas.Add(contextIndex); + scoped var localNamespaceBytes = elementNsBytes; - needsUpdate = true; - } - } - - if (needsUpdate) + Span remappedNamespaceBytes = stackalloc byte[sizeof(uint)]; + if (localContext != migratedContext) { - UpdateContextMetadata(ref currentSession.vectorBasicContext); + // Preserve the sub-namespace within the block, only the block itself moves + BinaryPrimitives.WriteUInt32LittleEndian(remappedNamespaceBytes, (uint)(localContext + (ns - migratedContext))); + + localNamespaceBytes = remappedNamespaceBytes; } - HandleMigratedElementKey(ref currentSession.stringBasicContext, ref currentSession.vectorBasicContext, elementNsBytes, elementKeyBytes, value); + HandleMigratedElementKey(ref currentSession.stringBasicContext, ref currentSession.vectorBasicContext, localNamespaceBytes, elementKeyBytes, value); return; } else if (input.arg1 == MigrateIndexKeyLogArg) @@ -230,36 +305,38 @@ internal void HandleVectorSetAddReplication( // but if you a migrate an EMPTY Vector Set that is not necessarily true // // So force reservation now - var (contextIndex, contextValue) = ContextMetadata.DecomposeContext(context & ~(ContextStep - 1)); - - var needsUpdate = false; - lock (this) - { - if (!contextMetadatas[contextIndex].IsMigrating(contextIndex != 0, contextValue)) - { - contextMetadatas[contextIndex].MarkInUse(contextIndex != 0, contextValue, ushort.MaxValue); - contextMetadatas[contextIndex].MarkMigrating(contextIndex != 0, contextValue); + var migratedContext = context & ~(ContextStep - 1); + var localContext = ResolveMigratedContext(currentSession, migratedContext); - _ = dirtyContextMetadatas.Add(contextIndex); + scoped var localIndexValue = value.ReadOnlySpan; - needsUpdate = true; - } - } - - if (needsUpdate) + Span remappedIndexValue = stackalloc byte[Index.Size]; + if (localContext != migratedContext) { - UpdateContextMetadata(ref currentSession.vectorBasicContext); + // The index records its own context, so it has to be rewritten to match where the data landed + localIndexValue.CopyTo(remappedIndexValue); + SetContextForMigration(remappedIndexValue, localContext + (context - migratedContext)); + + localIndexValue = remappedIndexValue; } ActiveThreadSession = currentSession; try { - HandleMigratedIndexKey(null, null, indexKey, value); + HandleMigratedIndexKey(null, null, indexKey, localIndexValue); } finally { ActiveThreadSession = null; } + + // Element records for a migration are all transmitted, applied, and acknowledged before its index + // key is sent, so no further record can name this context until a later migration reserves it + lock (this) + { + _ = migratedContextRemap.Remove(migratedContext); + } + return; } else if (input.arg1 == VectorManager.VADDSetFlagsArg) diff --git a/libs/server/Resp/Vector/VectorManager.cs b/libs/server/Resp/Vector/VectorManager.cs index d38fda784f3..095e0087759 100644 --- a/libs/server/Resp/Vector/VectorManager.cs +++ b/libs/server/Resp/Vector/VectorManager.cs @@ -172,7 +172,7 @@ private static void EnsureFilterBitmapSize(ref SpanByteAndMemory buffer, int res private readonly int dbId; - private ConcurrentDictionary recoveredIndexes; + private ConcurrentDictionary recoveredIndexes; private ConcurrentDictionary recoveredMetadata; public VectorManager(int dbId, GarnetServerOptions serverOptions, Func getTempSession, ILoggerFactory loggerFactory) @@ -308,6 +308,11 @@ public void ReconcileRecoveredState(bool requireNoReservedContexts = false) recoveredMetadata.Clear(); + // Rebuilding contextMetadatas invalidates any migration remapping built against the old array. + // An interrupted migration is treated as failed below - its context is marked for cleanup - so + // a surviving entry would steer the retried migration into a context being torn down. + ClearMigratedContextRemap(); + // If we come up and contexts are marked for migration, that means the migration FAILED // and we'd like those contexts back ASAP for (var i = 0; i < contextMetadatas.Length; i++) @@ -329,10 +334,22 @@ public void ReconcileRecoveredState(bool requireNoReservedContexts = false) } // Any non-deleted records we recovered for contexts being deleted, we need to undo that - foreach (var (context, _) in recoveredIndexes) + foreach (var (context, hashSlot) in recoveredIndexes) { var (contextIndex, contextValue) = ContextMetadata.DecomposeContext(context); + // The index record is written before the context metadata that reserves its context, so a + // recovery boundary between the two leaves a live index record pointing at a free context. + // Reserving it here keeps the context from being handed to a different Vector Set. + if (!contextMetadatas[contextIndex].IsInUse(contextIndex != 0, contextValue)) + { + contextMetadatas[contextIndex].MarkInUse(contextIndex != 0, contextValue, hashSlot); + + _ = dirtyContextMetadatas.Add(contextIndex); + + needsUpdated = true; + } + if (contextMetadatas[contextIndex].IsCleaningUp(contextIndex != 0, contextValue)) { contextMetadatas[contextIndex].ClearIsCleaningUp(contextIndex != 0, contextValue); @@ -399,7 +416,10 @@ public void RecoveredVectorSetIndexKey(ref TSourceLogRecord re } ReadIndex(record.ValueSpan, out var context, out _, out _, out _, out _, out _, out _, out _, out _); - recoveredIndexes[context] = 0; + + // The hash slot is needed to restore the context reservation in ReconcileRecoveredState, which + // has only this map to work from - the record itself is not retained past this call + recoveredIndexes[context] = HashSlotUtils.HashSlot(record.Key); } /// diff --git a/test/cluster/Garnet.test.cluster.migrate/ClusterMigrateTests.cs b/test/cluster/Garnet.test.cluster.migrate/ClusterMigrateTests.cs index b23d70fab18..129c39f0bbb 100644 --- a/test/cluster/Garnet.test.cluster.migrate/ClusterMigrateTests.cs +++ b/test/cluster/Garnet.test.cluster.migrate/ClusterMigrateTests.cs @@ -1368,6 +1368,8 @@ public async Task ClusterSimpleMigrateContinuousReadWrite(CancellationToken canc context.clusterTestUtils.SetConfigEpoch(dstNodeIndex, dstNodeIndex + 2, logger: context.logger); context.clusterTestUtils.Meet(srcNodeIndex, dstNodeIndex, logger: context.logger); context.clusterTestUtils.WaitUntilNodeIsKnown(dstNodeIndex, srcNodeIndex, logger: context.logger); + // MIGRATE is issued against the source, so the source has to know the target endpoint + context.clusterTestUtils.WaitUntilNodeIsKnown(srcNodeIndex, dstNodeIndex, logger: context.logger); var migrateSlots = new List { 0, 10 }; // Start operations @@ -1748,6 +1750,9 @@ public void ClusterMigrateDataSlotsRange() context.clusterTestUtils.SetConfigEpoch(srcNodeIndex, srcNodeIndex + 1, logger: context.logger); context.clusterTestUtils.SetConfigEpoch(dstNodeIndex, dstNodeIndex + 2, logger: context.logger); context.clusterTestUtils.Meet(srcNodeIndex, dstNodeIndex, logger: context.logger); + context.clusterTestUtils.WaitUntilNodeIsKnown(dstNodeIndex, srcNodeIndex, logger: context.logger); + // MIGRATE is issued against the source, so the source has to know the target endpoint + context.clusterTestUtils.WaitUntilNodeIsKnown(srcNodeIndex, dstNodeIndex, logger: context.logger); var keySize = 16; var keyCount = 1024; @@ -2083,6 +2088,8 @@ public void ClusterMigrateWrite() context.clusterTestUtils.SetConfigEpoch(targetNodeIndex, targetNodeIndex + 1, logger: context.logger); context.clusterTestUtils.Meet(sourceNodeIndex, targetNodeIndex, logger: context.logger); context.clusterTestUtils.WaitUntilNodeIsKnown(targetNodeIndex, sourceNodeIndex, logger: context.logger); + // Slot state transitions are issued against the source, so the source has to know the target node id + context.clusterTestUtils.WaitUntilNodeIsKnown(sourceNodeIndex, targetNodeIndex, logger: context.logger); var sourceNodeId = context.clusterTestUtils.GetNodeIdFromNode(sourceNodeIndex, context.logger); var targetNodeId = context.clusterTestUtils.GetNodeIdFromNode(targetNodeIndex, context.logger); @@ -2169,6 +2176,8 @@ public void ClusterMigrateSetCopyUpdate(CancellationToken cancellationToken) context.clusterTestUtils.SetConfigEpoch(targetNodeIndex, targetNodeIndex + 1, logger: context.logger); context.clusterTestUtils.Meet(sourceNodeIndex, targetNodeIndex, logger: context.logger); context.clusterTestUtils.WaitUntilNodeIsKnown(targetNodeIndex, sourceNodeIndex, logger: context.logger); + // Slot state transitions are issued against the source, so the source has to know the target node id + context.clusterTestUtils.WaitUntilNodeIsKnown(sourceNodeIndex, targetNodeIndex, logger: context.logger); var sourceNodeId = context.clusterTestUtils.GetNodeIdFromNode(sourceNodeIndex, context.logger); var targetNodeId = context.clusterTestUtils.GetNodeIdFromNode(targetNodeIndex, context.logger); @@ -2242,6 +2251,8 @@ public void ClusterMigrateCustomProcDelRMW(CancellationToken cancellationToken) context.clusterTestUtils.SetConfigEpoch(targetNodeIndex, targetNodeIndex + 1, logger: context.logger); context.clusterTestUtils.Meet(sourceNodeIndex, targetNodeIndex, logger: context.logger); context.clusterTestUtils.WaitUntilNodeIsKnown(targetNodeIndex, sourceNodeIndex, logger: context.logger); + // Slot state transitions are issued against the source, so the source has to know the target node id + context.clusterTestUtils.WaitUntilNodeIsKnown(sourceNodeIndex, targetNodeIndex, logger: context.logger); var sourceNodeId = context.clusterTestUtils.GetNodeIdFromNode(sourceNodeIndex, context.logger); var targetNodeId = context.clusterTestUtils.GetNodeIdFromNode(targetNodeIndex, context.logger); diff --git a/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs b/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs index 8fefccb8a43..dd0ba0cbfa5 100644 --- a/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs +++ b/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs @@ -962,6 +962,11 @@ void ClusterDivergentReplicasTest(bool performRMW, bool disableObjects, bool ckp _ = context.clusterTestUtils.AddDelSlotsRange(newPrimaryIndex, [(0, 16383)], addslot: true, context.logger); context.clusterTestUtils.BumpEpoch(newPrimaryIndex, logger: context.logger); + // Slot re-assignment settles asynchronously, so the new primary has to observe itself as the + // owner before it will serve writes instead of redirecting them + var newPrimaryId = context.clusterTestUtils.ClusterMyId(newPrimaryIndex, context.logger); + context.clusterTestUtils.WaitForSlotOwnership(newPrimaryIndex, newPrimaryId, [0, 16383], context.logger); + // New primary diverges to its own history by new random seed kvpairCount <<= 1; if (disableObjects) @@ -974,7 +979,6 @@ void ClusterDivergentReplicasTest(bool performRMW, bool disableObjects, bool ckp if (!ckptBeforeDivergence || multiCheckpointAfterDivergence) context.clusterTestUtils.Checkpoint(newPrimaryIndex, logger: context.logger); - var newPrimaryId = context.clusterTestUtils.ClusterMyId(newPrimaryIndex, context.logger); while (true) { var replicaConfig = context.clusterTestUtils.ClusterNodes(replicaIndex, context.logger); diff --git a/test/cluster/Garnet.test.cluster.vectorsets/VectorSets/ClusterVectorSetTests.cs b/test/cluster/Garnet.test.cluster.vectorsets/VectorSets/ClusterVectorSetTests.cs index 3a74b21501a..79687036978 100644 --- a/test/cluster/Garnet.test.cluster.vectorsets/VectorSets/ClusterVectorSetTests.cs +++ b/test/cluster/Garnet.test.cluster.vectorsets/VectorSets/ClusterVectorSetTests.cs @@ -1337,10 +1337,14 @@ public void VectorSetMigrateManyBySlot() ClassicAssert.IsTrue(exc0.StartsWith("Key has MOVED to ")); } + // Same convergence window the single-slot variant of this test uses: the source replica keeps + // serving the migrated keys for a while, then passes through a window where the slot is not yet + // known-stable from its point of view, before it settles on redirecting to the new primary. var start = Stopwatch.GetTimestamp(); var success = false; - while (Stopwatch.GetElapsedTime(start) < TimeSpan.FromSeconds(5)) + string lastResponse = null; + while (Stopwatch.GetElapsedTime(start) < TimeSpan.FromSeconds(15)) { try { @@ -1348,29 +1352,30 @@ public void VectorSetMigrateManyBySlot() foreach (var (key, _, _, data, _) in primary0Keys.Concat(primary1Keys)) { var exc1 = (string)context.clusterTestUtils.Execute(secondary0, "VSIM", [key, "XB8", data, "WITHSCORES", "WITHATTRIBS"], flags: CommandFlags.NoRedirect); - if (!exc1.StartsWith("Key has MOVED to ")) + lastResponse = exc1; + if (exc1 is null || !exc1.StartsWith("Key has MOVED to ")) { migrationNotFinished = true; break; } } - if (migrationNotFinished) + if (!migrationNotFinished) { - continue; + success = true; + break; } - - success = true; - break; } - catch + catch (Exception ex) { - // Secondary can still have the key for a bit - Thread.Sleep(100); + // Secondary can still have the key for a bit (VSIM returns vector data, not a string) + lastResponse = ex.Message; } + + Thread.Sleep(100); } - ClassicAssert.IsTrue(success, "Original replica still has Vector Set long after primary has completed"); + ClassicAssert.IsTrue(success, $"Original replica still has Vector Set long after primary has completed; last response was '{lastResponse}'"); // Check available on new secondary var readonlyOnReplica1 = (string)context.clusterTestUtils.Execute(secondary1, "READONLY", [], flags: CommandFlags.NoRedirect); @@ -1379,37 +1384,67 @@ public void VectorSetMigrateManyBySlot() start = Stopwatch.GetTimestamp(); success = false; + lastResponse = null; - while (Stopwatch.GetElapsedTime(start) < TimeSpan.FromSeconds(5)) + // Poll until every migrated key is readable on the new replica, and only then assert on the + // contents. ClusterTestUtils.Execute turns any exception into a single-element reply holding the + // message, and a VSIM ... WITHSCORES WITHATTRIBS hit is always three elements, so a single element + // always means the read did not land: MOVED while this replica's view of slot ownership catches up, + // CLUSTERDOWN, or an SE.Redis timeout. Asserting inside the loop would record a failure on the test + // result even for a poll that the very next one resolves. + List<(string Key, byte[] Data, byte[][] Reply)> replicated = []; + + while (Stopwatch.GetElapsedTime(start) < TimeSpan.FromSeconds(15)) { success = true; + replicated.Clear(); foreach (var (key, _, _, data, _) in primary0Keys.Concat(primary1Keys)) { - var migrateSimRes = (byte[][])context.clusterTestUtils.Execute(secondary1, "VSIM", [key, "XB8", data, "WITHSCORES", "WITHATTRIBS"], flags: CommandFlags.NoRedirect); - - if (migrateSimRes.Length == 1 && Encoding.UTF8.GetString(migrateSimRes[0]).StartsWith("Key has MOVED to ")) + byte[][] migrateSimRes; + try { + migrateSimRes = (byte[][])context.clusterTestUtils.Execute(secondary1, "VSIM", [key, "XB8", data, "WITHSCORES", "WITHATTRIBS"], flags: CommandFlags.NoRedirect); + } + catch (Exception ex) + { + lastResponse = ex.Message; success = false; break; } - ClassicAssert.AreEqual(3, migrateSimRes.Length); - - var (elem, attr, score) = expected[(key, data)]; + // A nil reply casts to a null array, so null must be handled alongside the + // single-element error reply. + if (migrateSimRes is null || migrateSimRes.Length == 1) + { + lastResponse = migrateSimRes is null ? "" : Encoding.UTF8.GetString(migrateSimRes[0]); + success = false; + break; + } - ClassicAssert.IsTrue(elem.SequenceEqual(migrateSimRes[0])); - ClassicAssert.AreEqual(score, float.Parse(Encoding.ASCII.GetString(migrateSimRes[1]))); - ClassicAssert.IsTrue(attr.SequenceEqual(migrateSimRes[2])); + replicated.Add((key, data, migrateSimRes)); } if (success) { break; } + + Thread.Sleep(100); } - ClassicAssert.IsTrue(success, "New replica hasn't replicated Vector Set long after primary has received data"); + ClassicAssert.IsTrue(success, $"New replica hasn't replicated Vector Set long after primary has received data; last response was '{lastResponse}'"); + + foreach (var (key, data, migrateSimRes) in replicated) + { + ClassicAssert.AreEqual(3, migrateSimRes.Length, $"Unexpected VSIM reply length for key '{key}'"); + + var (elem, attr, score) = expected[(key, data)]; + + ClassicAssert.IsTrue(elem.SequenceEqual(migrateSimRes[0])); + ClassicAssert.AreEqual(score, float.Parse(Encoding.ASCII.GetString(migrateSimRes[1]))); + ClassicAssert.IsTrue(attr.SequenceEqual(migrateSimRes[2])); + } } [Test] diff --git a/test/cluster/Garnet.test.cluster/ClusterTestContext.cs b/test/cluster/Garnet.test.cluster/ClusterTestContext.cs index a8f9bb071ad..f64aec38888 100644 --- a/test/cluster/Garnet.test.cluster/ClusterTestContext.cs +++ b/test/cluster/Garnet.test.cluster/ClusterTestContext.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Net; @@ -65,6 +66,14 @@ public class ClusterTestContext public CancellationTokenSource cts; + /// + /// Total time budget shared across all nodes in . Kept below the + /// teardown timeout so stalled nodes are reported precisely instead of tripping the outer guard. + /// No per-node cap is applied on top of this: a single slow node is only a problem once it + /// threatens the budget the remaining nodes still need to release their ports. + /// + private static readonly TimeSpan DisposeClusterBudget = TimeSpan.FromSeconds(45); + public void EnableGarnetLoggingEvents(GarnetTestLoggingEventType[] events) { foreach (var e in events) @@ -539,20 +548,89 @@ public GarnetServer CreateInstance( /// public void DisposeCluster() { - if (nodes != null) + if (nodes == null) + return; + + // Every node's dispose is started before any of them is waited on, and each is then waited on + // under a bounded share of one budget. GarnetServer.Dispose closes its listeners before draining + // handlers, so a node that stalls late in its own dispose cannot keep the others bound to their + // ports. Starting them inside the wait loop instead would mean a single node exhausting the + // budget left every later node undisposed, and aborting the loop on the first failure would do + // the same - either way the abandoned ports cascade into startup failures for subsequent tests + // in the same process. + var stalledNodes = new List(); + var disposeFailures = new List(); + var disposeTasks = new Task[nodes.Length]; + + for (var i = 0; i < nodes.Length; i++) + { + var node = nodes[i]; + if (node == null) + continue; + + nodes[i] = null; + logger.LogDebug("\t a. Before dispose node {i}{testName}", i, TestContext.CurrentContext.Test.Name); + + // Dispose blocks, so it gets a dedicated thread rather than a pool thread: on a two-core + // runner the pool injects threads slowly enough that a queued dispose could sit unstarted + // behind the ones already blocked, which is the same abandoned port by another route. + disposeTasks[i] = Task.Factory.StartNew(() => node.Dispose(true), TaskCreationOptions.LongRunning); + } + + var elapsed = Stopwatch.StartNew(); + + for (var i = 0; i < disposeTasks.Length; i++) { - for (var i = 0; i < nodes.Length; i++) + var disposeTask = disposeTasks[i]; + if (disposeTask == null) + continue; + + var wait = DisposeClusterBudget - elapsed.Elapsed; + + // Task.Wait rethrows a faulted dispose as an AggregateException. Letting it propagate + // would abandon every later node, which is the cascade this loop exists to prevent, so + // the fault is caught here and reported from disposeTask.Exception below. + bool completed; + try { - if (nodes[i] != null) - { - logger.LogDebug("\t a. Before dispose node {i}{testName}", i, TestContext.CurrentContext.Test.Name); - var node = nodes[i]; - nodes[i] = null; - node.Dispose(true); - logger.LogDebug("\t b. After dispose node {i}{testName}", i, TestContext.CurrentContext.Test.Name); - } + completed = wait > TimeSpan.Zero && disposeTask.Wait(wait); } + catch (AggregateException) + { + completed = true; + } + + // The dispose is already running on its own thread, so a node that outlives the budget + // still goes on to free its port; it is only the waiting that is given up here. + if (!completed) + { + stalledNodes.Add(i); + logger.LogError("\t !. Dispose stalled for node {i}{testName}", i, TestContext.CurrentContext.Test.Name); + continue; + } + + if (disposeTask.Exception is { } disposeException) + { + // Unwrapped so the reported failure names the original fault rather than the + // AggregateException the task wraps it in. + disposeFailures.Add(disposeException.InnerExceptions.Count == 1 + ? disposeException.InnerExceptions[0] + : disposeException); + logger.LogError(disposeException, "\t !. Dispose failed for node {i}{testName}", i, TestContext.CurrentContext.Test.Name); + continue; + } + + logger.LogDebug("\t b. After dispose node {i}{testName}", i, TestContext.CurrentContext.Test.Name); } + + if (stalledNodes.Count > 0) + disposeFailures.Add(new TimeoutException($"Dispose stalled for node(s): {string.Join(", ", stalledNodes)}")); + + if (disposeFailures.Count == 1) + throw disposeFailures[0]; + + if (disposeFailures.Count > 1) + throw new AggregateException("Dispose failed for multiple nodes", disposeFailures); } /// diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index 55d3f728887..de205b02ab5 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -192,7 +192,7 @@ public void GlobalsForbidden() var db = redis.GetDatabase(0); var globalFuncExc = - ClassicAssert.Throws( + TestUtils.ThrowsRedisException( () => { _ = db.ScriptEvaluate( @@ -204,13 +204,13 @@ public void GlobalsForbidden() ); ClassicAssert.IsTrue(globalFuncExc.Message.Contains("Attempt to modify a readonly table")); - var globalVar = ClassicAssert.Throws(() => db.ScriptEvaluate("global_var = 'hello'")); + var globalVar = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("global_var = 'hello'")); ClassicAssert.IsTrue(globalVar.Message.Contains("Attempt to modify a readonly table")); - var metatableUpdateOnGlobals = ClassicAssert.Throws(() => db.ScriptEvaluate("setmetatable(_G, nil)")); + var metatableUpdateOnGlobals = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("setmetatable(_G, nil)")); ClassicAssert.IsTrue(metatableUpdateOnGlobals.Message.Contains("Attempt to modify a readonly table")); - var rawSetG = ClassicAssert.Throws(() => db.ScriptEvaluate("rawset(_G, 'hello', 'world')")); + var rawSetG = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("rawset(_G, 'hello', 'world')")); ClassicAssert.IsTrue(globalVar.Message.Contains("Attempt to modify a readonly table")); } @@ -228,7 +228,7 @@ public void ReadOnlyGlobalTables() foreach (var illegal in illegalToModify) { - var exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"table.insert({illegal}, 'foo')")); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"table.insert({illegal}, 'foo')")); ClassicAssert.IsTrue(exc.Message.Contains("Attempt to modify a readonly table")); } @@ -481,7 +481,7 @@ public void CanDoScriptFlush() server.ScriptFlush(); // Assert the script is not found - _ = Assert.Throws(() => db.ScriptEvaluate(scriptId)); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate(scriptId)); } [Test] @@ -551,11 +551,11 @@ public void FailureStatusReturn() var db = redis.GetDatabase(0); var statusReplyScript = "return redis.error_reply('GET')"; - var excReply = ClassicAssert.Throws(() => db.ScriptEvaluate(statusReplyScript)); + var excReply = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate(statusReplyScript)); ClassicAssert.AreEqual("ERR GET", excReply.Message); var directReplyScript = "return { err = 'Failure' }"; - var excDirect = ClassicAssert.Throws(() => db.ScriptEvaluate(directReplyScript)); + var excDirect = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate(directReplyScript)); ClassicAssert.AreEqual("Failure", excDirect.Message); } @@ -567,11 +567,11 @@ public void MiscMath() // ATan2 { - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.atan2()")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.atan2(1)")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.atan2(1, 2, 3)")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.atan2('a', 1)")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.atan2(1, 'b')")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.atan2()")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.atan2(1)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.atan2(1, 2, 3)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.atan2('a', 1)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.atan2(1, 'b')")); var val = (string)db.ScriptEvaluate("return tostring(math.atan2(0.1, 0.2))"); ClassicAssert.AreEqual(Math.Round(Math.Atan2(0.1, 0.2), 14).ToString(), val); @@ -579,9 +579,9 @@ public void MiscMath() // Cosh { - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.cosh()")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.cosh('a')")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.cosh(1, 2)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.cosh()")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.cosh('a')")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.cosh(1, 2)")); var val = (string)db.ScriptEvaluate("return tostring(math.cosh(0.1))"); ClassicAssert.AreEqual(Math.Round(Math.Cosh(0.1), 14).ToString(), val); @@ -589,9 +589,9 @@ public void MiscMath() // Log10 { - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.log10()")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.log10('a')")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.log10(1, 2)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.log10()")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.log10('a')")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.log10(1, 2)")); var val = (string)db.ScriptEvaluate("return tostring(math.log10(0.1))"); ClassicAssert.AreEqual("-1.0", val); @@ -599,11 +599,11 @@ public void MiscMath() // Pow { - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.pow()")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.pow(1)")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.pow(1, 2, 3)")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.pow('a', 1)")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.pow(1, 'b')")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.pow()")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.pow(1)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.pow(1, 2, 3)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.pow('a', 1)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.pow(1, 'b')")); var val = (string)db.ScriptEvaluate("return tostring(math.pow(0.1, 0.2))"); ClassicAssert.AreEqual(Math.Round(Math.Pow(0.1, 0.2), 14).ToString(), val); @@ -611,9 +611,9 @@ public void MiscMath() // Sinh { - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.sinh()")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.sinh('a')")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.sinh(1, 2)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.sinh()")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.sinh('a')")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.sinh(1, 2)")); var val = (string)db.ScriptEvaluate("return tostring(math.sinh(0.1))"); ClassicAssert.AreEqual(Math.Round(Math.Sinh(0.1), 14).ToString(), val); @@ -621,9 +621,9 @@ public void MiscMath() // Tanh { - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.tanh()")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.tanh('a')")); - _ = ClassicAssert.Throws(() => db.ScriptEvaluate("math.tanh(1, 2)")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.tanh()")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.tanh('a')")); + _ = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("math.tanh(1, 2)")); var val = (string)db.ScriptEvaluate("return tostring(math.tanh(0.2))"); ClassicAssert.AreEqual(Math.Round(Math.Tanh(0.2), 14).ToString(), val); @@ -665,10 +665,10 @@ public void RedisSha1Hex() var resTable = (string)db.ScriptEvaluate("return redis.sha1hex({ 1234 })"); ClassicAssert.AreEqual("da39a3ee5e6b4b0d3255bfef95601890afd80709", resTable); - var excEmpty = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.sha1hex()")); + var excEmpty = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.sha1hex()")); ClassicAssert.IsTrue(excEmpty.Message.StartsWith("ERR wrong number of arguments")); - var excTwo = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.sha1hex('a', 'b')")); + var excTwo = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.sha1hex('a', 'b')")); ClassicAssert.IsTrue(excTwo.Message.StartsWith("ERR wrong number of arguments")); } @@ -678,16 +678,16 @@ public void RedisLog() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); - var excZero = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.log()")); + var excZero = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.log()")); ClassicAssert.IsTrue(excZero.Message.StartsWith("ERR redis.log() requires two arguments or more.")); - var excOne = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.log(redis.LOG_DEBUG)")); + var excOne = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.log(redis.LOG_DEBUG)")); ClassicAssert.IsTrue(excOne.Message.StartsWith("ERR redis.log() requires two arguments or more.")); - var excBadLevelType = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.log('hello', 'world')")); + var excBadLevelType = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.log('hello', 'world')")); ClassicAssert.IsTrue(excBadLevelType.Message.StartsWith("ERR First argument must be a number (log level).")); - var excBadLevelValue = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.log(-1, 'world')")); + var excBadLevelValue = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.log(-1, 'world')")); ClassicAssert.IsTrue(excBadLevelValue.Message.StartsWith("ERR Invalid debug level.")); // Test logs at each level @@ -723,7 +723,7 @@ public void RedisSetRepl() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); - var excNotSupported = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.set_repl(redis.REPL_ALL)")); + var excNotSupported = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.set_repl(redis.REPL_ALL)")); ClassicAssert.IsTrue(excNotSupported.Message.StartsWith("ERR redis.set_repl is not supported in Garnet")); var constantsDefined = (int[])db.ScriptEvaluate("return {redis.REPL_ALL, redis.REPL_AOF, redis.REPL_REPLICA, redis.REPL_SLAVE, redis.REPL_NONE}"); @@ -747,10 +747,10 @@ public void RedisDebugAndBreakpoint() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); - var excBreakpoint = ClassicAssert.Throws(() => db.ScriptEvaluate("redis.breakpoint()")); + var excBreakpoint = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("redis.breakpoint()")); ClassicAssert.IsTrue(excBreakpoint.Message.StartsWith("ERR redis.breakpoint is not supported in Garnet")); - var excDebug = ClassicAssert.Throws(() => db.ScriptEvaluate("redis.debug('hello')")); + var excDebug = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("redis.debug('hello')")); ClassicAssert.IsTrue(excDebug.Message.StartsWith("ERR redis.debug is not supported in Garnet")); } @@ -765,16 +765,16 @@ public void RedisAclCheckCmd() using var denyRedis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(authUsername: "deny")); var denyDB = denyRedis.GetDatabase(0); - var noArgs = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.acl_check_cmd()")); + var noArgs = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.acl_check_cmd()")); ClassicAssert.IsTrue(noArgs.Message.StartsWith("ERR Please specify at least one argument for this redis lib call")); - var invalidCmdArgType = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.acl_check_cmd({123})")); + var invalidCmdArgType = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.acl_check_cmd({123})")); ClassicAssert.IsTrue(invalidCmdArgType.Message.StartsWith("ERR Lua redis lib command arguments must be strings or integers")); - var invalidCmd = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.acl_check_cmd('nope')")); + var invalidCmd = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.acl_check_cmd('nope')")); ClassicAssert.IsTrue(invalidCmd.Message.StartsWith("ERR Invalid command passed to redis.acl_check_cmd()")); - var invalidArgType = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.acl_check_cmd('GET', {123})")); + var invalidArgType = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.acl_check_cmd('GET', {123})")); ClassicAssert.IsTrue(invalidArgType.Message.StartsWith("ERR Lua redis lib command arguments must be strings or integers")); var canRun = (bool)db.ScriptEvaluate("return redis.acl_check_cmd('GET')"); @@ -814,13 +814,13 @@ public void RedisSetResp() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); - var noArgs = ClassicAssert.Throws(() => db.ScriptEvaluate("redis.setresp()")); + var noArgs = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("redis.setresp()")); ClassicAssert.IsTrue(noArgs.Message.StartsWith("ERR redis.setresp() requires one argument.")); - var tooManyArgs = ClassicAssert.Throws(() => db.ScriptEvaluate("redis.setresp(1, 2)")); + var tooManyArgs = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("redis.setresp(1, 2)")); ClassicAssert.IsTrue(tooManyArgs.Message.StartsWith("ERR redis.setresp() requires one argument.")); - var badArg = ClassicAssert.Throws(() => db.ScriptEvaluate("redis.setresp({123})")); + var badArg = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("redis.setresp({123})")); ClassicAssert.IsTrue(badArg.Message.StartsWith("ERR RESP version must be 2 or 3.")); var resp2 = db.ScriptEvaluate("redis.setresp(2)"); @@ -829,7 +829,7 @@ public void RedisSetResp() var resp3 = db.ScriptEvaluate("redis.setresp(3)"); ClassicAssert.IsTrue(resp3.IsNull); - var badRespVersion = ClassicAssert.Throws(() => db.ScriptEvaluate("redis.setresp(1)")); + var badRespVersion = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("redis.setresp(1)")); ClassicAssert.IsTrue(badRespVersion.Message.StartsWith("ERR RESP version must be 2 or 3.")); } @@ -974,7 +974,7 @@ public void ScriptExistsErrors() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); - var exc = ClassicAssert.Throws(() => db.Execute("SCRIPT", "EXISTS")); + var exc = TestUtils.ThrowsRedisException(() => db.Execute("SCRIPT", "EXISTS")); ClassicAssert.AreEqual("ERR wrong number of arguments for 'script|exists' command", exc.Message); } @@ -986,13 +986,13 @@ public void ScriptFlushErrors() // > 1 args { - var exc = ClassicAssert.Throws(() => db.Execute("SCRIPT", "FLUSH", "ASYNC", "BAR")); + var exc = TestUtils.ThrowsRedisException(() => db.Execute("SCRIPT", "FLUSH", "ASYNC", "BAR")); ClassicAssert.AreEqual("ERR SCRIPT FLUSH only support SYNC|ASYNC option", exc.Message); } // 1 arg, but not ASYNC or SYNC { - var exc = ClassicAssert.Throws(() => db.Execute("SCRIPT", "FLUSH", "NOW")); + var exc = TestUtils.ThrowsRedisException(() => db.Execute("SCRIPT", "FLUSH", "NOW")); ClassicAssert.AreEqual("ERR SCRIPT FLUSH only support SYNC|ASYNC option", exc.Message); } } @@ -1020,8 +1020,8 @@ public void MultiSessionScriptFlush() _ = db1.Execute("SCRIPT", "FLUSH", "SYNC"); - var exc1 = ClassicAssert.Throws(() => db1.Execute("EVALSHA", hash, "0")); - var exc2 = ClassicAssert.Throws(() => db2.Execute("EVALSHA", hash, "0")); + var exc1 = TestUtils.ThrowsRedisException(() => db1.Execute("EVALSHA", hash, "0")); + var exc2 = TestUtils.ThrowsRedisException(() => db2.Execute("EVALSHA", hash, "0")); ClassicAssert.True(exc1.Message.StartsWith("NOSCRIPT ")); ClassicAssert.True(exc2.Message.StartsWith("NOSCRIPT ")); @@ -1065,13 +1065,13 @@ public void ScriptLoadErrors() // 0 args { - var exc = ClassicAssert.Throws(() => db.Execute("SCRIPT", "LOAD")); + var exc = TestUtils.ThrowsRedisException(() => db.Execute("SCRIPT", "LOAD")); ClassicAssert.AreEqual("ERR wrong number of arguments for 'script|load' command", exc.Message); } // > 1 args { - var exc = ClassicAssert.Throws(() => db.Execute("SCRIPT", "LOAD", "return 'foo'", "return 'bar'")); + var exc = TestUtils.ThrowsRedisException(() => db.Execute("SCRIPT", "LOAD", "return 'foo'", "return 'bar'")); ClassicAssert.AreEqual("ERR wrong number of arguments for 'script|load' command", exc.Message); } } @@ -1111,7 +1111,7 @@ public void ScriptExistsMultiple() private static void DoErroneousRedisCall(IDatabase db, string[] args, string expectedError) { - var exc = Assert.Throws(() => db.ScriptEvaluate($"return redis.call({string.Join(',', args)})")); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return redis.call({string.Join(',', args)})")); ClassicAssert.IsNotNull(exc); StringAssert.StartsWith(expectedError, exc!.Message); } @@ -1381,7 +1381,7 @@ public void IntentionalOOM() var loadedScriptOOM = scriptOOM.Load(redis.GetServers()[0]); // OOM actually happens and is reported - var exc = ClassicAssert.Throws(() => db.ScriptEvaluate(loadedScriptOOM, new { Ctrl = "OOM" })); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate(loadedScriptOOM, new { Ctrl = "OOM" })); ClassicAssert.AreEqual("ERR Lua encountered an error: not enough memory", exc.Message); // We can still run the script without issue (with non-crashing args) afterwards @@ -1472,7 +1472,7 @@ public void Issue939() } // Finally, check that nil is an illegal argument - var exc = ClassicAssert.Throws(() => db.ScriptEvaluate("return redis.call('GET', nil)")); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.call('GET', nil)")); ClassicAssert.True(exc.Message.StartsWith("ERR Lua redis lib command arguments must be strings or integers")); } @@ -1489,10 +1489,10 @@ public void Issue1079() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(); - var brokenExc1 = ClassicAssert.Throws(() => db.Execute("EVAL", BrokenScript, 0)); + var brokenExc1 = TestUtils.ThrowsRedisException(() => db.Execute("EVAL", BrokenScript, 0)); ClassicAssert.True(brokenExc1.Message.StartsWith("Compilation error: ")); - var brokenExc2 = ClassicAssert.Throws(() => db.Execute("EVAL", BrokenScript, 0)); + var brokenExc2 = TestUtils.ThrowsRedisException(() => db.Execute("EVAL", BrokenScript, 0)); ClassicAssert.AreEqual(brokenExc1.Message, brokenExc2.Message); var success = (string)db.Execute("EVAL", FixedScript, 0); @@ -1782,7 +1782,7 @@ public void Resp2ToLuaConversions() ClassicAssert.AreEqual("table", simpleStringRes[0]); ClassicAssert.AreEqual("PONG", simpleStringRes[1]); - var errExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return { err = 'ERR mapped to ERR response' }")); + var errExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return { err = 'ERR mapped to ERR response' }")); ClassicAssert.AreEqual("ERR mapped to ERR response", errExc.Message); var nullBulkRes = (string[])db.ScriptEvaluate("local res = redis.call('GET', KEYS[1]); return { type(res), tostring(res) };", [(RedisKey)"not-set-ever"]); @@ -1814,14 +1814,14 @@ public void NoScriptCommandsForbidden() foreach (var (cmd, _) in fullCommands) { - var exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"redis.call('{cmd}')")); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"redis.call('{cmd}')")); ClassicAssert.True(exc.Message.StartsWith("ERR This Redis command is not allowed from script"), $"Allowed NoScript command: {cmd}"); } var subCommands = allCommands.Where(static kv => (kv.Value.SubCommands?.Length ?? 0) > 0).SelectMany(static kv => kv.Value.SubCommands.Where(static t => t.Flags.HasFlag(RespCommandFlags.NoScript)).Select(t => (kv.Key, t.Name.Split('|')[1]))); foreach (var (cmd, subCmd) in subCommands) { - var exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"redis.call('{cmd}', '{subCmd}')")); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"redis.call('{cmd}', '{subCmd}')")); ClassicAssert.True(exc.Message.StartsWith("ERR This Redis command is not allowed from script"), $"Allowed NoScript command: {cmd}|{subCmd}"); } } @@ -1842,20 +1842,20 @@ public void PermissionsEnforced() ClassicAssert.AreEqual("bar", allowRes[0]); // Not a lot of sub commands a non-admin can run, so use CLUSTER|MYID and check the exception - var allowSubExc = ClassicAssert.Throws(() => allowDb.ScriptEvaluate("return redis.call('CLUSTER', 'MYID')")); + var allowSubExc = TestUtils.ThrowsRedisException(() => allowDb.ScriptEvaluate("return redis.call('CLUSTER', 'MYID')")); ClassicAssert.False(allowSubExc.Message.Contains("NOPERM")); - var exc = ClassicAssert.Throws(() => denyDb.ScriptEvaluate("return redis.call('GET', 'foo')")); + var exc = TestUtils.ThrowsRedisException(() => denyDb.ScriptEvaluate("return redis.call('GET', 'foo')")); ClassicAssert.IsTrue(exc.Message.Contains("NOPERM")); - var excSub = ClassicAssert.Throws(() => denyDb.ScriptEvaluate("return redis.call('CLUSTER', 'MYID')")); + var excSub = TestUtils.ThrowsRedisException(() => denyDb.ScriptEvaluate("return redis.call('CLUSTER', 'MYID')")); ClassicAssert.IsTrue(excSub.Message.Contains("NOPERM")); // SET is dispatched through a separate fast path in LuaRunner, so cover it explicitly using var denySetRedis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(authUsername: "denyset")); var denySetDb = denySetRedis.GetDatabase(); - var excSet = ClassicAssert.Throws(() => denySetDb.ScriptEvaluate("return redis.call('SET', 'foo', 'baz')")); + var excSet = TestUtils.ThrowsRedisException(() => denySetDb.ScriptEvaluate("return redis.call('SET', 'foo', 'baz')")); ClassicAssert.IsTrue(excSet.Message.Contains("NOPERM"), $"Expected NOPERM for denied SET, got: {excSet.Message}"); var allowSetRes = (string[])denySetDb.ScriptEvaluate("return redis.call('GET', 'foo')"); @@ -1890,7 +1890,7 @@ public void IntentionalTimeout() var loadedScriptTimeout = scriptTimeout.Load(redis.GetServers()[0]); // Timeout actually happens and is reported - var exc = ClassicAssert.Throws(() => db.ScriptEvaluate(loadedScriptTimeout, new { Ctrl = "Timeout" })); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate(loadedScriptTimeout, new { Ctrl = "Timeout" })); ClassicAssert.AreEqual("ERR Lua script exceeded configured timeout", exc.Message); // We can still run the script without issue (with non-crashing args) afterwards @@ -2001,12 +2001,12 @@ public void Bit() // tobit { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.tobit()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.tobit()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("tobit")); // Extra arguments are legal, but ignored - var badTypeExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.tobit({})")); + var badTypeExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.tobit({})")); ClassicAssert.True(badTypeExc.Message.Contains("bad argument") && badTypeExc.Message.Contains("tobit")); // Rules are suprisingly subtle, so test a bunch of tricky values @@ -2043,15 +2043,15 @@ public void Bit() // tohex { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.tohex()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.tohex()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("tohex")); // Extra arguments are legal, but ignored - var badType1Exc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.tohex({})")); + var badType1Exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.tohex({})")); ClassicAssert.True(badType1Exc.Message.Contains("bad argument") && badType1Exc.Message.Contains("tohex")); - var badType2Exc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.tohex(1, {})")); + var badType2Exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.tohex(1, {})")); ClassicAssert.True(badType2Exc.Message.Contains("bad argument") && badType2Exc.Message.Contains("tohex")); // Make sure casing is handled correctly @@ -2093,12 +2093,12 @@ public void Bit() // bswap { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.bswap()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.bswap()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("bswap")); // Extra arguments are legal, but ignored - var badTypeExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.bswap({})")); + var badTypeExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.bswap({})")); ClassicAssert.True(badTypeExc.Message.Contains("bad argument") && badTypeExc.Message.Contains("bswap")); // Just brute force a bunch of trial values @@ -2123,12 +2123,12 @@ public void Bit() // bnot { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.bnot()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.bnot()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("bnot")); // Extra arguments are legal, but ignored - var badTypeExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return bit.bnot({})")); + var badTypeExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return bit.bnot({})")); ClassicAssert.True(badTypeExc.Message.Contains("bad argument") && badTypeExc.Message.Contains("bnot")); foreach (var input in new int[] { 0, 1, 2, 4, 8, 32, 64, 128, 256, 0x70F0_F0F0, 0x6BCD_EF01, int.MinValue, int.MaxValue, -1 }) @@ -2150,16 +2150,16 @@ public void Bit() foreach (var op in ops) { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return bit.{op.Name}()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return bit.{op.Name}()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains(op.Name)); - var badType1Exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return bit.{op.Name}({{}})")); + var badType1Exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return bit.{op.Name}({{}})")); ClassicAssert.True(badType1Exc.Message.Contains("bad argument") && badType1Exc.Message.Contains(op.Name)); - var badType2Exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return bit.{op.Name}(1, {{}})")); + var badType2Exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return bit.{op.Name}(1, {{}})")); ClassicAssert.True(badType2Exc.Message.Contains("bad argument") && badType2Exc.Message.Contains(op.Name)); - var badType3Exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return bit.{op.Name}(1, 2, {{}})")); + var badType3Exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return bit.{op.Name}(1, 2, {{}})")); ClassicAssert.True(badType3Exc.Message.Contains("bad argument") && badType3Exc.Message.Contains(op.Name)); // Gin up some unusual values and test them in different combinations @@ -2198,13 +2198,13 @@ public void Bit() foreach (var op in ops) { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return bit.{op.Name}()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return bit.{op.Name}()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains(op.Name)); - var badType1Exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return bit.{op.Name}({{}})")); + var badType1Exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return bit.{op.Name}({{}})")); ClassicAssert.True(badType1Exc.Message.Contains("bad argument") && badType1Exc.Message.Contains(op.Name)); - var badType2Exc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return bit.{op.Name}(1, {{}})")); + var badType2Exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return bit.{op.Name}(1, {{}})")); ClassicAssert.True(badType2Exc.Message.Contains("bad argument") && badType2Exc.Message.Contains(op.Name)); // Extra args are allowed, but ignored @@ -2230,13 +2230,13 @@ public void CJson() // Encoding { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cjson.encode()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cjson.encode()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("encode")); - var twoArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cjson.encode(1, 2)")); + var twoArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cjson.encode(1, 2)")); ClassicAssert.True(twoArgExc.Message.Contains("bad argument") && twoArgExc.Message.Contains("encode")); - var badTypeExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cjson.encode((function() end))")); + var badTypeExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cjson.encode((function() end))")); ClassicAssert.True(badTypeExc.Message.Contains("Cannot serialise")); var nilResp = (string)db.ScriptEvaluate("return cjson.encode(nil)"); @@ -2311,7 +2311,7 @@ public void CJson() ClassicAssert.AreEqual(new string('[', 1000) + 1 + new string(']', 1000), deeplyNestedButLegal); var deeplyNestedExc = - ClassicAssert.Throws( + TestUtils.ThrowsRedisException( () => db.ScriptEvaluate( @"local nested = 1 for x = 1, 1001 do @@ -2326,16 +2326,16 @@ public void CJson() // Decoding { - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cjson.decode()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cjson.decode()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("decode")); - var twoArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cjson.decode(1, 2)")); + var twoArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cjson.decode(1, 2)")); ClassicAssert.True(twoArgExc.Message.Contains("bad argument") && twoArgExc.Message.Contains("decode")); - var badTypeExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cjson.decode({})")); + var badTypeExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cjson.decode({})")); ClassicAssert.True(badTypeExc.Message.Contains("bad argument") && badTypeExc.Message.Contains("decode")); - var badFormatExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cjson.decode('hello world')")); + var badFormatExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cjson.decode('hello world')")); ClassicAssert.True(badFormatExc.Message.Contains("Expected value but found invalid token")); var nullDecode = (string)db.ScriptEvaluate("return type(cjson.decode('null'))"); @@ -2387,7 +2387,7 @@ public void CJson() } ClassicAssert.AreEqual(0, deeplyNestedButLegalCur.Length); - var deeplyNestedExc = ClassicAssert.Throws(() => db.ScriptEvaluate($"return cjson.decode('{new string('[', 1001)}{new string(']', 1001)}')")); + var deeplyNestedExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate($"return cjson.decode('{new string('[', 1001)}{new string(']', 1001)}')")); ClassicAssert.True(deeplyNestedExc.Message.Contains("Found too many nested data structures")); } } @@ -2398,7 +2398,7 @@ public void CMsgPackPack() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(); - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cmsgpack.pack()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cmsgpack.pack()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("pack")); // Multiple args are legal, and concat @@ -2592,13 +2592,13 @@ public void CMsgPackUnpack() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(); - var noArgExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cmsgpack.unpack()")); + var noArgExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cmsgpack.unpack()")); ClassicAssert.True(noArgExc.Message.Contains("bad argument") && noArgExc.Message.Contains("unpack")); // Multiple arguments are allowed, but ignored // Table ends before it should - var badDataExc = ClassicAssert.Throws(() => db.ScriptEvaluate("return cmsgpack.unpack('\\220\\0\\96')")); + var badDataExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return cmsgpack.unpack('\\220\\0\\96')")); ClassicAssert.True(badDataExc.Message.Contains("Missing bytes in input")); var nullResp = (string)db.ScriptEvaluate($"return type(cmsgpack.unpack({ToLuaString(0xC0)}))"); @@ -2906,31 +2906,31 @@ public void StructPackOnErrors() var db = redis.GetDatabase(); // Not power of two - var excNotPowerOfTwoArgs = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.pack('!9')")); + var excNotPowerOfTwoArgs = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.pack('!9')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excNotPowerOfTwoArgs.Message); // Invalid format - var excFormat = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.pack(123, 123)")); + var excFormat = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.pack(123, 123)")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to pack", excFormat.Message); // Format opt size exceed max int size 32 - var excOptSize = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.pack('I64', 512)")); + var excOptSize = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.pack('I64', 512)")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excOptSize.Message); // Format opt size integeral size overflow - var excOptSizeOverflow = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.pack('I2147483648', 512)")); + var excOptSizeOverflow = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.pack('I2147483648', 512)")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excOptSizeOverflow.Message); // String too short, expected at least {size} bytes but got {l} - var excArgPack = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.pack('c6', 'hello')")); + var excArgPack = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.pack('c6', 'hello')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to pack", excArgPack.Message); // Invalid Control Options - var excControlOptions = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.pack('@I', 'hello')")); + var excControlOptions = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.pack('@I', 'hello')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excControlOptions.Message); // Invalid alignment - var excBadAlignment = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.pack('!9c1', 'Z')")); + var excBadAlignment = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.pack('!9c1', 'Z')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excBadAlignment.Message); } @@ -3063,55 +3063,55 @@ public void StructUnpackOnErrors() var db = redis.GetDatabase(); // Invalid format - var excFormat = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack(123, 123)")); + var excFormat = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack(123, 123)")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excFormat.Message); // Format opt size exceed max int size 32 - var excOptSize = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('I64', '\\000\\000\\000\\000')")); + var excOptSize = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('I64', '\\000\\000\\000\\000')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excOptSize.Message); // Format opt size integeral size overflow - var excOptSizeOverflow = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('I2147483648', '\\000\\002')")); + var excOptSizeOverflow = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('I2147483648', '\\000\\002')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excOptSizeOverflow.Message); // String too short, expected at least {size} bytes but got {l} - var excArgPack = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('c6', 'hello')")); + var excArgPack = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('c6', 'hello')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excArgPack.Message); // Invalid Control Options - var excControlOptions = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('@I', 'hello')")); + var excControlOptions = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('@I', 'hello')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excControlOptions.Message); // Missing argument - var excMissingArgs = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('!8')")); + var excMissingArgs = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('!8')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excMissingArgs.Message); // Invalid number of arguments - var excBadAlignment = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('!5', '\\157\\255\\255\\255')")); + var excBadAlignment = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('!5', '\\157\\255\\255\\255')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excBadAlignment.Message); // Invalid Third argument - var excBadThirdArg = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('@I', 'hello', 'test')")); + var excBadThirdArg = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('@I', 'hello', 'test')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excBadThirdArg.Message); // Invalid Third argument - var excArgTooShort = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('c4', '\\065\\066')")); + var excArgTooShort = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('c4', '\\065\\066')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excArgTooShort.Message); // Missing character size - var excMissingCharacterSize = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('c0', 'dynamic')")); + var excMissingCharacterSize = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('c0', 'dynamic')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excMissingCharacterSize.Message); // Invalid character size - var excInvalidCharacterZeroSize = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('sc0', 'size\\000dynamic')")); + var excInvalidCharacterZeroSize = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('sc0', 'size\\000dynamic')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excInvalidCharacterZeroSize.Message); // Invalid String without terminator - var excBadString = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('s', 'hello')")); + var excBadString = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('s', 'hello')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", excBadString.Message); // Third pos has to be greater than 0 - var unpackBadPosRes = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.unpack('s', 'hello', 0)")); + var unpackBadPosRes = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.unpack('s', 'hello', 0)")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to unpack", unpackBadPosRes.Message); } @@ -3155,16 +3155,16 @@ public void StructSizeOnErrors() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(); - var excFormatString = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.size('s')")); + var excFormatString = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.size('s')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excFormatString.Message); - var excFormatCharacter = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.size('c0')")); + var excFormatCharacter = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.size('c0')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excFormatCharacter.Message); - var excBadControlOptions = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.size('@I')")); + var excBadControlOptions = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.size('@I')")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excBadControlOptions.Message); - var excBadFormat = ClassicAssert.Throws(() => db.ScriptEvaluate("return struct.size(123)")); + var excBadFormat = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return struct.size(123)")); ClassicAssert.AreEqual("ERR Lua encountered an error: bad argument to format", excBadFormat.Message); } @@ -3266,10 +3266,10 @@ public void LoadString() var db = redis.GetDatabase(); // load and loadstring are not part of the sandbox allowed functions - var loadExc = ClassicAssert.Throws(() => db.ScriptEvaluate("local x = load('return 123'); return x()")); + var loadExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("local x = load('return 123'); return x()")); ClassicAssert.True(loadExc.Message.Contains("attempt to call a nil value")); - var loadstringExc = ClassicAssert.Throws(() => db.ScriptEvaluate("local x = loadstring('return 123'); return x()")); + var loadstringExc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("local x = loadstring('return 123'); return x()")); ClassicAssert.True(loadstringExc.Message.Contains("attempt to call a nil value")); } @@ -3362,7 +3362,7 @@ public void StressTimeouts() else { // Periodically cause a timeout - var exc = ClassicAssert.Throws(() => db.ScriptEvaluate(loadedScriptTimeout, timeout)); + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate(loadedScriptTimeout, timeout)); ClassicAssert.AreEqual("ERR Lua script exceeded configured timeout", exc.Message); } } @@ -3546,7 +3546,7 @@ public void LuaSubscribe_StillNoScriptBlocked() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); - var exc = ClassicAssert.Throws(() => + var exc = TestUtils.ThrowsRedisException(() => db.ScriptEvaluate("return redis.call('SUBSCRIBE', KEYS[1])", [new RedisKey("any_channel")])); ClassicAssert.IsTrue(exc.Message.Contains("not allowed from script"), diff --git a/test/standalone/Garnet.test.vectorset/ExceptionInjectionShutdownTests.cs b/test/standalone/Garnet.test.vectorset/ExceptionInjectionShutdownTests.cs new file mode 100644 index 00000000000..6598430876e --- /dev/null +++ b/test/standalone/Garnet.test.vectorset/ExceptionInjectionShutdownTests.cs @@ -0,0 +1,152 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System; +using System.Threading.Tasks; +using Garnet.common; +using NUnit.Framework; +using NUnit.Framework.Legacy; +using StackExchange.Redis; + +namespace Garnet.test +{ + /// + /// A thread parked in is only released by + /// , but the cleanup a test runs on its way out + /// is . A test that leaves between a waiter + /// arriving and being re-enabled strands that waiter, and because the waiter is a server thread holding + /// a pooled network buffer, LimitedFixedBufferPool.Dispose then spins forever on a reference that + /// is never returned - hanging the whole test process instead of failing one test. + /// + [TestFixture] + public class ExceptionInjectionShutdownTests : TestBase + { + private const ExceptionInjectionType Pause = ExceptionInjectionType.VectorSet_Pause_Before_Synthetic_Replication_Rmw; + + private global::Garnet.GarnetServer server; + + [SetUp] + public void Setup() + { + TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); + } + + [TearDown] + public void TearDown() + { + ExceptionInjectionHelper.DisableException(Pause); + + server?.Dispose(); + server = null; + + TestUtils.DeleteDirectory(TestUtils.MethodTestDir); + TestUtils.OnTearDown(); + } + + [Test] + public void ServerDisposeCompletesWhileAnInjectionPointIsParked() + { + TestUtils.IgnoreIfExceptionInjectionDisabled(); + + server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, enableVectorSetPreview: true); + server.Start(); + + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + ExceptionInjectionHelper.EnableException(Pause); + + // Parks inside ReplicateVectorSetAdd while holding a pooled network buffer. + _ = Task.Run(() => db.Execute("VADD", ["parked-vs", "VALUES", "3", "1", "2", "3", "elem"])); + + // ResetAndWaitAsync clears the flag when it arrives, so this is the arrival signal. + ClassicAssert.IsTrue( + ExceptionInjectionHelper.WaitOnClearAsync(Pause).Wait(TimeSpan.FromSeconds(30)), + "the server never reached the injection point"); + + // What a failing test leaves behind: the flag is cleared, so nothing will ever release the waiter. + ExceptionInjectionHelper.DisableException(Pause); + + // Hand ownership to the dispose task before starting it: if the waiter is stranded this call + // never returns, and TearDown must not go on to block the run on the same disposal. + var disposing = server; + server = null; + + var dispose = Task.Run(() => disposing.Dispose(deleteDir: false)); + + ClassicAssert.IsTrue( + dispose.Wait(TimeSpan.FromSeconds(30)), + "GarnetServer.Dispose() did not complete: a parked injection point kept a pooled buffer " + + "checked out, so the buffer pool drain never finished"); + } + + /// + /// Releasing only the waiters that are already parked leaves the same hang one moment later: + /// disposal closes listeners before it drains handlers, so a request that is already in flight can + /// reach a still-armed injection point after shutdown has begun and park there instead. Nothing + /// re-enables it, so it strands its pooled buffer exactly as before. + /// + [Test] + public void ArrivingAtAnInjectionPointDuringShutdownDoesNotPark() + { + TestUtils.IgnoreIfExceptionInjectionDisabled(); + + ExceptionInjectionHelper.EnableException(Pause); + ExceptionInjectionHelper.SuspendParking(); + + try + { + var arriving = Task.Run(() => ExceptionInjectionHelper.ResetAndWait(Pause)); + + ClassicAssert.IsTrue( + arriving.Wait(TimeSpan.FromSeconds(30)), + "a caller reaching an injection point after shutdown began parked instead of proceeding, " + + "so it would still be holding its pooled buffer when the drain runs"); + } + finally + { + ExceptionInjectionHelper.ResumeParking(); + ExceptionInjectionHelper.DisableException(Pause); + } + } + + /// + /// The suspension must not outlive the shutdown that raised it, or a later test in the same process + /// would find its injection points no longer pausing anything. + /// + [Test] + public void ParkingResumesAfterShutdownCompletes() + { + TestUtils.IgnoreIfExceptionInjectionDisabled(); + + ExceptionInjectionHelper.SuspendParking(); + ExceptionInjectionHelper.ResumeParking(); + + ExceptionInjectionHelper.EnableException(Pause); + + try + { + var parked = Task.Run(() => ExceptionInjectionHelper.ResetAndWait(Pause)); + + ClassicAssert.IsTrue( + ExceptionInjectionHelper.WaitOnClearAsync(Pause).Wait(TimeSpan.FromSeconds(30)), + "the caller never reached the injection point"); + + ClassicAssert.IsFalse( + parked.Wait(TimeSpan.FromSeconds(1)), + "the injection point did not pause: a suspension leaked past the shutdown that raised it"); + + // Releasing it the normal way proves the rendezvous is otherwise intact. + ExceptionInjectionHelper.EnableException(Pause); + + ClassicAssert.IsTrue( + parked.Wait(TimeSpan.FromSeconds(30)), + "re-enabling the injection point did not release the parked caller"); + } + finally + { + ExceptionInjectionHelper.DisableException(Pause); + } + } + } +} \ No newline at end of file diff --git a/test/standalone/Garnet.test.vectorset/RespVectorSetTests.cs b/test/standalone/Garnet.test.vectorset/RespVectorSetTests.cs index a715636da7d..03aeb566381 100644 --- a/test/standalone/Garnet.test.vectorset/RespVectorSetTests.cs +++ b/test/standalone/Garnet.test.vectorset/RespVectorSetTests.cs @@ -71,7 +71,7 @@ public void DisabledWithFeatureFlag() foreach (var cmd in vectorSetCommands) { // Should all fault before any validation - var exc = ClassicAssert.Throws(() => db.Execute(cmd.ToString())); + var exc = TestUtils.ThrowsRedisException(() => db.Execute(cmd.ToString())); ClassicAssert.AreEqual("ERR Vector Set (preview) commands are not enabled", exc.Message); } } @@ -95,22 +95,22 @@ public void WrongTypeForVectorSetOpsOnNonVectorSetKeys() switch (cmd) { case RespCommand.VADD: - exc = ClassicAssert.Throws(() => db.Execute("VADD", ["foo", "REDUCE", "50", "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); + exc = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["foo", "REDUCE", "50", "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); break; case RespCommand.VCARD: // TODO: Implement when VCARD works continue; case RespCommand.VDIM: - exc = ClassicAssert.Throws(() => db.Execute("VDIM", ["foo"])); + exc = TestUtils.ThrowsRedisException(() => db.Execute("VDIM", ["foo"])); break; case RespCommand.VEMB: - exc = ClassicAssert.Throws(() => db.Execute("VEMB", ["foo", new byte[] { 0, 0, 0, 0 }])); + exc = TestUtils.ThrowsRedisException(() => db.Execute("VEMB", ["foo", new byte[] { 0, 0, 0, 0 }])); break; case RespCommand.VGETATTR: - exc = ClassicAssert.Throws(() => db.Execute("VGETATTR", ["foo", new byte[] { 0, 0, 0, 0 }])); + exc = TestUtils.ThrowsRedisException(() => db.Execute("VGETATTR", ["foo", new byte[] { 0, 0, 0, 0 }])); break; case RespCommand.VINFO: - exc = ClassicAssert.Throws(() => db.Execute("VINFO", ["foo"])); + exc = TestUtils.ThrowsRedisException(() => db.Execute("VINFO", ["foo"])); break; case RespCommand.VISMEMBER: // TODO: Implement when VISMEMBER works @@ -122,13 +122,13 @@ public void WrongTypeForVectorSetOpsOnNonVectorSetKeys() // TODO: Implement when VRANDMEMBER works continue; case RespCommand.VREM: - exc = ClassicAssert.Throws(() => db.Execute("VREM", ["foo", new byte[] { 0, 0, 0, 0 }])); + exc = TestUtils.ThrowsRedisException(() => db.Execute("VREM", ["foo", new byte[] { 0, 0, 0, 0 }])); break; case RespCommand.VSETATTR: // TODO: Implement when VSETATTR works continue; case RespCommand.VSIM: - exc = ClassicAssert.Throws(() => db.Execute("VSIM", ["foo", "VALUES", "75", "110.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "COUNT", "5", "EPSILON", "1.0", "EF", "40"])); + exc = TestUtils.ThrowsRedisException(() => db.Execute("VSIM", ["foo", "VALUES", "75", "110.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "COUNT", "5", "EPSILON", "1.0", "EF", "40"])); break; default: throw new InvalidOperationException($"Unexpected Vector Set command: {cmd}"); @@ -182,7 +182,7 @@ public void VADD() var res5 = db.Execute("VADD", ["fizz", "REDUCE", "50", "VALUES", "75", "150.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"]); ClassicAssert.AreEqual(1, (int)res5); - var exc1 = ClassicAssert.Throws(() => db.Execute("VADD", ["fizz", "VALUES", "4", "5.0", "6.0", "7.0", "8.0", new byte[] { 0, 0, 0, 1 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); + var exc1 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["fizz", "VALUES", "4", "5.0", "6.0", "7.0", "8.0", new byte[] { 0, 0, 0, 1 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); ClassicAssert.AreEqual("ERR Vector dimension mismatch - got 4 but set has 75", exc1.Message); // Add without specifying EF after first vector @@ -190,11 +190,11 @@ public void VADD() ClassicAssert.AreEqual(1, (int)res6); // Add without specifying M after first vector - var exc2 = ClassicAssert.Throws(() => db.Execute("VADD", ["fizz", "REDUCE", "50", "VALUES", "75", "180.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 4 }, "CAS", "NOQUANT", "EF", "16"])); + var exc2 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["fizz", "REDUCE", "50", "VALUES", "75", "180.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 4 }, "CAS", "NOQUANT", "EF", "16"])); ClassicAssert.AreEqual("ERR asked M value mismatch with existing vector set", exc2.Message); // Mismatch vector size for projection - var exc3 = ClassicAssert.Throws(() => db.Execute("VADD", ["fizz", "REDUCE", "50", "VALUES", "5", "1.0", "2.0", "3.0", "4.0", "5.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); + var exc3 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["fizz", "REDUCE", "50", "VALUES", "5", "1.0", "2.0", "3.0", "4.0", "5.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); ClassicAssert.AreEqual("ERR REDUCE dimension must be <= vector dimensions", exc3.Message); } @@ -258,7 +258,7 @@ public void VADDXPREQB8() } // REDUCE not allowed with XPREQ8 - var exc1 = ClassicAssert.Throws(() => db.Execute("VADD", ["fizz", "REDUCE", "2", "XB8", smallVectorData, new byte[] { 0, 0, 0, 0 }, "XPREQ8"])); + var exc1 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["fizz", "REDUCE", "2", "XB8", smallVectorData, new byte[] { 0, 0, 0, 0 }, "XPREQ8"])); ClassicAssert.AreEqual("ERR asked quantization mismatch with existing vector set", exc1.Message); // Create a vector set with XB8 + XPREQ8 @@ -287,61 +287,61 @@ public void VADDErrors() var vectorSetKey = $"{nameof(VADDErrors)}_{Guid.NewGuid()}"; // Bad arity - var exc1 = ClassicAssert.Throws(() => db.Execute("VADD")); + var exc1 = TestUtils.ThrowsRedisException(() => db.Execute("VADD")); ClassicAssert.AreEqual("ERR wrong number of arguments for 'VADD' command", exc1.Message); - var exc2 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey])); + var exc2 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey])); ClassicAssert.AreEqual("ERR wrong number of arguments for 'VADD' command", exc2.Message); - var exc3 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "FP32"])); + var exc3 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "FP32"])); ClassicAssert.AreEqual("ERR wrong number of arguments for 'VADD' command", exc3.Message); - var exc4 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES"])); + var exc4 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES"])); ClassicAssert.AreEqual("ERR wrong number of arguments for 'VADD' command", exc4.Message); - var exc5 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1"])); + var exc5 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1"])); ClassicAssert.AreEqual("ERR wrong number of arguments for 'VADD' command", exc5.Message); - var exc6 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "1.0"])); + var exc6 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "1.0"])); ClassicAssert.AreEqual("ERR wrong number of arguments for 'VADD' command", exc6.Message); // Reduce after vector - var exc7 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "2", "1.0", "2.0", "bar", "REDUCE", "1"])); + var exc7 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "2", "1.0", "2.0", "bar", "REDUCE", "1"])); ClassicAssert.AreEqual("ERR invalid option after element", exc7.Message); // Duplicate flags // TODO: Redis doesn't error on these which seems... wrong, confirm with them - //var exc8 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "CAS", "CAS"])); - //var exc9 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "NOQUANT", "Q8"])); - //var exc10 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "EF", "1", "EF", "1"])); - //var exc11 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "SETATTR", "abc", "SETATTR", "abc"])); - //var exc12 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M", "5", "M", "5"])); + //var exc8 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "CAS", "CAS"])); + //var exc9 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "NOQUANT", "Q8"])); + //var exc10 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "EF", "1", "EF", "1"])); + //var exc11 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "SETATTR", "abc", "SETATTR", "abc"])); + //var exc12 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M", "5", "M", "5"])); // M out of range (Redis imposes M >= 4 and m <= 4096 - var exc13 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M", "1"])); + var exc13 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M", "1"])); ClassicAssert.AreEqual("ERR M must be an integer between 4 and 4096", exc13.Message); - var exc14 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M", "10000"])); + var exc14 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M", "10000"])); ClassicAssert.AreEqual("ERR M must be an integer between 4 and 4096", exc14.Message); // Missing/bad option value - var exc20 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "EF"])); + var exc20 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "EF"])); ClassicAssert.AreEqual("ERR invalid option after element", exc20.Message); - var exc21 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "EF", "0"])); + var exc21 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "EF", "0"])); ClassicAssert.AreEqual("ERR EF must be an integer between 1 and 1000000", exc21.Message); - var exc22 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "SETATTR"])); + var exc22 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "SETATTR"])); ClassicAssert.AreEqual("ERR invalid option after element", exc22.Message); - var exc23 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M"])); + var exc23 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "M"])); ClassicAssert.AreEqual("ERR invalid option after element", exc23.Message); - var exc24 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "2", "2.0", "bar"])); + var exc24 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "2", "2.0", "bar"])); ClassicAssert.AreEqual("ERR invalid vector specification", exc24.Message); - var exc25 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "0", "bar"])); + var exc25 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "0", "bar"])); ClassicAssert.AreEqual("ERR invalid vector specification", exc25.Message); - var exc26 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "fizz", "bar"])); + var exc26 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "fizz", "bar"])); ClassicAssert.AreEqual("ERR invalid vector specification", exc26.Message); // Unknown option - var exc27 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "FOO"])); + var exc27 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "1", "2.0", "bar", "FOO"])); ClassicAssert.AreEqual("ERR invalid option after element", exc27.Message); // Malformed FP32 var binary = new float[] { 1, 2, 3 }; var blob = MemoryMarshal.Cast(binary)[..^1].ToArray(); - var exc15 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "FP32", blob, "bar"])); + var exc15 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "FP32", blob, "bar"])); ClassicAssert.AreEqual("ERR invalid vector specification", exc15.Message); // Mismatch after creating a vector set @@ -349,41 +349,41 @@ public void VADDErrors() _ = db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 1, 0 }, "NOQUANT", "EF", "6", "M", "10", "XDISTANCE_METRIC", "L2"]); - var exc16 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "2", "1.0", "2.0", "fizz", "NOQUANT", "EF", "6", "M", "10"])); + var exc16 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "2", "1.0", "2.0", "fizz", "NOQUANT", "EF", "6", "M", "10"])); ClassicAssert.AreEqual("ERR Vector dimension mismatch - got 2 but set has 75", exc16.Message); - var exc17 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "fizz", "XPREQ8", "EF", "6", "M", "10"])); + var exc17 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "fizz", "XPREQ8", "EF", "6", "M", "10"])); ClassicAssert.AreEqual("ERR asked quantization mismatch with existing vector set", exc17.Message); - var exc18 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "fizz", "NOQUANT", "EF", "12", "M", "20"])); + var exc18 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "fizz", "NOQUANT", "EF", "12", "M", "20"])); ClassicAssert.AreEqual("ERR asked M value mismatch with existing vector set", exc18.Message); // TODO: Redis doesn't appear to validate attributes... so that's weird // Empty Vector Set keys are forbidden (TODO: Remove this constraint) - var exc19 = ClassicAssert.Throws(() => db.Execute("VADD", ["", "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "XPREQ8"])); + var exc19 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["", "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "XPREQ8"])); ClassicAssert.AreEqual("ERR Vector Set key cannot be empty", exc19.Message); // Malformed XDISTANCE_METRIC - var exc31 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "bar", "NOQUANT", "XDISTANCE_METRIC"])); + var exc31 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "bar", "NOQUANT", "XDISTANCE_METRIC"])); ClassicAssert.AreEqual("ERR invalid option after element", exc31.Message); - var exc32 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "bar", "NOQUANT", "XDISTANCE_METRIC", "FOO"])); + var exc32 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "bar", "NOQUANT", "XDISTANCE_METRIC", "FOO"])); ClassicAssert.AreEqual("ERR invalid XDISTANCE_METRIC", exc32.Message); // Invalid vector type keyword (not FP32, VALUES, or XB8) - var exc40 = ClassicAssert.Throws(() => db.Execute("VADD", ["mykey", "GARBAGE", "data", "elem1"])); + var exc40 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["mykey", "GARBAGE", "data", "elem1"])); ClassicAssert.AreEqual("ERR invalid vector specification", exc40.Message); // VALUES count exceeding MaxVectorDimensions (65536) must be rejected - var exc41 = ClassicAssert.Throws(() => db.Execute("VADD", ["foo", "VALUES", "100000", "1.0", "elem"])); + var exc41 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["foo", "VALUES", "100000", "1.0", "elem"])); ClassicAssert.IsTrue(exc41.Message.Contains("maximum"), $"Expected dimension limit error, got: {exc41.Message}"); // EF exceeding MaxExplorationFactor (1,000,000) must be rejected - var exc42 = ClassicAssert.Throws(() => db.Execute("VADD", ["foo", "VALUES", "3", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "2000000000", "M", "32"])); + var exc42 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["foo", "VALUES", "3", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "2000000000", "M", "32"])); ClassicAssert.IsTrue(exc42.Message.Contains("EF must be an integer between"), $"Expected EF validation error, got: {exc42.Message}"); // REDUCE dim exceeding vector dimensions must be rejected - var exc43 = ClassicAssert.Throws(() => db.Execute("VADD", ["foo", "REDUCE", "100000", "VALUES", "3", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); + var exc43 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", ["foo", "REDUCE", "100000", "VALUES", "3", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"])); ClassicAssert.IsTrue(exc43.Message.Contains("REDUCE dimension must be <= vector dimensions"), $"Expected REDUCE dimension limit error, got: {exc43.Message}"); - var exc33 = ClassicAssert.Throws(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "bar", "NOQUANT", "XDISTANCE_METRIC", "XCOSINE_NORMALIZED"])); + var exc33 = TestUtils.ThrowsRedisException(() => db.Execute("VADD", [vectorSetKey, "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "bar", "NOQUANT", "XDISTANCE_METRIC", "XCOSINE_NORMALIZED"])); ClassicAssert.AreEqual("ERR Distance metric mismatch - got XCosine_Normalized but set has L2", exc33.Message); } @@ -437,7 +437,7 @@ public void VectorSetOpacity() var res1 = db.Execute("VADD", ["foo", "REDUCE", "50", "VALUES", "75", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", "4.0", "1.0", "2.0", "3.0", new byte[] { 0, 0, 0, 0 }, "CAS", "NOQUANT", "EF", "16", "M", "32"]); ClassicAssert.AreEqual(1, (int)res1); - var res2 = ClassicAssert.Throws(() => db.StringGet("foo")); + var res2 = TestUtils.ThrowsRedisException(() => db.StringGet("foo")); ClassicAssert.True(res2.Message.Contains("WRONGTYPE")); } @@ -793,7 +793,7 @@ public void VDIM() var res4 = db.Execute("VDIM", "bar"); ClassicAssert.AreEqual(75, (int)res4); - var exc1 = ClassicAssert.Throws(() => db.Execute("VDIM", "fizz")); + var exc1 = TestUtils.ThrowsRedisException(() => db.Execute("VDIM", "fizz")); ClassicAssert.IsTrue(exc1.Message.Contains("Key not found")); // TODO: Add WRONGTYPE behavior check once implemented @@ -1057,7 +1057,7 @@ public void VSIMBadFilters() foreach (var (filter, why) in badFilters) { - var exc = ClassicAssert.Throws( + var exc = TestUtils.ThrowsRedisException( () => db.Execute("VSIM", [VectorSet, "VALUES", "3", "0.0", "0.0", "0.0", "FILTER", filter, "COUNT", "10"]), $"Expected compile failure for filter '{filter}' ({why})"); ClassicAssert.AreEqual(CompileErr, exc.Message, $"Wrong error message for filter '{filter}' ({why})"); @@ -1246,19 +1246,19 @@ public void VSIMErrors() ClassicAssert.AreEqual(1, (int)res1); // FILTER-EF exceeding MaxFilteringScaleFactor must be rejected - var exc1 = ClassicAssert.Throws(() => db.Execute("VSIM", ["foo", "VALUES", "3", "0.0", "0.0", "0.0", "FILTER", ".year > 1950", "FILTER-EF", "999999999", "COUNT", "3", "WITHATTRIBS"])); + var exc1 = TestUtils.ThrowsRedisException(() => db.Execute("VSIM", ["foo", "VALUES", "3", "0.0", "0.0", "0.0", "FILTER", ".year > 1950", "FILTER-EF", "999999999", "COUNT", "3", "WITHATTRIBS"])); ClassicAssert.AreEqual("ERR FILTER-EF must be an integer between 4 and 256", exc1.Message); // COUNT exceeding MaxRetrieveCount must be rejected - var exc2 = ClassicAssert.Throws(() => db.Execute("VSIM", ["foo", "VALUES", "3", "0.0", "0.0", "0.0", "COUNT", "999999999"])); + var exc2 = TestUtils.ThrowsRedisException(() => db.Execute("VSIM", ["foo", "VALUES", "3", "0.0", "0.0", "0.0", "COUNT", "999999999"])); ClassicAssert.AreEqual("ERR COUNT must be an integer between 0 and 100000000", exc2.Message); // VALUES count exceeding MaxVectorDimensions (65536) must be rejected - var exc3 = ClassicAssert.Throws(() => db.Execute("VSIM", ["foo", "VALUES", "100000", "1.0"])); + var exc3 = TestUtils.ThrowsRedisException(() => db.Execute("VSIM", ["foo", "VALUES", "100000", "1.0"])); ClassicAssert.AreEqual("ERR vector exceeds maximum of 65536 dimensions", exc3.Message); // EF exceeding MaxExplorationFactor (1,000,000) must be rejected - var exc4 = ClassicAssert.Throws(() => db.Execute("VSIM", ["foo", "VALUES", "3", "0.0", "0.0", "0.0", "EF", "2000000000"])); + var exc4 = TestUtils.ThrowsRedisException(() => db.Execute("VSIM", ["foo", "VALUES", "3", "0.0", "0.0", "0.0", "EF", "2000000000"])); ClassicAssert.AreEqual("ERR EF must be an integer between 1 and 1000000", exc4.Message); } @@ -1699,7 +1699,7 @@ public void RepeatedVectorSetDeletes() var addRes2 = (int)db.Execute("VADD", ["foo", "XB8", bytes2, new byte[] { 0, 0, 0, 1 }, "XPREQ8"]); ClassicAssert.AreEqual(1, addRes2); - var readExc = ClassicAssert.Throws(() => db.Execute("GET", ["foo"])); + var readExc = TestUtils.ThrowsRedisException(() => db.Execute("GET", ["foo"])); ClassicAssert.IsTrue(readExc.Message.Equals("WRONGTYPE Operation against a key holding the wrong kind of value."), $"In iteration: {i}"); } @@ -2798,7 +2798,7 @@ public void VREM() // Remove on non-vector set fails // TODO: test against Redis, how do they respond (I expect WRONGTYPE, but needs verification) //_ = db.StringSet("fizz", "buzz"); - //var exc1 = ClassicAssert.Throws(() => db.Execute("VREM", "fizz", new byte[] { 0, 0, 0, 0 })); + //var exc1 = TestUtils.ThrowsRedisException(() => db.Execute("VREM", "fizz", new byte[] { 0, 0, 0, 0 })); //ClassicAssert.AreEqual("", exc1.Message); // Remove exists diff --git a/test/standalone/Garnet.test.vectorset/VectorSetOverwriteTests.cs b/test/standalone/Garnet.test.vectorset/VectorSetOverwriteTests.cs index 0a79d160cd4..8fa557ecbf1 100644 --- a/test/standalone/Garnet.test.vectorset/VectorSetOverwriteTests.cs +++ b/test/standalone/Garnet.test.vectorset/VectorSetOverwriteTests.cs @@ -136,12 +136,11 @@ static async Task RunCommandAsync(IDatabaseAsync executeDB, IDatabaseAsync readD } [Test] - public async Task SETAsync() + public Task SETAsync() { - await TestVectorSetOverwrittenCommandAsync(RunCommandPlainAsync).ConfigureAwait(false); - await TestVectorSetOverwrittenCommandAsync(RunCommandEXAsync).ConfigureAwait(false); + return TestVectorSetOverwrittenCommandAsync(RunCommandAsync); - static async Task RunCommandPlainAsync(IDatabaseAsync executeDB, IDatabaseAsync readDB, RedisKey againstKey) + static async Task RunCommandAsync(IDatabaseAsync executeDB, IDatabaseAsync readDB, RedisKey againstKey) { var res = await executeDB.StringSetAsync(againstKey, "foo").ConfigureAwait(false); ClassicAssert.IsTrue(res); @@ -149,8 +148,14 @@ static async Task RunCommandPlainAsync(IDatabaseAsync executeDB, IDatabaseAsync var finalValue = await readDB.StringGetAsync(againstKey).ConfigureAwait(false); ClassicAssert.AreEqual("foo", finalValue); } + } + + [Test] + public Task SETKeepTtlAsync() + { + return TestVectorSetOverwrittenCommandAsync(RunCommandAsync); - static async Task RunCommandEXAsync(IDatabaseAsync executeDB, IDatabaseAsync readDB, RedisKey againstKey) + static async Task RunCommandAsync(IDatabaseAsync executeDB, IDatabaseAsync readDB, RedisKey againstKey) { var res = (string)await executeDB.ExecuteAsync("SET", againstKey, "foo", "KEEPTTL").ConfigureAwait(false); ClassicAssert.AreEqual("OK", res); diff --git a/test/standalone/Garnet.test.vectorset/VectorSetRecoveredContextReservationTests.cs b/test/standalone/Garnet.test.vectorset/VectorSetRecoveredContextReservationTests.cs new file mode 100644 index 00000000000..b12056bdfbb --- /dev/null +++ b/test/standalone/Garnet.test.vectorset/VectorSetRecoveredContextReservationTests.cs @@ -0,0 +1,137 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using NUnit.Framework; +using NUnit.Framework.Legacy; +using StackExchange.Redis; + +namespace Garnet.test +{ + /// + /// A Vector Set's index record is written before the context metadata that reserves its context, and + /// the metadata is only flushed once index creation succeeds. A checkpoint taken between the two + /// captures a live index record whose context is not marked in use, so recovery has to restore that + /// reservation from the index records themselves - otherwise the free-list hands the same context to + /// the next Vector Set created and the two share a namespace. + /// + [TestFixture] + public class VectorSetRecoveredContextReservationTests : TestBase + { + private global::Garnet.GarnetServer server; + + [SetUp] + public void Setup() + { + TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); + } + + [TearDown] + public void TearDown() + { + server?.Dispose(); + server = null; + + TestUtils.DeleteDirectory(TestUtils.MethodTestDir); + TestUtils.OnTearDown(); + } + + private void StartServer(bool tryRecover) + { + server = TestUtils.CreateGarnetServer( + TestUtils.MethodTestDir, + memorySize: "8m", + pageSize: "16k", + enableAOF: true, + aofMemorySize: "2g", + tryRecover: tryRecover, + enableVectorSetPreview: true); + server.Start(); + } + + [Test] + public async Task RecoveredVectorSetDoesNotShareContextWithNewVectorSetAsync() + { + const string Recovered = "recovered-vs"; + const string Fresh = "fresh-vs"; + const int Elements = 500; + const int Dim = 32; + + StartServer(tryRecover: false); + + using (var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(allowAdmin: true))) + { + var db = redis.GetDatabase(0); + + for (var i = 0; i < Elements; i++) + { + ClassicAssert.AreEqual(1, (int)db.Execute("VADD", BuildVaddArgs(Recovered, Dim, i, $"recovered_{i}"))); + } + +#pragma warning disable CS0618 // ForegroundSave is obsolete but is what the recovery tests use + redis.GetServers().Single().Save(SaveType.ForegroundSave); +#pragma warning restore CS0618 + + var committed = await server.Store.WaitForCommitAsync(); + ClassicAssert.IsTrue(committed, "checkpoint commit did not complete"); + } + + server.Dispose(deleteDir: false); + StartServer(tryRecover: true); + + using (var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(allowAdmin: true))) + { + var db = redis.GetDatabase(0); + + ClassicAssert.AreEqual(Elements, (int)db.Execute("VCARD", [Recovered]), + "the recovered Vector Set lost elements during recovery"); + + // Creating a Vector Set now must not be handed the recovered set's context. If it is, the two + // share a namespace and writes to one are visible in - and destroy - the other. + for (var i = 0; i < Elements; i++) + { + ClassicAssert.AreEqual(1, (int)db.Execute("VADD", BuildVaddArgs(Fresh, Dim, i + Elements, $"fresh_{i}"))); + } + + ClassicAssert.AreEqual(Elements, (int)db.Execute("VCARD", [Fresh]), + "the newly created Vector Set did not receive all of its elements"); + + ClassicAssert.AreEqual(Elements, (int)db.Execute("VCARD", [Recovered]), + "creating a new Vector Set after recovery changed the recovered Vector Set's cardinality, " + + "so the two were handed the same context"); + + var simArgs = new List { Recovered, "VALUES", $"{Dim}" }; + for (var d = 0; d < Dim; d++) + { + simArgs.Add($"{((0 % 7) + 1) * (d + 1) % 13}"); + } + simArgs.Add("COUNT"); + simArgs.Add($"{Elements}"); + + var recoveredMembers = ((RedisResult[])db.Execute("VSIM", simArgs.ToArray())) + .Select(static r => (string)r) + .ToArray(); + + CollectionAssert.IsNotEmpty(recoveredMembers); + CollectionAssert.IsSubsetOf( + recoveredMembers, + Enumerable.Range(0, Elements).Select(static i => $"recovered_{i}").ToArray(), + "the recovered Vector Set returned elements belonging to the Vector Set created after recovery"); + } + } + + private static object[] BuildVaddArgs(string key, int dim, int seed, string element) + { + var args = new List { key, "VALUES", $"{dim}" }; + for (var d = 0; d < dim; d++) + { + args.Add($"{((seed % 7) + 1) * (d + 1) % 13}"); + } + args.Add(element); + + return args.ToArray(); + } + } +} \ No newline at end of file diff --git a/test/standalone/Garnet.test/TestUtils.cs b/test/standalone/Garnet.test/TestUtils.cs index 203c3ea7a4a..54427dc6546 100644 --- a/test/standalone/Garnet.test/TestUtils.cs +++ b/test/standalone/Garnet.test/TestUtils.cs @@ -12,6 +12,7 @@ using System.Net.Security; using System.Net.Sockets; using System.Runtime.CompilerServices; +using System.Runtime.ExceptionServices; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; @@ -279,6 +280,63 @@ public static void WaitUntilNextSecond(IDatabase db, long baseSeconds) } } + /// + /// Asserts that throws , running it on a dedicated thread. + /// + /// + /// StackExchange.Redis' synchronous request path (ConnectionMultiplexer.ExecuteSyncImpl) parks the caller on a + /// [ThreadStatic] result box. A server error reply faults that box from the connection's reader thread, and when + /// the fault lands before the caller reaches Monitor.Wait the caller skips the wait and recycles the box while the + /// reader thread still has a Monitor.PulseAll outstanding on it. The next synchronous call on the same thread + /// takes the recycled box, consumes that stale pulse, and returns with neither a result nor an exception, shifting every + /// subsequent reply on that thread by one. Running calls that are expected to fail on their own thread keeps the + /// recycled box off the shared test thread. + /// + /// Expected exception type. + /// Delegate performing the call that is expected to fail. + /// The exception thrown by . + public static TActual ThrowsRedisException(TestDelegate code) where TActual : Exception + => ClassicAssert.Throws(CaptureOnDedicatedThread(code)); + + /// + /// Variant of that reports on failure. + /// + /// Expected exception type. + /// Delegate performing the call that is expected to fail. + /// Message reported when no matching exception is thrown. + /// Format arguments for . + /// The exception thrown by . + public static TActual ThrowsRedisException(TestDelegate code, string message, params object[] args) where TActual : Exception + => ClassicAssert.Throws(CaptureOnDedicatedThread(code), message, args); + + /// + /// Runs to completion on a dedicated thread, returning a delegate that replays whatever it threw. + /// + /// Delegate to run. + /// Delegate that rethrows the captured exception, preserving its original stack trace. + private static TestDelegate CaptureOnDedicatedThread(TestDelegate code) + { + ExceptionDispatchInfo captured = null; + + var thread = new Thread(() => + { + try + { + code(); + } + catch (Exception ex) + { + captured = ExceptionDispatchInfo.Capture(ex); + } + }) + { IsBackground = true }; + + thread.Start(); + thread.Join(); + + return () => captured?.Throw(); + } + /// /// Create GarnetServer /// From 961a5a82d85c023b3cf145f5ce4b457f0f4867a0 Mon Sep 17 00:00:00 2001 From: Badrish Chandramouli Date: Tue, 8 Sep 2026 19:29:12 -0700 Subject: [PATCH 2/3] Fix replication teardown races that hang server dispose and poison CI fixtures AofSyncDriver.Dispose() disposed each AOF sync task's GarnetClientSession while its worker task was still running. GarnetClientSession documents that it "expects mono-threaded client access, i.e., no concurrent invocations of API by client", so this is a contract violation, and it is the dominant source of flakiness in the cluster suites. When the disposing thread wins the race, the worker has just published its send frame (SendResponse nulls responseObject before sending) and has not yet reacquired one. The disposer's ReturnResponseObject() therefore returns nothing, and the worker then calls GetResponseObject(): LightConcurrentStack.TryPop samples disposed==false on an empty stack, releases the latch, and the caller allocates a fresh GarnetSaeaBuffer renting from the replication pool. The disposer disposes the stack in that gap, the worker dies at its next Throttle() check without returning the frame, and its second Dispose() returns immediately on the disposed counter. The buffer is stranded. LimitedFixedBufferPool.Dispose() then spin-waits forever for totalReferences to reach zero, so ReplicationManager.Dispose() never returns, the node never releases its port, and every later test in the fixture fails on "Failed to connect within 30 seconds". CI shows exactly this: one unreturned 64 MB Replication SaeaSendBuffer (2 << AofPageSizeBits, i.e. an AOF sync buffer) logged immediately before "Timed out waiting for DisposeCluster", followed by 26 consecutive failures in the same job. Fix the ordering rather than making the sender thread-safe, which would paper over the contract violation and add synchronization to the send path: - AofSyncTask.RunAofSyncTaskAsync disposes its client before leaving the active worker monitor, so a drained monitor implies every client was torn down by the one thread that used it and its buffer is back in the pool. - AofSyncDriver.Dispose() breaks the connections instead of disposing the sessions, waits for the workers to quiesce, and only then disposes the tasks. The task dispose is kept so a client connected without a running worker still returns its buffer, and it also makes the iterator disposal race-free. - GarnetClientSession.CloseConnection() closes only the socket, which is what unblocks a worker parked on a send without touching the buffer it owns. This is confined to teardown; no hot path changes. Verified by modelling the acquire-then-abandon sequence directly against GarnetTcpNetworkSender: when a foreign thread disposes the sender while the worker holds a rented frame, LimitedFixedBufferPool.Dispose() never completes; when the owning thread returns its own frame, the pool drains immediately. Also fix RespTests.ClientKillTestAsync, which asserted IsConnected was false after a single fire-and-forget Ping. Socket.Connected reflects the last completed I/O, and TCP guarantees the first send after a remote FIN succeeds into the kernel buffer, so the assertion could only pass by winning a race with the client's receive loop. It now pings until the disconnect surfaces, bounded at 10s, so a CLIENT KILL that genuinely failed still fails the test. Validated on 2 cores (taskset -c 0-1) to match the CI runners: Garnet.test.cluster.multilog 105 passed / 12 skipped / 0 failed, with no DisposeCluster timeout, no buffer pool dispose diagnostic and no leaked epochs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 30f31d5b-9e53-4111-91a3-e16c94510a77 --- .../ClientSession/GarnetClientSession.cs | 9 ++++++++ .../PrimaryOps/AofOperations/AofSyncDriver.cs | 14 ++++++++++--- .../PrimaryOps/AofOperations/AofSyncTask.cs | 18 +++++++++++++++- test/standalone/Garnet.test/RespTests.cs | 21 +++++++++++++++---- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/libs/client/ClientSession/GarnetClientSession.cs b/libs/client/ClientSession/GarnetClientSession.cs index 7a1e988da0a..1cb7b9ee442 100644 --- a/libs/client/ClientSession/GarnetClientSession.cs +++ b/libs/client/ClientSession/GarnetClientSession.cs @@ -407,6 +407,15 @@ public Task ReconnectAsync(int timeoutMs = 0, CancellationToken token = default) return ConnectAsync(timeoutMs, token); } + /// + /// Closes the underlying connection so a thread blocked on network I/O through this session + /// fails fast, without disposing the session itself. The session expects mono-threaded access, + /// so a caller that is not the thread using the session must not dispose it: doing so races + /// that thread's use of the send buffer it has rented. Closing the connection is safe because + /// it only invalidates the socket, leaving the buffer owned by the thread that rented it. + /// + public void CloseConnection() => socket?.Dispose(); + /// /// Dispose instance /// diff --git a/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncDriver.cs b/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncDriver.cs index abe18580540..8fb702ee452 100644 --- a/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncDriver.cs +++ b/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncDriver.cs @@ -121,13 +121,21 @@ public void Dispose() // Cancel cts cts?.Cancel(); - // Dispose sync tasks + // Break the connections so a task blocked on a send fails fast. The client sessions are + // deliberately not disposed here: they expect mono-threaded access, and a task may still + // be writing through one. Disposing it from this thread races that write and can strand + // the send buffer the task rented, which leaves the replication buffer pool's dispose + // spinning forever and the whole server stuck in shutdown holding its port. foreach (var aofSyncTask in aofSyncTasks) - aofSyncTask?.Dispose(); + aofSyncTask?.CloseConnection(); - // Wait for tasks to exit + // Wait for tasks to exit; each disposes its own client before leaving the monitor activeWorkerMonitor.Dispose(); + // Dispose sync tasks, now that no task is running + foreach (var aofSyncTask in aofSyncTasks) + aofSyncTask?.Dispose(); + // Finally, dispose the cts cts?.Dispose(); } diff --git a/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncTask.cs b/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncTask.cs index 9e8915007db..be464afea0f 100644 --- a/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncTask.cs +++ b/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncTask.cs @@ -143,6 +143,19 @@ public AofSyncTask( this.logger = logger; } + /// + /// Closes the network connection so a task blocked on a send fails fast, without + /// disposing the client session the task may still be writing through. + /// + public void CloseConnection() + { + try + { + garnetClient?.CloseConnection(); + } + catch { } + } + public void Dispose() { try @@ -344,9 +357,12 @@ await iter.BulkConsumeAllAsync( } finally { + // The client is disposed before leaving the monitor so that a drained monitor + // means every client has already been torn down by the one thread that was + // using it, and the send buffer it rented is back in the replication pool. + garnetClient?.Dispose(); if (enteredMonitor) _ = aofSyncDriver.activeWorkerMonitor.Exit(); - garnetClient?.Dispose(); } [Conditional("DEBUG")] diff --git a/test/standalone/Garnet.test/RespTests.cs b/test/standalone/Garnet.test/RespTests.cs index eb54dccdd25..803b488c0e0 100644 --- a/test/standalone/Garnet.test/RespTests.cs +++ b/test/standalone/Garnet.test/RespTests.cs @@ -4580,12 +4580,25 @@ static async Task ConnectAsync() // Check that we really killed the connection backing a GarnetClient static void AssertNotConnected(GarnetClient client) { - // Force the issue by attempting a command - try + // IsConnected reads Socket.Connected, which reports the state as of the last completed + // I/O. Once the server closes the connection the first send still succeeds locally and + // only a following one observes the reset, so a single ping cannot establish that the + // connection is gone. Keep issuing pings until the disconnect surfaces; a connection + // that was not killed keeps answering them and still fails the assert below. + var elapsed = Stopwatch.StartNew(); + while (client.IsConnected && elapsed.Elapsed < TimeSpan.FromSeconds(10)) { - client.Ping(static (_, __) => { }); + try + { + client.Ping(static (_, __) => { }); + } + catch + { + break; + } + + Thread.Sleep(10); } - catch { } ClassicAssert.IsFalse(client.IsConnected); } From 2c47d137f5363929023f48bc0361925d87a122fb Mon Sep 17 00:00:00 2001 From: Badrish Chandramouli Date: Tue, 8 Sep 2026 20:13:16 -0700 Subject: [PATCH 3/3] Close the connect-time window where a connection break is lost AofSyncDriver.Dispose() relies on CloseConnection() to fail a worker's pending network I/O so the worker exits and the driver's wait for it completes. That signal could be lost: CloseConnection() reads the socket field, and a worker inside ConnectAsync has not published it yet. The window is not narrow. The field is written in the continuation of `socket = await ConnectSendSocketAsync(...)`, which on a loaded 2-core runner can be delayed arbitrarily. ConnectSendSocketAsync honours the token, but nothing after it does on the non-TLS path: NetworkHandler.StartAsync returns immediately when TLS is off, and the AUTH, CLIENT SETINFO and CLIENT SETNAME exchanges that follow await a reply with no cancellation token. A worker that lost the break therefore rents its send buffer and parks forever on a peer that is also shutting down, so the driver's wait never completes and the pool dispose spins. Make it a handshake: CloseConnection() records the request before closing, and the connecting thread rechecks it immediately after publishing the socket, so either the closer observes the socket or the connector observes the request. The recheck sits before the network handler is constructed and before the send buffer is rented, so the throw path allocates nothing. Both sides need a StoreLoad fence for the handshake to hold. Publishing the socket and reading the request are a store then a load of a different location, which the acquire load does not order, so without the barrier both sides can read stale values and the break is lost anyway. Verified on 2 cores (taskset -c 0-1): Garnet.test.cluster.multilog 105 passed / 12 skipped / 0 failed, Garnet.test.cluster.replication 107/107, with no DisposeCluster timeout and no buffer pool dispose diagnostic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 30f31d5b-9e53-4111-91a3-e16c94510a77 --- .../ClientSession/GarnetClientSession.cs | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/libs/client/ClientSession/GarnetClientSession.cs b/libs/client/ClientSession/GarnetClientSession.cs index 1cb7b9ee442..17e70b709e9 100644 --- a/libs/client/ClientSession/GarnetClientSession.cs +++ b/libs/client/ClientSession/GarnetClientSession.cs @@ -42,6 +42,8 @@ public sealed partial class GarnetClientSession : IServerHook, IMessageConsumer static readonly Exception disposeException = new GarnetClientDisposedException(); Socket socket; + + int closeRequested; int disposed; // Send @@ -147,6 +149,7 @@ public GarnetClientSession( public unsafe void Connect(CancellationToken token = default) { socket = ConnectSendSocket(); + ThrowIfCloseRequested(); networkHandler = new GarnetClientSessionTcpNetworkHandler( this, socket, @@ -199,6 +202,7 @@ public unsafe void Connect(CancellationToken token = default) public async Task ConnectAsync(int timeoutMs = 0, CancellationToken token = default) { socket = await ConnectSendSocketAsync(timeoutMs, token).ConfigureAwait(false); + ThrowIfCloseRequested(); networkHandler = new GarnetClientSessionTcpNetworkHandler( this, socket, @@ -413,8 +417,37 @@ public Task ReconnectAsync(int timeoutMs = 0, CancellationToken token = default) /// so a caller that is not the thread using the session must not dispose it: doing so races /// that thread's use of the send buffer it has rented. Closing the connection is safe because /// it only invalidates the socket, leaving the buffer owned by the thread that rented it. + /// + /// The session is not reusable afterwards: the request is sticky, so a later + /// or throws. + /// + public void CloseConnection() + { + _ = Interlocked.Exchange(ref closeRequested, 1); + Volatile.Read(ref socket)?.Dispose(); + } + + /// + /// A close that lands while the connection is still being established would otherwise be lost: + /// the socket is not yet published for to find, and the exchanges + /// that follow await a reply with no cancellation token, so the connecting thread would park + /// indefinitely on a peer that is shutting down. Checking here after the socket is published + /// means either the closer observes the socket or the connecting thread observes the request. /// - public void CloseConnection() => socket?.Dispose(); + void ThrowIfCloseRequested() + { + // Publishing the socket and reading the request are a store followed by a load of a + // different location, so without a StoreLoad fence both sides can miss: this thread's + // store is still buffered while it reads a stale request, and the closer reads a stale + // socket. The closer's Interlocked.Exchange fences its own side; this fences ours. + Interlocked.MemoryBarrier(); + + if (Volatile.Read(ref closeRequested) == 0) + return; + + socket?.Dispose(); + throw new ObjectDisposedException(nameof(GarnetClientSession), "Connection was closed while connecting"); + } /// /// Dispose instance