diff --git a/Directory.Packages.props b/Directory.Packages.props index c917e3623e0..72f013b4e0a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -25,7 +25,7 @@ - + diff --git a/libs/client/ClientSession/GarnetClientSession.cs b/libs/client/ClientSession/GarnetClientSession.cs index 7a1e988da0a..b7e1d96b8e6 100644 --- a/libs/client/ClientSession/GarnetClientSession.cs +++ b/libs/client/ClientSession/GarnetClientSession.cs @@ -42,6 +42,13 @@ public sealed partial class GarnetClientSession : IServerHook, IMessageConsumer static readonly Exception disposeException = new GarnetClientDisposedException(); Socket socket; + + /// + /// Set by and never cleared, so a close is sticky: it also lets + /// observe a close that lands while the connection is still + /// being established, which would otherwise leave that thread awaiting a reply that never comes. + /// + int closeRequested; int disposed; // Send @@ -147,6 +154,7 @@ public GarnetClientSession( public unsafe void Connect(CancellationToken token = default) { socket = ConnectSendSocket(); + ThrowIfCloseRequested(); networkHandler = new GarnetClientSessionTcpNetworkHandler( this, socket, @@ -199,6 +207,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, @@ -407,6 +416,44 @@ 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. + /// + /// 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. + /// + 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 /// diff --git a/libs/cluster/Server/Gossip/Gossip.cs b/libs/cluster/Server/Gossip/Gossip.cs index 130d2e99154..a08ff85dc17 100644 --- a/libs/cluster/Server/Gossip/Gossip.cs +++ b/libs/cluster/Server/Gossip/Gossip.cs @@ -343,18 +343,30 @@ async Task GossipMainAsync() while (true) { ctsGossip.Token.ThrowIfCancellationRequested(); - await InitConnectionsAsync().ConfigureAwait(false); - // Choose between full broadcast or sample gossip to few nodes - if (GossipSamplePercent == 100) - await BroadcastGossipSendAsync().ConfigureAwait(false); - else - await GossipSampleSendAsync().ConfigureAwait(false); + try + { + ExceptionInjectionHelper.TriggerException(ExceptionInjectionType.Cluster_Gossip_Round_Fail); + await InitConnectionsAsync().ConfigureAwait(false); + + // Choose between full broadcast or sample gossip to few nodes + if (GossipSamplePercent == 100) + await BroadcastGossipSendAsync().ConfigureAwait(false); + else + await GossipSampleSendAsync().ConfigureAwait(false); + } + catch (Exception ex) when (!ctsGossip.Token.IsCancellationRequested) + { + // Nothing restarts this task, so letting a single failed round leave the loop would stop + // this node gossiping for the rest of the process. Its view then stops being offered to + // the rest of the cluster and no further configuration change can ever converge. + logger?.LogWarning("Gossip round failed {msg}", ex.Message); + } await Task.Delay(gossipDelay, ctsGossip.Token).ConfigureAwait(false); } } - catch (TaskCanceledException) when (ctsGossip.Token.IsCancellationRequested) + catch (OperationCanceledException) when (ctsGossip.Token.IsCancellationRequested) { // Suppress the exception if the task was cancelled because of store wrapper disposal } 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..5f46fa1cdc6 100644 --- a/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncTask.cs +++ b/libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncTask.cs @@ -114,7 +114,10 @@ public AofSyncTask( this.remoteNodeId = remoteNodeId; this.cts = cts; appendOnlyFile = clusterProvider.storeWrapper.appendOnlyFile; - timePulseEnabled = clusterProvider.serverOptions.MultiLogEnabled; + // Only a log sharded across several physical sublogs needs the pulse. On a single + // physical sublog every replay task rendezvouses on the same batch and publishes the + // same log address, so no virtual sublog can lag behind another for a pulse to relieve. + timePulseEnabled = clusterProvider.serverOptions.AofPhysicalSublogCount > 1; if (timePulseEnabled) { physicalSublog = appendOnlyFile.Log.GetSubLog(physicalSublogIdx); @@ -143,6 +146,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 +360,24 @@ await iter.BulkConsumeAllAsync( } finally { - if (enteredMonitor) - _ = aofSyncDriver.activeWorkerMonitor.Exit(); - garnetClient?.Dispose(); + try + { + // 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(); + } + catch (Exception ex) + { + // Leaving the monitor is what releases the dispose that is waiting on this + // task, so it has to happen even if tearing the client down fails. + logger?.LogError(ex, "[{sublogIdx}]({method}) failed to dispose client", physicalSublogIdx, nameof(RunAofSyncTaskAsync)); + } + finally + { + if (enteredMonitor) + _ = aofSyncDriver.activeWorkerMonitor.Exit(); + } } [Conditional("DEBUG")] 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/common/Testing/ExceptionInjectionType.cs b/libs/common/Testing/ExceptionInjectionType.cs index dda1fa7e4c7..d3539b42097 100644 --- a/libs/common/Testing/ExceptionInjectionType.cs +++ b/libs/common/Testing/ExceptionInjectionType.cs @@ -125,5 +125,25 @@ public enum ExceptionInjectionType /// Vector Set: pause in the index-recreate window before RMW write refreshed index pointer back to the main store. /// VectorSet_Pause_Before_Recreate_Rmw, + + /// + /// Pause before the RMW that persists a ContextMetadata record. + /// + VectorSet_Pause_Before_Context_Metadata_Rmw, + + /// + /// Pause when a checkpoint reaches the end of its fuzzy region, before it captures the + /// last address the snapshot covers. + /// + Checkpoint_Pause_At_Flush_Begin, + /// + /// Cluster gossip: fail a single round of the main gossip loop, to exercise recovery from a transient + /// error raised while connections are initialized or the configuration is broadcast. + /// + Cluster_Gossip_Round_Fail, + /// + /// Replica full sync: fail recovery of the checkpoint received from the primary, before any of it is applied. + /// + Replication_Fail_Replica_Checkpoint_Recovery, } } \ No newline at end of file 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/Databases/SingleDatabaseManager.cs b/libs/server/Databases/SingleDatabaseManager.cs index 420fbe96578..e0a28f8190c 100644 --- a/libs/server/Databases/SingleDatabaseManager.cs +++ b/libs/server/Databases/SingleDatabaseManager.cs @@ -61,6 +61,8 @@ public override async ValueTask RecoverCheckpointAsync(bool replicaRecover = fal { if (replicaRecover) { + ExceptionInjectionHelper.TriggerException(ExceptionInjectionType.Replication_Fail_Replica_Checkpoint_Recovery); + // Note: Since replicaRecover only pertains to cluster-mode, we can use the default store pointers (since multi-db mode is disabled in cluster-mode) if (metadata!.storeIndexToken != default && metadata.storeHlogToken != default) { @@ -88,7 +90,10 @@ public override async ValueTask RecoverCheckpointAsync(bool replicaRecover = fal // be visible at the default log level. Logger?.LogError(ex, "Error during recovery of store; storeVersion = {storeVersion};", storeVersion); - if (StoreWrapper.serverOptions.FailOnRecoveryError) + // A replica that continues here would hold an incomplete store while still advertising the + // replication offset the primary sent it, diverging from the primary with nothing to signal it. + // Fail the sync instead, independently of FailOnRecoveryError, which governs standalone startup. + if (replicaRecover || StoreWrapper.serverOptions.FailOnRecoveryError) throw; } diff --git a/libs/server/Metrics/GarnetServerMonitor.cs b/libs/server/Metrics/GarnetServerMonitor.cs index 4f03adcb9c0..c6e39d5ad83 100644 --- a/libs/server/Metrics/GarnetServerMonitor.cs +++ b/libs/server/Metrics/GarnetServerMonitor.cs @@ -309,6 +309,10 @@ private async Task MainMonitorTaskAsync(CancellationToken token) CleanupGlobalLatencyMetrics(); } } + catch (OperationCanceledException) when (token.IsCancellationRequested) + { + // Suppress the exception if the task was cancelled because the monitor was disposed + } catch (Exception ex) { logger?.LogCritical(ex, "MainMonitorTask exception"); diff --git a/libs/server/Objects/Hash/HashObject.cs b/libs/server/Objects/Hash/HashObject.cs index f88f365a48c..2357be3f6e8 100644 --- a/libs/server/Objects/Hash/HashObject.cs +++ b/libs/server/Objects/Hash/HashObject.cs @@ -156,6 +156,8 @@ public HashObject(Dictionary hash, Dictionary expi this.expirationQueue = expirationQueue; #if NET9_0_OR_GREATER hashSpanLookup = hash.GetAlternateLookup>(); + if (expirationTimes is not null) + expirationTimeSpanLookup = expirationTimes.GetAlternateLookup>(); #endif } diff --git a/libs/server/Resp/Vector/VectorManager.ContextMetadata.cs b/libs/server/Resp/Vector/VectorManager.ContextMetadata.cs index 91f85f88d32..5657a5ee3b1 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); @@ -657,6 +661,8 @@ private void UpdateContextMetadata(ref VectorBasicContext ctx) input.CallbackContext = (nint)Unsafe.AsPointer(ref MemoryMarshal.GetReference(dataSpan)); } + ExceptionInjectionHelper.ResetAndWait(ExceptionInjectionType.VectorSet_Pause_Before_Context_Metadata_Rmw); + var status = ctx.RMW(key, ref input); if (status.IsPending) diff --git a/libs/server/Resp/Vector/VectorManager.Replication.cs b/libs/server/Resp/Vector/VectorManager.Replication.cs index afcd435f146..ff2d5cef843 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,113 @@ 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(); + + /// + /// The local contexts already claimed as the target of a remapping in . + /// + /// A claimed context is marked migrating, and a later incoming context whose own value happens to equal + /// that target would otherwise read that mark as its own interrupted migration and adopt the context + /// directly, leaving two migrated Vector Sets sharing one context. + /// + /// Guarded by lock (this), like the rest of the context metadata. + /// + private readonly HashSet migratedContextTargets = []; + + /// + /// 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(); + migratedContextTargets.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 + && !migratedContextTargets.Contains(migratedContext) + && 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; + _ = migratedContextTargets.Add(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; + _ = migratedContextTargets.Add(localContext); + + _ = dirtyContextMetadatas.Add(contextIndex); + } + + UpdateContextMetadata(ref currentSession.vectorBasicContext); + + return localContext; + } + + /// + /// For testing purposes, resolve a context the way an incoming migrated record would. + /// + public ulong ResolveMigratedContextForTest(ulong migratedContext) + { + using var session = (RespServerSession)getTempSession(); + + return ResolveMigratedContext(session.storageSession, migratedContext); + } + /// /// Vector Set adds are phrased as reads (once the index is created), so they require special handling. /// @@ -194,28 +302,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 +331,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..d3139805952 100644 --- a/libs/server/Resp/Vector/VectorManager.cs +++ b/libs/server/Resp/Vector/VectorManager.cs @@ -6,7 +6,6 @@ using System.Buffers.Binary; using System.Collections.Concurrent; using System.Diagnostics; -using System.Linq; using System.Runtime.InteropServices; using System.Text; using System.Threading.Channels; @@ -172,7 +171,19 @@ private static void EnsureFilterBitmapSize(ref SpanByteAndMemory buffer, int res private readonly int dbId; - private ConcurrentDictionary recoveredIndexes; + /// + /// Context of each Vector Set index record found during recovery, mapped to the hash slot of the key + /// that owns it. restores the reservation for each of these + /// contexts, and needs the hash slot recorded here because the record itself is not retained. + /// + private ConcurrentDictionary recoveredIndexes; + + /// + /// Newest context metadata recovered for each index into contextMetadatas. A snapshot holds + /// every version of a record written in the range it covers, so the same index is presented more than + /// once; entries are kept by ContextMetadata.Version and trimmed by + /// . + /// private ConcurrentDictionary recoveredMetadata; public VectorManager(int dbId, GarnetServerOptions serverOptions, Func getTempSession, ILoggerFactory loggerFactory) @@ -275,6 +286,14 @@ public void ReconcileRecoveredState(bool requireNoReservedContexts = false) ref var ctx = ref session.storageSession.vectorBasicContext; + // recoveredIndexes only holds the index records the store reported while ingesting a checkpoint + // snapshot, which covers a single range of the log - anything already flushed to the main log + // before the checkpoint is recovered without being reported. Liveness below is decided by + // absence, so completing the set from the recovered store is what keeps a live Vector Set from + // being mistaken for an abandoned context and having its element records deleted. + RecoveredIndexScanFunctions indexScan = new(this); + _ = session.storageSession.stringBasicContext.Session.IterateLookupSnapshot(ref indexScan); + var needsUpdated = false; lock (this) @@ -292,22 +311,58 @@ public void ReconcileRecoveredState(bool requireNoReservedContexts = false) } // Any ContextMetadatas we found need to be restored - if (!recoveredMetadata.IsEmpty) + // + // The array must also span every recovered index, because the reservation for a context is + // restored below through contextMetadatas[contextIndex] - the metadata record holding that + // context can be missing when the index record is not. + var maxIndex = -1; + foreach (var (index, metadata) in recoveredMetadata) { - var maxContext = recoveredMetadata.Keys.Max(); - contextMetadatas = new ContextMetadata[maxContext + 1]; + if (!metadata.IsEmpty && index > maxIndex) + { + maxIndex = index; + } + } + + foreach (var context in recoveredIndexes.Keys) + { + var (contextIndex, _) = ContextMetadata.DecomposeContext(context); + if (contextIndex > maxIndex) + { + maxIndex = contextIndex; + } + } + + if (maxIndex >= 0) + { + var priorMetadatas = contextMetadatas; + + contextMetadatas = new ContextMetadata[maxIndex + 1]; for (var i = 0; i < contextMetadatas.Length; i++) { - if (!recoveredMetadata.TryGetValue(i, out contextMetadatas[i])) + if (!recoveredMetadata.TryGetValue(i, out contextMetadatas[i]) || contextMetadatas[i].IsEmpty) { - contextMetadatas[i] = new(); + // Nothing was recovered for this index, so keep whatever is already in memory rather + // than defaulting it. A cluster node with AOF enabled reconciles twice while starting + // up - once from RecoverCheckpointAndAOFAsync and again from StoreWrapper - and the + // first pass consumes recoveredMetadata, so the second has only the index records to + // go on. Defaulting here would drop a reservation that has no index record instead of + // marking it for cleanup below, leaving that context free to hand to the next Vector + // Set while the data behind it is still present, and would reset the persisted + // version so that later metadata writes are cancelled as stale. + contextMetadatas[i] = i < priorMetadatas.Length ? priorMetadatas[i] : new(); } } } 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 +384,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); @@ -388,6 +455,41 @@ public void ReconcileRecoveredState(bool requireNoReservedContexts = false) _ = cleanupTaskChannel.TryPublish(); } + /// + /// Collects every live Vector Set index record in the store into . + /// + private sealed class RecoveredIndexScanFunctions : IScanIteratorFunctions + { + private readonly VectorManager manager; + + internal RecoveredIndexScanFunctions(VectorManager manager) + { + this.manager = manager; + } + + public void OnException(Exception exception, long numberOfRecords) { } + public bool OnStart(long beginAddress, long endAddress) => true; + public void OnStop(bool completed, long numberOfRecords) { } + + /// + public bool Reader(in TSourceLogRecord logRecord, RecordMetadata recordMetadata, long numberOfRecords, out CursorRecordResult cursorRecordResult) + where TSourceLogRecord : ISourceLogRecord + { + cursorRecordResult = CursorRecordResult.Skip; + + if (logRecord.HasNamespace || logRecord.RecordType != RecordType || logRecord.ValueSpan.Length != IndexSize) + { + return true; + } + + ReadIndex(logRecord.ValueSpan, out var context, out _, out _, out _, out _, out _, out _, out _, out _); + manager.recoveredIndexes[context] = HashSlotUtils.HashSlot(logRecord.Key); + + cursorRecordResult = CursorRecordResult.Accept; + return true; + } + } + /// /// Called during recovery for each Vector Set index key. /// @@ -399,7 +501,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); } /// @@ -415,17 +520,19 @@ public void RecoveredContextMetadata(ref TSourceLogRecord reco var index = BinaryPrimitives.ReadInt32LittleEndian(record.Key); var metadata = MemoryMarshal.Cast(record.ValueSpan)[0]; - // During recovery, we can trim off empty ContextMetadata + // A snapshot covers a range of the log, so it holds every version of a record written in that + // range - the same index is legitimately presented more than once, newest version last only if + // each update was copied to the tail. ContextMetadata.Version orders them, and only the newest + // describes the state to restore. // - // ReconcileRecoveredState will fill in any gaps this causes - if (metadata.IsEmpty) + // Empty entries are kept here and trimmed in ReconcileRecoveredState, so that a newest version + // which released its last context still supersedes the older non-empty versions. + lock (recoveredMetadata) { - return; - } - - if (!recoveredMetadata.TryAdd(index, metadata)) - { - throw new GarnetException($"Recovered multiple instances of the same ContextMetadata: {index}"); + if (!recoveredMetadata.TryGetValue(index, out var existing) || metadata.Version >= existing.Version) + { + recoveredMetadata[index] = metadata; + } } } diff --git a/libs/server/Servers/GarnetServerTcp.cs b/libs/server/Servers/GarnetServerTcp.cs index dc95044145d..f29f7faa9ed 100644 --- a/libs/server/Servers/GarnetServerTcp.cs +++ b/libs/server/Servers/GarnetServerTcp.cs @@ -7,6 +7,7 @@ using System.Net; using System.Net.Sockets; using System.Threading; +using System.Threading.Tasks; using Garnet.common; using Garnet.networking; using Garnet.server.TLS; @@ -287,7 +288,14 @@ private unsafe bool HandleNewConnection(SocketAsyncEventArgs e) { IncrementConnectionsReceived(); ExceptionInjectionHelper.TriggerException(ExceptionInjectionType.Dispose_After_Handler_Registered_Before_Start); - handler.Start(tlsOptions?.TlsServerOptions, remoteEndpointName); + + // A TLS handshake takes several network round trips, driven by the peer. Completing it here + // would hold the accept loop for its whole duration, so a peer that connects and negotiates + // slowly - or never - stops this server from accepting any other connection. StartAsync + // performs the socket setup synchronously and leaves only the handshake to the thread pool. + var startTask = handler.StartAsync(tlsOptions?.TlsServerOptions, remoteEndpointName); + if (!startTask.IsCompletedSuccessfully) + _ = ObserveHandlerStartAsync(startTask, handler); } catch (Exception ex) { @@ -310,6 +318,25 @@ private unsafe bool HandleNewConnection(SocketAsyncEventArgs e) return true; } + /// + /// Observes the result of an in-flight handler start so a failed handshake is reported and cleaned up + /// the same way a synchronous failure is, without keeping the accept loop waiting for it. + /// + /// Task returned by the handler's asynchronous start. + /// Handler the task belongs to. + private async Task ObserveHandlerStartAsync(Task startTask, ServerTcpNetworkHandler handler) + { + try + { + await startTask.ConfigureAwait(false); + } + catch (Exception ex) + { + logger?.LogError(ex, "Error calling Start on network handler"); + handler.Dispose(); + } + } + /// /// Create session (message consumer) given incoming bytes /// diff --git a/libs/server/Storage/Functions/GarnetRecordTriggers.cs b/libs/server/Storage/Functions/GarnetRecordTriggers.cs index d9e121e9f99..2fd9c54ce44 100644 --- a/libs/server/Storage/Functions/GarnetRecordTriggers.cs +++ b/libs/server/Storage/Functions/GarnetRecordTriggers.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. using System; +using Garnet.common; using Tsavorite.core; namespace Garnet.server @@ -180,6 +181,7 @@ public readonly void OnCheckpoint(CheckpointTrigger trigger, Guid checkpointToke case CheckpointTrigger.FlushBegin: rangeIndexManager?.SnapshotAllTreesForCheckpoint(checkpointToken); rangeIndexManager?.ClearCheckpointBarrier(); + ExceptionInjectionHelper.ResetAndWait(ExceptionInjectionType.Checkpoint_Pause_At_Flush_Begin); break; case CheckpointTrigger.CheckpointCompleted: vectorManager?.CheckpointCompleted(); diff --git a/libs/server/StoreWrapper.cs b/libs/server/StoreWrapper.cs index 5e81d8be9fc..b7aa164b98d 100644 --- a/libs/server/StoreWrapper.cs +++ b/libs/server/StoreWrapper.cs @@ -688,6 +688,11 @@ async Task CommitTaskAsync(int commitFrequencyMs, CancellationToken token = defa } } } + catch (OperationCanceledException) when (token.IsCancellationRequested) + { + // Suppress the exception if the task was cancelled because the store wrapper was disposed + // or because the node transitioned to a replica and primary-only tasks were suspended + } catch (Exception ex) { logger?.LogError(ex, "CommitTask exception received."); @@ -713,6 +718,11 @@ async Task CompactionTaskAsync(int compactionFrequencySecs, CancellationToken to await Task.Delay(compactionFrequencySecs * 1000, token).ConfigureAwait(false); } } + catch (OperationCanceledException) when (token.IsCancellationRequested) + { + // Suppress the exception if the task was cancelled because the store wrapper was disposed + // or because the node transitioned to a replica and primary-only tasks were suspended + } catch (Exception ex) { logger?.LogError(ex, "CompactionTask exception received."); diff --git a/libs/storage/Tsavorite/cs/src/core/Index/Recovery/Recovery.cs b/libs/storage/Tsavorite/cs/src/core/Index/Recovery/Recovery.cs index 4f0199ecae3..14fa9419caa 100644 --- a/libs/storage/Tsavorite/cs/src/core/Index/Recovery/Recovery.cs +++ b/libs/storage/Tsavorite/cs/src/core/Index/Recovery/Recovery.cs @@ -157,7 +157,7 @@ public struct LogFileInfo /// Address of ; the start of the lowest object log segment /// in use by the hybrid log at snapshot PREPARE time public long hybridLogObjectFileStartAddress; - /// The objectLogTail taken at the start of WAIT_FLUSH, corresponding to the hlog's FlushedUntilAddress at that point + /// The hlogEndObjectLogTail taken at PERSISTENCE_CALLBACK, corresponding to the hlog's FlushedUntilAddress at that point public long hybridLogObjectFileEndAddress; /// The snapshotEndObjectLogTail taken at PERSISTENCE_CALLBACK, which corresponds to the object log position for the final TailAddress /// written by the checkpoint. (Start address is always 0.) @@ -253,7 +253,7 @@ public LogFileInfo GetLogFileSize(Guid token) // a main-log Flush. However the snapshot does cause object-log segments for the mutable range to be written. hasSnapshotObjects = hasSnapshotObjects, hybridLogObjectFileStartAddress = hasSnapshotObjects ? (long)current.info.beginAddressObjectLogSegment << current.info.hlogEndObjectLogTail.SegmentSizeBits : 0, - hybridLogObjectFileEndAddress = hasSnapshotObjects ? (long)current.info.snapshotStartObjectLogTail.CurrentAddress : 0, + hybridLogObjectFileEndAddress = hasSnapshotObjects ? (long)current.info.hlogEndObjectLogTail.CurrentAddress : 0, snapshotObjectFileEndAddress = hasSnapshotObjects ? (long)current.info.snapshotEndObjectLogTail.CurrentAddress : 0, }; diff --git a/libs/storage/Tsavorite/cs/test/test.recovery/ObjectLogCheckpointTransferTests.cs b/libs/storage/Tsavorite/cs/test/test.recovery/ObjectLogCheckpointTransferTests.cs new file mode 100644 index 00000000000..26e3c302f2c --- /dev/null +++ b/libs/storage/Tsavorite/cs/test/test.recovery/ObjectLogCheckpointTransferTests.cs @@ -0,0 +1,207 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Garnet.test; +using NUnit.Framework; +using NUnit.Framework.Legacy; +using Tsavorite.core; +using static Tsavorite.test.TestUtils; + +namespace Tsavorite.test.recovery.objects +{ + using ClassAllocator = ObjectAllocator>; + using ClassStoreFunctions = StoreFunctions; + + /// + /// Verifies that the byte ranges reported by + /// are sufficient to recover a Snapshot checkpoint on a different node. Cluster replication full sync transfers only + /// those ranges of the main log, the main object log, the snapshot and the snapshot object log; the target node then + /// recovers from them, so a range that stops short of the data referenced by the transferred main log leaves the + /// target unable to deserialize its objects. + /// + [TestFixture] + public class ObjectLogCheckpointTransferTests : TestBase + { + const int NumRecords = 6000; + const long MainLogSegmentSize = 1L << 20; + const long ObjectLogSegmentSize = 1L << 30; + const int ThrottleCheckpointFlushDelayMs = 20; + const string LogBaseName = "xfer.log"; + const string ObjectLogBaseName = "xfer.obj.log"; + const string CheckpointDirName = "check-points"; + + string sourceDir; + string targetDir; + + [SetUp] + public void Setup() + { + RecreateDirectory(MethodTestDir); + sourceDir = Path.Combine(MethodTestDir, "source"); + targetDir = Path.Combine(MethodTestDir, "target"); + } + + [TearDown] + public void TearDown() => TestUtils.OnTearDown(); + + // flushMainLogDuringCheckpoint models a concurrent main-log ReadOnly flush - the shift of + // ReadOnlyAddress that flushes the pages it makes immutable - starting after the checkpoint has + // captured its snapshot-start object-log position: it advances both FlushedUntilAddress and the + // main object-log tail before the checkpoint records them at PERSISTENCE_CALLBACK. + [Test] + [Category("TsavoriteKV"), Category("CheckpointRestore")] + public async Task TransferredSnapshotCheckpointRecoversAllObjects([Values] bool flushMainLogDuringCheckpoint) + { + Prepare(sourceDir, out var log, out var objlog, out var store); + Guid token; + LogFileInfo logFileInfo; + try + { + using (var session = store.NewSession(new TestObjectFunctions())) + { + var bContext = session.BasicContext; + for (var ii = 0; ii < NumRecords; ii++) + _ = bContext.Upsert(new TestObjectKey { key = ii }, new TestObjectValue { value = ii }); + } + + ClassicAssert.IsTrue(store.TryInitiateHybridLogCheckpoint(out token, CheckpointType.Snapshot)); + + Task concurrentFlush = null; + if (flushMainLogDuringCheckpoint) + { + concurrentFlush = Task.Run(() => + { + // The flush buffers are created immediately after the snapshot-start object-log position is captured, + // so this waits until the checkpoint is inside its (throttled) snapshot flush. + while (store._hybridLogCheckpoint.objectLogFlushBuffers is null) + Thread.Yield(); + // Page-aligned, as produced by AllocatorBase.CalculateReadOnlyAddress for automatic and size-tracker shifts. + var readOnlyAddress = store.Log.TailAddress & ~(MinKvLogPageSize - 1); + store.Log.ShiftReadOnlyAddress(readOnlyAddress, wait: true); + }); + } + + await store.CompleteCheckpointAsync().AsTask().ConfigureAwait(false); + if (concurrentFlush is not null) + await concurrentFlush.ConfigureAwait(false); + + logFileInfo = store.GetLogFileSize(token); + } + finally + { + Destroy(log, objlog, store); + } + + TransferCheckpoint(token, logFileInfo); + + Prepare(targetDir, out log, out objlog, out store); + try + { + _ = await store.RecoverAsync(default, token).ConfigureAwait(false); + + using var session = store.NewSession(new TestObjectFunctions()); + var bContext = session.BasicContext; + for (var ii = 0; ii < NumRecords; ii++) + { + var key = new TestObjectKey { key = ii }; + TestObjectInput input = default; + TestObjectOutput output = new(); + var status = bContext.Read(key, ref input, ref output); + if (status.IsPending) + { + Assert.That(bContext.CompletePendingWithOutputs(out var completedOutputs, wait: true), Is.True); + (status, output) = GetSinglePendingResult(completedOutputs); + } + + ClassicAssert.IsTrue(status.Found, $"key {ii} not found on the transfer target"); + ClassicAssert.AreEqual(ii, output.value.value, $"key {ii} has the wrong value on the transfer target"); + } + } + finally + { + Destroy(log, objlog, store); + } + } + + // Copies exactly the file ranges that TsavoriteSnapshotReader sends to a replica during full sync. + private void TransferCheckpoint(Guid token, LogFileInfo logFileInfo) + { + var sourceCheckpointDir = Path.Combine(sourceDir, CheckpointDirName, "cpr-checkpoints", token.ToString()); + var targetCheckpointDir = Path.Combine(targetDir, CheckpointDirName, "cpr-checkpoints", token.ToString()); + _ = Directory.CreateDirectory(targetCheckpointDir); + foreach (var metadataFile in Directory.GetFiles(sourceCheckpointDir, "info.dat*")) + File.Copy(metadataFile, Path.Combine(targetCheckpointDir, Path.GetFileName(metadataFile))); + + if (logFileInfo.hybridLogFileEndAddress > PageHeader.Size) + { + CopyRange(Path.Combine(sourceDir, LogBaseName), Path.Combine(targetDir, LogBaseName), + logFileInfo.hybridLogFileStartAddress, logFileInfo.hybridLogFileEndAddress, MainLogSegmentSize); + if (logFileInfo.hasSnapshotObjects) + CopyRange(Path.Combine(sourceDir, ObjectLogBaseName), Path.Combine(targetDir, ObjectLogBaseName), + logFileInfo.hybridLogObjectFileStartAddress, logFileInfo.hybridLogObjectFileEndAddress, ObjectLogSegmentSize); + } + + if (logFileInfo.snapshotFileEndAddress > PageHeader.Size) + { + CopyRange(Path.Combine(sourceCheckpointDir, "snapshot.dat"), Path.Combine(targetCheckpointDir, "snapshot.dat"), + 0, logFileInfo.snapshotFileEndAddress, MainLogSegmentSize); + if (logFileInfo.hasSnapshotObjects) + CopyRange(Path.Combine(sourceCheckpointDir, "snapshot.obj.dat"), Path.Combine(targetCheckpointDir, "snapshot.obj.dat"), + 0, logFileInfo.snapshotObjectFileEndAddress, ObjectLogSegmentSize); + } + } + + private static void CopyRange(string sourceBaseName, string targetBaseName, long startAddress, long endAddress, long segmentSize) + { + for (var segment = startAddress / segmentSize; startAddress < endAddress && segment <= (endAddress - 1) / segmentSize; segment++) + { + var segmentStartAddress = segment * segmentSize; + var from = Math.Max(startAddress, segmentStartAddress) - segmentStartAddress; + var to = Math.Min(endAddress, segmentStartAddress + segmentSize) - segmentStartAddress; + + var buffer = new byte[to - from]; + using (var sourceStream = File.OpenRead($"{sourceBaseName}.{segment}")) + { + _ = sourceStream.Seek(from, SeekOrigin.Begin); + sourceStream.ReadExactly(buffer); + } + + using var targetStream = new FileStream($"{targetBaseName}.{segment}", FileMode.Create, FileAccess.Write); + _ = targetStream.Seek(from, SeekOrigin.Begin); + targetStream.Write(buffer); + } + } + + private static void Prepare(string dir, out IDevice log, out IDevice objlog, out TsavoriteKV store) + { + _ = Directory.CreateDirectory(dir); + log = Devices.CreateLogDevice(Path.Combine(dir, LogBaseName)); + objlog = Devices.CreateLogDevice(Path.Combine(dir, ObjectLogBaseName)); + store = new(new() + { + IndexSize = 1L << 22, + LogDevice = log, + ObjectLogDevice = objlog, + SegmentSize = MainLogSegmentSize, + ObjectLogSegmentSize = ObjectLogSegmentSize, + LogMemorySize = 32 * MinKvLogPageSize, + PageSize = MinKvLogPageSize, + ThrottleCheckpointFlushDelayMs = ThrottleCheckpointFlushDelayMs, + CheckpointDir = Path.Combine(dir, CheckpointDirName) + }, StoreFunctions.Create(new TestObjectKey.Comparer(), () => new TestObjectValue.Serializer()) + , (allocatorSettings, storeFunctions) => new(allocatorSettings, storeFunctions) + ); + } + + private static void Destroy(IDevice log, IDevice objlog, TsavoriteKV store) + { + store.Dispose(); + log.Dispose(); + objlog.Dispose(); + } + } +} \ No newline at end of file diff --git a/test/cluster/Garnet.test.cluster.migrate/ClusterMigrateTests.cs b/test/cluster/Garnet.test.cluster.migrate/ClusterMigrateTests.cs index 1c3f53e2c54..a8f70b00f63 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.multilog/MultiLogTests/ClusterReplicationShardedLog.cs b/test/cluster/Garnet.test.cluster.multilog/MultiLogTests/ClusterReplicationShardedLog.cs index 2be80e64df1..7fe5a1c8844 100644 --- a/test/cluster/Garnet.test.cluster.multilog/MultiLogTests/ClusterReplicationShardedLog.cs +++ b/test/cluster/Garnet.test.cluster.multilog/MultiLogTests/ClusterReplicationShardedLog.cs @@ -462,5 +462,59 @@ public async Task ClusterAofUpgradeSLtoSLMRRecoverAsync([Values] bool useStoredP ClassicAssert.AreEqual(values, result); } } + + [Test, Order(6)] + [Category("REPLICATION")] + public async Task ClusterReplicationSinglePhysicalLogTailWitnessPulseTest() + { + var primaryNodeIndex = 0; + var replicaNodeIndex = 1; + + // One physical sublog replayed by several tasks. Multi-log is on, so the primary emits + // tail witness pulses, but records on a single physical sublog carry no generator + // sequence number and the replay side times its virtual sublogs by log address. + context.CreateInstances(2, + disableObjects: false, + enableAOF: true, + useTLS: useTLS, + asyncReplay: asyncReplay, + sublogCount: 1, + replayTaskCount: TestReplayTaskCount); + context.CreateConnection(useTLS: useTLS); + + var resp = context.clusterTestUtils.AddDelSlotsRange(primaryNodeIndex, [(0, 16383)], addslot: true, logger: context.logger); + ClassicAssert.AreEqual("OK", resp); + + context.clusterTestUtils.SetConfigEpoch(primaryNodeIndex, primaryNodeIndex + 1, logger: context.logger); + context.clusterTestUtils.SetConfigEpoch(replicaNodeIndex, replicaNodeIndex + 1, logger: context.logger); + context.clusterTestUtils.Meet(primaryNodeIndex, replicaNodeIndex, logger: context.logger); + context.clusterTestUtils.WaitUntilNodeIsKnown(primaryNodeIndex, replicaNodeIndex, logger: context.logger); + context.clusterTestUtils.WaitUntilNodeIsKnown(replicaNodeIndex, primaryNodeIndex, logger: context.logger); + + resp = context.clusterTestUtils.ClusterReplicate(replicaNodeIndex, primaryNodeIndex, logger: context.logger); + ClassicAssert.AreEqual("OK", resp); + context.clusterTestUtils.WaitForReplicaRecovery(replicaNodeIndex, logger: context.logger); + + // Leave the AOF sync task idle for longer than AofTailWitnessFreqMs (100 ms by default, + // and the harness does not override it) so it emits at least one pulse before anything is + // written. Everything below this point depends on the sync task having survived that pulse. + await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); + + string[] keys = ["{_}a", "{_}b", "{_}c"]; + string[] values = ["10", "15", "20"]; + for (var i = 0; i < keys.Length; i++) + { + var state = context.clusterTestUtils.SetKey(primaryNodeIndex, Encoding.ASCII.GetBytes(keys[i]), Encoding.ASCII.GetBytes(values[i]), out _, out _, logger: context.logger); + ClassicAssert.AreEqual(ResponseState.OK, state); + } + + context.clusterTestUtils.WaitForReplicaAofSync(primaryNodeIndex, replicaNodeIndex, context.logger); + + for (var i = 0; i < keys.Length; i++) + { + var value = context.clusterTestUtils.GetKey(replicaNodeIndex, Encoding.ASCII.GetBytes(keys[i]), out _, out _, out _); + ClassicAssert.AreEqual(values[i], value, $"At replica, key {keys[i]}"); + } + } } } \ No newline at end of file diff --git a/test/cluster/Garnet.test.cluster.replication.disklesssync/ReplicationTests/ClusterReplicationDisklessSyncTests.cs b/test/cluster/Garnet.test.cluster.replication.disklesssync/ReplicationTests/ClusterReplicationDisklessSyncTests.cs index 4e8467b0cc3..9dfd72c7ed7 100644 --- a/test/cluster/Garnet.test.cluster.replication.disklesssync/ReplicationTests/ClusterReplicationDisklessSyncTests.cs +++ b/test/cluster/Garnet.test.cluster.replication.disklesssync/ReplicationTests/ClusterReplicationDisklessSyncTests.cs @@ -86,13 +86,7 @@ void ResetAndReAttach(int replicaIndex, int primaryIndex, bool soft) void Failover(int replicaIndex, string option = null) { _ = context.clusterTestUtils.ClusterFailover(replicaIndex, option, logger: context.logger); - var role = context.clusterTestUtils.RoleCommand(replicaIndex, logger: context.logger); - while (!role.Value.Equals("master")) - { - ClusterTestUtils.BackOff(cancellationToken: context.cts.Token); - role = context.clusterTestUtils.RoleCommand(replicaIndex, logger: context.logger); - } - + context.clusterTestUtils.WaitForPrimaryRole(replicaIndex, context.logger); } // Deterministic field value so a huge hash can be verified without holding every value in memory. diff --git a/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs b/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs index edff700fa70..56a74f583de 100644 --- a/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs +++ b/test/cluster/Garnet.test.cluster.replication/ReplicationTests/ClusterReplicationBaseTests.cs @@ -672,6 +672,9 @@ public void ClusterReplicationSimpleFailover([Values] bool performRMW, [Values] // Enable when old primary becomes replica context.clusterTestUtils.WaitForReplicaRecovery(primaryIndex, logger: context.logger); + // The promoted node has to be usable as a primary by this client before writes are sent to it + context.clusterTestUtils.WaitForPrimaryRole(replicaIndex, context.logger); + // Check if allowed to write to new Primary if (!performRMW) context.PopulatePrimary(ref context.kvPairs, keyLength, kvpairCount, replicaIndex, slotMap: slotMap); @@ -744,6 +747,9 @@ public void ClusterFailoverAttachReplicas([Values] bool performRMW, [Values] boo context.clusterTestUtils.WaitForNoFailover(1, context.logger); context.clusterTestUtils.WaitForNoFailover(2, context.logger); + // Node 1 has to be usable as a primary by this client before writes are sent to it + context.clusterTestUtils.WaitForPrimaryRole(1, context.logger); + // Wait for replica to recover context.clusterTestUtils.WaitForReplicaRecovery(2, context.logger); @@ -1001,6 +1007,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) @@ -1013,7 +1024,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 3bf35f61efd..5634d080390 100644 --- a/test/cluster/Garnet.test.cluster.vectorsets/VectorSets/ClusterVectorSetTests.cs +++ b/test/cluster/Garnet.test.cluster.vectorsets/VectorSets/ClusterVectorSetTests.cs @@ -1031,7 +1031,7 @@ public async Task VectorSetMigrateSingleBySlotAsync() var success = false; string lastResponse = null; - while (Stopwatch.GetElapsedTime(start) < TimeSpan.FromSeconds(15)) + while (Stopwatch.GetElapsedTime(start) < TimeSpan.FromSeconds(5)) { try { @@ -1337,9 +1337,13 @@ public void VectorSetMigrateManyBySlot() ClassicAssert.IsTrue(exc0.StartsWith("Key has MOVED to ")); } + // Every path through this loop has to sleep. Polling without one spins a core flat out for the + // whole window, and on a two-core runner that starves the cluster nodes this is waiting on, so + // the convergence it is polling for cannot happen until the window has already elapsed. var start = Stopwatch.GetTimestamp(); var success = false; + string lastResponse = null; while (Stopwatch.GetElapsedTime(start) < TimeSpan.FromSeconds(5)) { 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; + + // 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(5)) { 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/ClusterConfigTests.cs b/test/cluster/Garnet.test.cluster/ClusterConfigTests.cs index e4b11afa9b3..f3df00446bd 100644 --- a/test/cluster/Garnet.test.cluster/ClusterConfigTests.cs +++ b/test/cluster/Garnet.test.cluster/ClusterConfigTests.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Net; @@ -503,5 +505,198 @@ public void ClusterConfigMergeSlotMapReplicaHandoffDoesNotLeakToLaterSlotsTest() Assert.That(merged.GetWorkerIdFromSlot(UnownedSlot), Is.EqualTo(ClusterConfig.RESERVED_WORKER_ID), "the later unowned slot must not inherit the correction computed for the earlier slot"); } + + /// + /// Models one node of a gossiping cluster: its published configuration plus the two caches that + /// decide whether a configuration is actually put on the wire. + /// + private sealed class GossipNode + { + public string NodeId; + + /// + /// ClusterManager.currentConfig. + /// + public ClusterConfig Config; + + /// + /// GarnetServerNode.lastConfig, one per outgoing gossip connection. + /// + public readonly Dictionary LastConfigSentTo = []; + + /// + /// ClusterCommands.lastSentConfig, one per inbound gossip session. + /// + public readonly Dictionary LastConfigRepliedTo = []; + } + + private static readonly ConcurrentDictionary EmptyBanList = new(); + + /// + /// Models ClusterManager.TryMerge. + /// + private static void GossipTryMerge(GossipNode node, ClusterConfig senderConfig) + { + var currentCopy = node.Config.Copy(); + var next = currentCopy.Merge(senderConfig, EmptyBanList).HandleConfigEpochCollision(senderConfig); + if (currentCopy != next) + node.Config = next; + } + + /// + /// Models one gossip exchange: GarnetServerNode.TryGossip sends a configuration only when the local + /// configuration object changed since the last send on that connection, the receiver merges it and + /// replies with its own pre-merge configuration under the same rule, and the sender merges the reply. + /// + private static void GossipExchange(GossipNode from, GossipNode to, bool withMeet = false) + { + byte[] request; + if (!from.LastConfigSentTo.TryGetValue(to.NodeId, out var lastSent) || lastSent != from.Config) + { + from.LastConfigSentTo[to.NodeId] = from.Config; + request = from.Config.ToByteArray(); + } + else + { + request = []; + } + + var receiverConfig = to.Config; + if (request.Length > 0) + { + var senderConfig = ClusterConfig.FromByteArray(request); + if (withMeet || receiverConfig.IsKnown(senderConfig.LocalNodeId)) + GossipTryMerge(to, senderConfig); + } + + byte[] reply; + if (withMeet || !to.LastConfigRepliedTo.TryGetValue(from.NodeId, out var lastReplied) || lastReplied != receiverConfig) + { + to.LastConfigRepliedTo[from.NodeId] = receiverConfig; + reply = receiverConfig.ToByteArray(); + } + else + { + reply = []; + } + + if (reply.Length > 0) + { + var replyConfig = ClusterConfig.FromByteArray(reply); + + // A MEET response is merged unconditionally, the peer is trusted because an admin issued the meet. + if (withMeet || from.Config.IsKnown(replyConfig.LocalNodeId)) + GossipTryMerge(from, replyConfig); + } + } + + /// + /// Replays what SimpleSetupCluster does: assign slots to the two primaries, set config epochs, then + /// issue every CLUSTER MEET from the first node. + /// + private static List BuildGossipCluster() + { + var nodes = new List(); + for (var i = 0; i < 4; i++) + { + var nodeId = Generator.CreateHexId(); + nodes.Add(new GossipNode + { + NodeId = nodeId, + Config = new ClusterConfig().InitializeLocalWorker( + nodeId, "127.0.0.1", ClusterTestContext.Port + i, configEpoch: 0, + Garnet.cluster.NodeRole.PRIMARY, null, "") + }); + } + + nodes[0].Config = nodes[0].Config.AssignSlots([.. Enumerable.Range(0, 8192)], ClusterConfig.LOCAL_WORKER_ID, SlotState.STABLE); + nodes[1].Config = nodes[1].Config.AssignSlots([.. Enumerable.Range(8192, 8192)], ClusterConfig.LOCAL_WORKER_ID, SlotState.STABLE); + + for (var i = 0; i < nodes.Count; i++) + nodes[i].Config = nodes[i].Config.SetLocalWorkerConfigEpoch(i + 1); + + for (var i = 1; i < nodes.Count; i++) + GossipExchange(nodes[0], nodes[i], withMeet: true); + + return nodes; + } + + /// + /// The state WaitForSyncAsync waits for: every node agrees that the two primaries own their halves of + /// the slot space and that the two replicas own nothing and are flagged as replicas. + /// + private static bool GossipClusterConverged(List nodes) + { + foreach (var node in nodes) + { + for (var slot = 0; slot < 8192; slot++) + if (node.Config.GetNodeIdFromSlot((ushort)slot) != nodes[0].NodeId) return false; + + for (var slot = 8192; slot < 16384; slot++) + if (node.Config.GetNodeIdFromSlot((ushort)slot) != nodes[1].NodeId) return false; + + if (node.Config.GetNodeRoleFromNodeId(nodes[2].NodeId) != Garnet.cluster.NodeRole.REPLICA) return false; + if (node.Config.GetNodeRoleFromNodeId(nodes[3].NodeId) != Garnet.cluster.NodeRole.REPLICA) return false; + } + return true; + } + + private static string DescribeGossipCluster(List nodes) + { + var sb = new StringBuilder(); + foreach (var node in nodes) + { + var info = node.Config.GetClusterInfo(null); + for (var i = 0; i < nodes.Count; i++) + info = info.Replace(nodes[i].NodeId, $"n{i}"); + sb.Append(info).Append('\n'); + } + return sb.ToString(); + } + + /// + /// Drives the real merge code through the cluster layout SimpleSetupCluster builds, over many gossip + /// orderings. A configuration is only put on the wire when the sender's configuration object changed + /// since its last send on that connection, so a view that a merge refuses to take is never offered + /// again and any divergence the merge rules allow is permanent rather than transient. + /// + [Test, Order(13)] + [Category("CLUSTER-CONFIG"), CancelAfter(60_000)] + public void ClusterConfigGossipConvergesForEveryOrderingTest() + { + const int Trials = 200; + const int RoundsPerTrial = 60; + + var pairs = new List<(int From, int To)>(); + for (var from = 0; from < 4; from++) + for (var to = 0; to < 4; to++) + if (from != to) pairs.Add((from, to)); + + var random = new Random(12345); + for (var trial = 0; trial < Trials; trial++) + { + var nodes = BuildGossipCluster(); + + // CLUSTER REPLICATE bumps the local config epoch so the role change can propagate. Gossip runs + // between the two calls because the test waits for the first primary to see its replica. + nodes[2].Config = nodes[2].Config.MakeReplicaOf(nodes[0].NodeId).BumpLocalNodeConfigEpoch(); + var interleaved = random.Next(0, 8); + for (var i = 0; i < interleaved; i++) + { + var (from, to) = pairs[random.Next(pairs.Count)]; + GossipExchange(nodes[from], nodes[to]); + } + nodes[3].Config = nodes[3].Config.MakeReplicaOf(nodes[1].NodeId).BumpLocalNodeConfigEpoch(); + + for (var round = 0; round < RoundsPerTrial && !GossipClusterConverged(nodes); round++) + { + foreach (var (from, to) in pairs.OrderBy(_ => random.Next())) + GossipExchange(nodes[from], nodes[to]); + } + + Assert.That(GossipClusterConverged(nodes), Is.True, + $"cluster did not converge after {RoundsPerTrial} gossip rounds in trial {trial}:\n{DescribeGossipCluster(nodes)}"); + } + } } } \ No newline at end of file diff --git a/test/cluster/Garnet.test.cluster/ClusterNegativeTests.cs b/test/cluster/Garnet.test.cluster/ClusterNegativeTests.cs index e0033c27c54..b966212938f 100644 --- a/test/cluster/Garnet.test.cluster/ClusterNegativeTests.cs +++ b/test/cluster/Garnet.test.cluster/ClusterNegativeTests.cs @@ -471,6 +471,52 @@ public void ClusterReplicaSyncTimeoutTest() context.ValidateKVCollectionAgainstReplica(ref context.kvPairs, replicaIndex); } + + [Test, Order(6), CancelAfter(testTimeout)] + public void ClusterReplicaCheckpointRecoveryFailureAbortsSyncTest() + { + var primaryIndex = 0; + var replicaIndex = 1; + var nodes_count = 2; + context.CreateInstances(nodes_count, disableObjects: false, enableAOF: true, timeout: timeout); + context.CreateConnection(); + + _ = context.clusterTestUtils.AddDelSlotsRange(primaryIndex, [(0, 16383)], addslot: true, logger: context.logger); + context.clusterTestUtils.SetConfigEpoch(primaryIndex, primaryIndex + 1, logger: context.logger); + context.clusterTestUtils.SetConfigEpoch(replicaIndex, replicaIndex + 1, logger: context.logger); + context.clusterTestUtils.Meet(primaryIndex, replicaIndex, logger: context.logger); + + var keyLength = 32; + var kvpairCount = 32; + context.kvPairs = []; + context.PopulatePrimary(ref context.kvPairs, keyLength, kvpairCount, primaryIndex, null); + + // Take a checkpoint so the replica performs a disk-based full sync and recovers from the shipped token + var primaryLastSaveTime = context.clusterTestUtils.LastSave(primaryIndex, logger: context.logger); + context.clusterTestUtils.WaitUntilNextSecond(primaryIndex, primaryLastSaveTime); + context.clusterTestUtils.Checkpoint(primaryIndex, logger: context.logger); + context.clusterTestUtils.WaitCheckpoint(primaryIndex, primaryLastSaveTime, logger: context.logger); + + try + { + ExceptionInjectionHelper.EnableException(ExceptionInjectionType.Replication_Fail_Replica_Checkpoint_Recovery); + + var respReplicate = context.clusterTestUtils.ClusterReplicate(replicaNodeIndex: replicaIndex, primaryNodeIndex: primaryIndex, failEx: false, logger: context.logger); + + // Recovery failed before applying any of the checkpoint, so the replica holds none of the primary's + // data. Reporting success here is what lets a replica advertise the primary's replication offset + // over an empty store, so the sync must fail and carry the underlying error. + ClassicAssert.AreEqual(0, context.clusterTestUtils.DBSize(replicaIndex, logger: context.logger), + "recovery must have left the replica store empty for this scenario to be under test"); + + Assert.That(respReplicate, Does.Contain(nameof(ExceptionInjectionType.Replication_Fail_Replica_Checkpoint_Recovery)), + "a failed replica checkpoint recovery must abort the sync and surface the underlying error"); + } + finally + { + ExceptionInjectionHelper.DisableException(ExceptionInjectionType.Replication_Fail_Replica_Checkpoint_Recovery); + } + } #endif [Test, Order(10), CancelAfter(60_000)] @@ -595,6 +641,20 @@ public void ClusterDontKnowReplicaFailTest([Values] bool useReplicaOf) } } + /// + /// Build a single-endpoint configuration matching the servers these tests create. + /// + /// Endpoint to connect to. + /// ACL user to authenticate as. + /// Password for . + static ConfigurationOptions GetNegativeTestConfig(IPEndPoint endPoint, string user, string password) + => TestUtils.GetConfig( + new EndPointCollection { endPoint }, + allowAdmin: true, + disablePubSub: true, + authUsername: user, + authPassword: password); + [Test, Order(13)] [Category("REPLICATION")] public void ClusterReplicateFails() @@ -616,13 +676,17 @@ public void ClusterReplicateFails() ClassicAssert.AreNotEqual(primaryEndpoint, replicaEndpoint, "Should have different endpoints for nodes"); - using var primaryConnection = ConnectionMultiplexer.Connect($"{primaryEndpoint.Address}:{primaryEndpoint.Port},user={UserName},password={Password},allowAdmin=true"); + // Connect through the shared harness configuration rather than a hand-built connection + // string: it trims the command map to match the pub/sub-disabled servers created above, + // applies the suite's connect retry policy, and enables exception detail and client names + // so a connect failure identifies the test and endpoint. + using var primaryConnection = ConnectionMultiplexer.Connect(GetNegativeTestConfig(primaryEndpoint, UserName, Password)); var primaryServer = primaryConnection.GetServer(primaryEndpoint); ClassicAssert.AreEqual("OK", (string)primaryServer.Execute("CLUSTER", ["ADDSLOTSRANGE", "0", "16383"], flags: CommandFlags.NoRedirect)); ClassicAssert.AreEqual("OK", (string)primaryServer.Execute("CLUSTER", ["MEET", replicaEndpoint.Address.ToString(), replicaEndpoint.Port.ToString()], flags: CommandFlags.NoRedirect)); - using var replicaConnection = ConnectionMultiplexer.Connect($"{replicaEndpoint.Address}:{replicaEndpoint.Port},user={UserName},password={Password},allowAdmin=true"); + using var replicaConnection = ConnectionMultiplexer.Connect(GetNegativeTestConfig(replicaEndpoint, UserName, Password)); var replicaServer = replicaConnection.GetServer(replicaEndpoint); // Try to replicate from a server that doesn't exist @@ -840,6 +904,68 @@ public async Task ClusterFailoverResetsPrimaryOnTakeOverFailureAsync(Cancellatio // Verify data is still accessible from the primary context.ValidateKVCollectionAgainstReplica(ref context.kvPairs, primaryIndex); } + + [Test, Order(17), CancelAfter(testTimeout)] + [Category("CLUSTER")] + public void ClusterGossipSurvivesFailedRound() + { + const int nodeCount = 3; + List<(int, int)>[] ranges = [[(0, 5460)], [(5461, 10921)], [(10922, 16383)]]; + + context.CreateInstances(nodeCount, timeout: timeout); + context.CreateConnection(); + + // Introduce the nodes to each other. This runs on the meet task rather than the main gossip loop, + // so it still completes while the loop below is failing. + for (var i = 0; i < nodeCount; i++) + context.clusterTestUtils.SetConfigEpoch(i, i + 1, logger: context.logger); + for (var i = 1; i < nodeCount; i++) + { + context.clusterTestUtils.Meet(0, i, logger: context.logger); + context.clusterTestUtils.WaitUntilNodeIsKnown(0, i, logger: context.logger); + } + for (var i = 0; i < nodeCount; i++) + context.clusterTestUtils.WaitClusterNodesSync(i, nodeCount, context.logger); + + // Fail a round on every node's gossip loop. Nothing restarts that loop, so a round that is allowed + // to terminate it leaves the node silent for the rest of the process. + ExceptionInjectionHelper.EnableException(ExceptionInjectionType.Cluster_Gossip_Round_Fail); + try + { + // Longer than the gossip interval, so every node has run at least one failing round. + Thread.Sleep(TimeSpan.FromSeconds(7)); + } + finally + { + ExceptionInjectionHelper.DisableException(ExceptionInjectionType.Cluster_Gossip_Round_Fail); + } + + // Nothing is failing any more, so the assignment made now has to reach every node. + for (var i = 0; i < nodeCount; i++) + _ = context.clusterTestUtils.AddDelSlotsRange(i, ranges[i], addslot: true, logger: context.logger); + + var deadline = DateTime.UtcNow.AddSeconds(30); + while (true) + { + var pending = -1; + for (var i = 0; i < nodeCount; i++) + { + var slots = context.clusterTestUtils.ClusterSlots(i, context.logger); + if (slots == null || slots.Count != nodeCount) + { + pending = i; + break; + } + } + + if (pending < 0) + break; + + ClassicAssert.Less(DateTime.UtcNow, deadline, + $"node {pending} never converged after a gossip round failed"); + Thread.Sleep(100); + } + } #endif } } \ No newline at end of file diff --git a/test/cluster/Garnet.test.cluster/ClusterTestContext.cs b/test/cluster/Garnet.test.cluster/ClusterTestContext.cs index 6e145252533..b7437ed2c1e 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); } /// @@ -831,18 +909,7 @@ public void ClusterFailoverSpinWait(int replicaNodeIndex, ILogger logger) _ = clusterTestUtils.ClusterFailover(replicaNodeIndex, "ABORT", logger); _ = clusterTestUtils.ClusterFailover(replicaNodeIndex, logger: logger); - var retryCount = 0; - while (true) - { - var role = clusterTestUtils.GetReplicationRole(replicaNodeIndex, logger: logger); - if (role.Equals("master")) break; - if (retryCount++ > 10000) - { - logger?.LogError("CLUSTER FAILOVER retry count reached"); - Assert.Fail(); - } - Thread.Sleep(1000); - } + clusterTestUtils.WaitForPrimaryRole(replicaNodeIndex, logger); } /// Opens a connection, assigns all slots to primaryIndex, and introduces every other created node. diff --git a/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs b/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs index 06ea1e14812..bebe108af4d 100644 --- a/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs +++ b/test/cluster/Garnet.test.cluster/ClusterTestUtils.cs @@ -16,7 +16,6 @@ using Garnet.client; using Garnet.common; using Garnet.server; -using Garnet.server.TLS; using GarnetClusterManagement; using Microsoft.Extensions.Logging; using NUnit.Framework; @@ -408,7 +407,10 @@ private async Task WaitForSyncAsync(ClientClusterConfig clusterConfig) if (count > 0) { - await BackOffAsync(cancellationToken: context.cts.Token).ConfigureAwait(false); + var msg = context.cts.IsCancellationRequested + ? DescribeSyncMismatch(server.EndPoint, expectedConfig, nodes) + : null; + await BackOffAsync(cancellationToken: context.cts.Token, msg: msg).ConfigureAwait(false); goto retry; } } @@ -416,6 +418,27 @@ private async Task WaitForSyncAsync(ClientClusterConfig clusterConfig) } } + /// + /// Describes which of the expected nodes a server has not yet agreed on, so a cluster that never + /// converges reports what it was waiting for. + /// + /// Server whose view was inspected. + /// Expected cluster configuration, keyed by node id. + /// Nodes the server reported, or null if it reported none. + private static string DescribeSyncMismatch(EndPoint endPoint, Dictionary expectedConfig, IEnumerable nodes) + { + var sb = new StringBuilder($"Cluster did not converge; {endPoint} disagrees on:"); + foreach (var (nodeId, raw) in expectedConfig) + { + var node = nodes?.FirstOrDefault(n => n.NodeId == nodeId); + if (node == null) + sb.Append($"\n {nodeId}: not known to this node. Expected: {raw.Trim()}"); + else if (!NodesEqual(new ClientClusterNode(raw.Trim()), node)) + sb.Append($"\n {nodeId}: expected [{raw.Trim()}] actual [{node.Raw?.Trim()}]"); + } + return sb.ToString(); + } + public (List, List) SimpleSetupCluster( int primary_count = -1, int replica_count = -1, @@ -906,7 +929,7 @@ public GarnetClientSession GetGarnetClientSession(int nodeIndex, bool useTLS = f { sslOptions = new SslClientAuthenticationOptions { - ClientCertificates = [CertificateUtils.GetMachineCertificateByFile(certFile, certPassword)], + ClientCertificates = [TestUtils.GetClientCertificate()], TargetHost = "GarnetTest", AllowRenegotiation = false, RemoteCertificateValidationCallback = TestUtils.ValidateServerCertificate, @@ -928,7 +951,7 @@ public GarnetClientSession CreateGarnetClientSession(int nodeIndex, bool useTLS { sslOptions = new SslClientAuthenticationOptions { - ClientCertificates = [CertificateUtils.GetMachineCertificateByFile(certFile, certPassword)], + ClientCertificates = [TestUtils.GetClientCertificate()], TargetHost = "GarnetTest", AllowRenegotiation = false, RemoteCertificateValidationCallback = TestUtils.ValidateServerCertificate, @@ -2282,6 +2305,10 @@ public void ResetReplica(int replicaIndex, int primaryIndex, ILogger logger = nu public string ClusterReplicate(int replicaNodeIndex, int primaryNodeIndex, bool async = false, bool failEx = true, ILogger logger = null) { var primaryId = ClusterMyId(primaryNodeIndex, logger: logger); + // CLUSTER REPLICATE resolves the primary's node id against the replica's own configuration. + // MEET only makes the node it is sent to aware of its target; the reverse direction arrives + // through the gossip handshake that follows, so wait for it before issuing the command. + WaitUntilNodeIdIsKnown(replicaNodeIndex, primaryId, logger: logger); return ClusterReplicate(replicaNodeIndex, primaryId, async: async, failEx: failEx, logger: logger); } @@ -3433,13 +3460,23 @@ public void WaitForFailoverCompleted(IPEndPoint endPoint, ILogger logger = null) } } + /// + /// Waits until the node reports the primary role and the multiplexer has observed it. + /// The multiplexer caches a per-endpoint replica flag and rejects a primary-only command + /// before it leaves the client, refreshing that flag only on its periodic configuration + /// check, so a promotion is not usable until the topology has been re-read. + /// public void WaitForPrimaryRole(int nodeIndex, ILogger logger = null) { + var endPoint = GetEndPoint(nodeIndex); while (true) { - var role = RoleCommand(nodeIndex, logger); - if (role.Value.Equals("master")) break; - BackOff(cancellationToken: context.cts.Token); + if (RoleCommand(endPoint, logger).Value.Equals("master")) + { + if (!redis.GetServer(endPoint).IsReplica) break; + _ = redis.Configure(); + } + BackOff(cancellationToken: context.cts.Token, msg: nameof(WaitForPrimaryRole)); } } diff --git a/test/standalone/Garnet.test.collections/MemberExpiry.cs b/test/standalone/Garnet.test.collections/MemberExpiry.cs new file mode 100644 index 00000000000..97a0f6cd038 --- /dev/null +++ b/test/standalone/Garnet.test.collections/MemberExpiry.cs @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Garnet.test +{ + /// + /// Absolute expiration deadlines for tests that exercise member-level expiration of hash fields + /// and sorted set members. Deadlines are absolute rather than relative so that no assertion + /// depends on an upper bound on the wall-clock time that elapses between commands. + /// + internal static class MemberExpiry + { + /// + /// Milliseconds by which leads the current time. The deadline has to be + /// in the future when the server processes the command: a deadline that has already passed + /// makes the server drop the member outright instead of letting it expire lazily. + /// + private const int ImminentLeadMs = 200; + + /// + /// Time to live, in milliseconds, of a deadline returned by . + /// + public const long PendingTtlMs = 60 * 60 * 1000; + + /// + /// An absolute deadline, in unix milliseconds, that is not reached while a test runs. A member + /// carrying it has a pending expiration but always reads back as live. + /// + public static long Pending() => DateTimeOffset.UtcNow.ToUnixTimeMilliseconds() + PendingTtlMs; + + /// + /// An absolute deadline, in unix milliseconds, shortly in the future. Pair it with + /// or to drive a member into the + /// expired state through the lazy expiration path. + /// + public static long Imminent() => DateTimeOffset.UtcNow.ToUnixTimeMilliseconds() + ImminentLeadMs; + + /// + /// Blocks until the wall clock is strictly past , which is the + /// condition under which the server reports a member as expired. + /// + /// Absolute deadline in unix milliseconds. + public static void WaitUntilPast(long deadlineMs) + { + for (var remaining = RemainingMs(deadlineMs); remaining >= 0; remaining = RemainingMs(deadlineMs)) + Thread.Sleep((int)remaining + 1); + } + + /// + /// Awaits until the wall clock is strictly past , which is the + /// condition under which the server reports a member as expired. + /// + /// Absolute deadline in unix milliseconds. + public static async Task WaitUntilPastAsync(long deadlineMs) + { + for (var remaining = RemainingMs(deadlineMs); remaining >= 0; remaining = RemainingMs(deadlineMs)) + await Task.Delay((int)remaining + 1).ConfigureAwait(false); + } + + private static long RemainingMs(long deadlineMs) => deadlineMs - DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); + } +} \ No newline at end of file diff --git a/test/standalone/Garnet.test.collections/RespHashTests.cs b/test/standalone/Garnet.test.collections/RespHashTests.cs index a565955c95e..e6ba50fcb8f 100644 --- a/test/standalone/Garnet.test.collections/RespHashTests.cs +++ b/test/standalone/Garnet.test.collections/RespHashTests.cs @@ -4,7 +4,6 @@ using System; using System.Linq; using System.Text; -using System.Threading; using System.Threading.Tasks; using Garnet.server; using NUnit.Framework; @@ -64,10 +63,17 @@ public async Task CanSetAndGetOnePairWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Title", "Tsavorite")]); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); string r = db.HashGet("user:user1", "Title"); ClassicAssert.AreEqual("Tsavorite", r); - await Task.Delay(200).ConfigureAwait(false); + + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); + r = db.HashGet("user:user1", "Title"); ClassicAssert.IsNull(r); } @@ -164,7 +170,7 @@ public void CanDelWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Title", "Tsavorite"), new HashEntry("Year", "2021")]); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(MemberExpiry.PendingTtlMs)); var result = db.HashDelete(new RedisKey("user:user1"), new RedisValue("Title")); ClassicAssert.AreEqual(true, result); string resultGet = db.HashGet("user:user1", "Year"); @@ -214,10 +220,17 @@ public async Task CanDoHLenWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Title", "Tsavorite"), new HashEntry("Year", "2021"), new HashEntry("Company", "Acme")]); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); var result = db.HashLength("user:user1"); ClassicAssert.AreEqual(3, result); - await Task.Delay(150).ConfigureAwait(false); + + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); + result = db.HashLength("user:user1"); ClassicAssert.AreEqual(2, result); db.HashSet("user:user1", [new HashEntry("Year", "new2021")]); // Trigger deletion of expired field @@ -247,14 +260,19 @@ public async Task CanDoGetAllWithExpire() HashEntry[] hashEntries = [new HashEntry("Title", "Tsavorite"), new HashEntry("Year", "2021"), new HashEntry("Company", "Acme")]; db.HashSet("user:user1", hashEntries); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); var result = db.HashGetAll("user:user1"); ClassicAssert.AreEqual(hashEntries.Length, result.Length); ClassicAssert.AreEqual(hashEntries.Length, result.Select(r => r.Name).Distinct().Count()); ClassicAssert.IsTrue(hashEntries.OrderBy(e => e.Name).SequenceEqual(result.OrderBy(r => r.Name))); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); result = db.HashGetAll("user:user1"); ClassicAssert.AreEqual(hashEntries.Length - 1, result.Length); @@ -285,12 +303,17 @@ public async Task CanDoHExistsWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Title", "Tsavorite"), new HashEntry("Year", "2021"), new HashEntry("Company", "Acme")]); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); var result = db.HashExists(new RedisKey("user:user1"), "Title"); ClassicAssert.IsTrue(result); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); result = db.HashExists(new RedisKey("user:user1"), "Title"); ClassicAssert.IsFalse(result); @@ -320,12 +343,17 @@ public async Task CanDoHStrLenWithExire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Title", "Tsavorite")]); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); long r = db.HashStringLength("user:user1", "Title"); ClassicAssert.AreEqual(9, r); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); r = db.HashStringLength("user:user1", "Title"); ClassicAssert.AreEqual(0, r); @@ -355,14 +383,19 @@ public async Task CanDoHKeysWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Title", "Tsavorite"), new HashEntry("Year", "2021"), new HashEntry("Company", "Acme")]); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); var result = db.HashKeys("user:user1"); ClassicAssert.AreEqual(3, result.Length); ClassicAssert.IsTrue(Array.Exists(result, t => t.Equals("Title"))); ClassicAssert.IsTrue(Array.Exists(result, t => t.Equals("Year"))); ClassicAssert.IsTrue(Array.Exists(result, t => t.Equals("Company"))); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); result = db.HashKeys("user:user1"); ClassicAssert.AreEqual(2, result.Length); @@ -398,14 +431,19 @@ public async Task CanDoHValsWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Title", "Tsavorite"), new HashEntry("Year", "2021"), new HashEntry("Company", "Acme")]); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); var result = db.HashValues("user:user1"); ClassicAssert.AreEqual(3, result.Length); ClassicAssert.IsTrue(Array.Exists(result, t => t.Equals("Tsavorite"))); ClassicAssert.IsTrue(Array.Exists(result, t => t.Equals("2021"))); ClassicAssert.IsTrue(Array.Exists(result, t => t.Equals("Acme"))); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); result = db.HashValues("user:user1"); ClassicAssert.AreEqual(2, result.Length); @@ -445,11 +483,16 @@ public async Task CanDoHIncrByWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Field1", "StringValue"), new HashEntry("Field2", "1")]); - db.HashFieldExpire("user:user1", ["Field2"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Field2"); var result = db.HashIncrement(new RedisKey("user:user1"), new RedisValue("Field2"), -4); ClassicAssert.AreEqual(-3, result); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Field2"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); result = db.HashIncrement(new RedisKey("user:user1"), new RedisValue("Field2"), -4); ClassicAssert.AreEqual(-4, result); @@ -503,11 +546,16 @@ public async Task CheckHashIncrementDoublePrecisionWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet("user:user1", [new HashEntry("Field1", "1.1111111111")]); - db.HashFieldExpire("user:user1", ["Field1"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Field1"); var result = db.HashIncrement(new RedisKey("user:user1"), new RedisValue("Field1"), 2.2222222222); ClassicAssert.AreEqual(3.3333333333, result, 1e-15); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Field1"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); result = db.HashIncrement(new RedisKey("user:user1"), new RedisValue("Field1"), 2.2222222222); ClassicAssert.AreEqual(2.2222222222, result, 1e-15); @@ -532,10 +580,20 @@ public async Task CanDoHSETNXCommandWithExpire() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); db.HashSet(new RedisKey("user:user1"), new RedisValue("Field"), new RedisValue("Hello")); - db.HashFieldExpire("user:user1", ["Field"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + var pending = MemberExpiry.Pending(); + db.Execute("HPEXPIREAT", "user:user1", pending, "FIELDS", "1", "Field"); db.HashSet(new RedisKey("user:user1"), new RedisValue("Field"), new RedisValue("Hello"), When.NotExists); - await Task.Delay(200).ConfigureAwait(false); + // SetNX on a field that is present is a no-op and leaves the expiration untouched + var expireTime = (RedisResult[])db.Execute("HPEXPIRETIME", "user:user1", "FIELDS", "1", "Field"); + ClassicAssert.AreEqual(pending, (long)expireTime[0]); + + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Field"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); string result = db.HashGet("user:user1", "Field"); ClassicAssert.IsNull(result); // SetNX should not reset the expiration @@ -618,11 +676,16 @@ public async Task CanDoRandomFieldWithExpire() var hashKey = new RedisKey("user:user1"); HashEntry[] hashEntries = [new HashEntry("Title", "Tsavorite")]; db.HashSet(hashKey, hashEntries); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); string field = db.HashRandomField(hashKey); ClassicAssert.AreEqual(field, "Title"); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); field = db.HashRandomField(hashKey); ClassicAssert.IsNull(field); @@ -637,12 +700,17 @@ public async Task CanDoRandomFieldsWithExpire() var hashKey = new RedisKey("user:user1"); HashEntry[] hashEntries = [new HashEntry("Title", "Tsavorite")]; db.HashSet(hashKey, hashEntries); - db.HashFieldExpire("user:user1", ["Title"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user1", MemberExpiry.Pending(), "FIELDS", "1", "Title"); var field = db.HashRandomFields(hashKey, 10).Select(x => (string)x).ToArray(); ClassicAssert.AreEqual(field.Length, 1); ClassicAssert.AreEqual("Title", field[0]); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user1", deadline, "FIELDS", "1", "Title"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); field = db.HashRandomFields(hashKey, 10).Select(x => (string)x).ToArray(); ClassicAssert.AreEqual(field.Length, 0); @@ -721,13 +789,18 @@ public async Task CanDoHashScanWithExpire() var db = redis.GetDatabase(0); db.HashSet("user:user789", [new HashEntry("email", "email@example.com"), new HashEntry("email1", "email1@example.com"), new HashEntry("email2", "email2@example.com"), new HashEntry("email3", "email3@example.com"), new HashEntry("age", "25")]); - db.HashFieldExpire("user:user789", ["email"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user789", MemberExpiry.Pending(), "FIELDS", "1", "email"); var members = db.HashScan("user:user789", "email*"); ClassicAssert.IsTrue(((IScanningCursor)members).Cursor == 0); ClassicAssert.IsTrue(members.Count() == 4, "HSCAN with MATCH failed."); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user789", deadline, "FIELDS", "1", "email"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); // HSCAN with match members = db.HashScan("user:user789", "email*"); @@ -824,13 +897,18 @@ public async Task CanDoHMGETWithExpire() var db = redis.GetDatabase(0); db.HashSet("user:user789", [new HashEntry("email", "email@example.com"), new HashEntry("email1", "email1@example.com"), new HashEntry("email2", "email2@example.com"), new HashEntry("email3", "email3@example.com"), new HashEntry("age", "25")]); - db.HashFieldExpire("user:user789", ["email"], TimeSpan.FromMilliseconds(100)); + + // Give the field a pending expiration that is not reached while the test runs + db.Execute("HPEXPIREAT", "user:user789", MemberExpiry.Pending(), "FIELDS", "1", "email"); var members = (string[])db.Execute("HMGET", "user:user789", "email", "email1"); ClassicAssert.AreEqual("email@example.com", members[0]); ClassicAssert.AreEqual("email1@example.com", members[1]); - await Task.Delay(200).ConfigureAwait(false); + // Expire the field + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "user:user789", deadline, "FIELDS", "1", "email"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); members = (string[])db.Execute("HMGET", "user:user789", "email", "email1"); ClassicAssert.IsNull(members[0]); @@ -1026,26 +1104,31 @@ public async Task CanDoHashExpire() var db = redis.GetDatabase(0); db.HashSet("myhash", [new HashEntry("field1", "hello"), new HashEntry("field2", "world"), new HashEntry("field3", "value3"), new HashEntry("field4", "value4"), new HashEntry("field5", "value5"), new HashEntry("field6", "value6")]); - var result = db.Execute("HEXPIRE", "myhash", "4", "FIELDS", "3", "field1", "field5", "nonexistfield"); + var pendingSeconds = MemberExpiry.PendingTtlMs / 1000; + var pendingDeadlineMs = MemberExpiry.Pending(); + var pendingDeadlineSeconds = pendingDeadlineMs / 1000; + + // Pending expirations that are not reached while the test runs + var result = db.Execute("HEXPIRE", "myhash", pendingSeconds, "FIELDS", "3", "field1", "field5", "nonexistfield"); var results = (RedisResult[])result; ClassicAssert.AreEqual(3, results.Length); ClassicAssert.AreEqual(1, (long)results[0]); ClassicAssert.AreEqual(1, (long)results[1]); ClassicAssert.AreEqual(-2, (long)results[2]); - result = db.Execute("HPEXPIRE", "myhash", "4000", "FIELDS", "2", "field2", "nonexistfield"); + result = db.Execute("HPEXPIRE", "myhash", MemberExpiry.PendingTtlMs, "FIELDS", "2", "field2", "nonexistfield"); results = (RedisResult[])result; ClassicAssert.AreEqual(2, results.Length); ClassicAssert.AreEqual(1, (long)results[0]); ClassicAssert.AreEqual(-2, (long)results[1]); - result = db.Execute("HEXPIREAT", "myhash", DateTimeOffset.UtcNow.AddSeconds(4).ToUnixTimeSeconds().ToString(), "FIELDS", "2", "field3", "nonexistfield"); + result = db.Execute("HEXPIREAT", "myhash", pendingDeadlineSeconds, "FIELDS", "2", "field3", "nonexistfield"); results = (RedisResult[])result; ClassicAssert.AreEqual(2, results.Length); ClassicAssert.AreEqual(1, (long)results[0]); ClassicAssert.AreEqual(-2, (long)results[1]); - result = db.Execute("HPEXPIREAT", "myhash", DateTimeOffset.UtcNow.AddSeconds(4).ToUnixTimeMilliseconds().ToString(), "FIELDS", "2", "field4", "nonexistfield"); + result = db.Execute("HPEXPIREAT", "myhash", pendingDeadlineMs, "FIELDS", "2", "field4", "nonexistfield"); results = (RedisResult[])result; ClassicAssert.AreEqual(2, results.Length); ClassicAssert.AreEqual(1, (long)results[0]); @@ -1053,26 +1136,25 @@ public async Task CanDoHashExpire() var ttl = (RedisResult[])db.Execute("HTTL", "myhash", "FIELDS", "2", "field1", "nonexistfield"); ClassicAssert.AreEqual(2, ttl.Length); - ClassicAssert.LessOrEqual((long)ttl[0], 4); - ClassicAssert.Greater((long)ttl[0], 1); + ClassicAssert.LessOrEqual((long)ttl[0], pendingSeconds); + ClassicAssert.Greater((long)ttl[0], 0); ClassicAssert.AreEqual(-2, (long)results[1]); ttl = (RedisResult[])db.Execute("HPTTL", "myhash", "FIELDS", "2", "field1", "nonexistfield"); ClassicAssert.AreEqual(2, ttl.Length); - ClassicAssert.LessOrEqual((long)ttl[0], 4000); - ClassicAssert.Greater((long)ttl[0], 1000); + ClassicAssert.LessOrEqual((long)ttl[0], MemberExpiry.PendingTtlMs); + ClassicAssert.Greater((long)ttl[0], 0); ClassicAssert.AreEqual(-2, (long)results[1]); - ttl = (RedisResult[])db.Execute("HEXPIRETIME", "myhash", "FIELDS", "2", "field1", "nonexistfield"); + // An absolute deadline is reported back exactly as it was set + ttl = (RedisResult[])db.Execute("HEXPIRETIME", "myhash", "FIELDS", "2", "field3", "nonexistfield"); ClassicAssert.AreEqual(2, ttl.Length); - ClassicAssert.LessOrEqual((long)ttl[0], DateTimeOffset.UtcNow.AddSeconds(4).ToUnixTimeSeconds()); - ClassicAssert.Greater((long)ttl[0], DateTimeOffset.UtcNow.AddSeconds(1).ToUnixTimeSeconds()); + ClassicAssert.AreEqual(pendingDeadlineSeconds, (long)ttl[0]); ClassicAssert.AreEqual(-2, (long)results[1]); - ttl = (RedisResult[])db.Execute("HPEXPIRETIME", "myhash", "FIELDS", "2", "field1", "nonexistfield"); + ttl = (RedisResult[])db.Execute("HPEXPIRETIME", "myhash", "FIELDS", "2", "field4", "nonexistfield"); ClassicAssert.AreEqual(2, ttl.Length); - ClassicAssert.LessOrEqual((long)ttl[0], DateTimeOffset.UtcNow.AddSeconds(4).ToUnixTimeMilliseconds()); - ClassicAssert.Greater((long)ttl[0], DateTimeOffset.UtcNow.AddSeconds(1).ToUnixTimeMilliseconds()); + ClassicAssert.AreEqual(pendingDeadlineMs, (long)ttl[0]); ClassicAssert.AreEqual(-2, (long)results[1]); results = (RedisResult[])db.Execute("HPERSIST", "myhash", "FIELDS", "3", "field5", "field6", "nonexistfield"); @@ -1081,7 +1163,10 @@ public async Task CanDoHashExpire() ClassicAssert.AreEqual(-1, (long)results[1]); // -1 if the field exists but has no associated expiration set. ClassicAssert.AreEqual(-2, (long)results[2]); - await Task.Delay(4500).ConfigureAwait(false); + // Expire the fields that still carry an expiration + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "myhash", deadline, "FIELDS", "4", "field1", "field2", "field3", "field4"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); var items = db.HashGetAll("myhash"); ClassicAssert.AreEqual(2, items.Length); @@ -1114,16 +1199,20 @@ public async Task CanDoHashExpireLTM() string[] smallExpireKeys = ["user:user0", "user:user1"]; string[] largeExpireKeys = ["user:user2", "user:user3"]; + // Expirations are absolute deadlines, so each phase is reached by waiting for a deadline to + // pass rather than by assuming a bound on the time that elapses between commands. + var deadline = MemberExpiry.Imminent(); foreach (var key in smallExpireKeys) { db.HashSet(key, [new HashEntry("Field1", "StringValue"), new HashEntry("Field2", "1")]); - db.Execute("HEXPIRE", key, "2", "FIELDS", "1", "Field1"); + db.Execute("HPEXPIREAT", key, deadline, "FIELDS", "1", "Field1"); } + // Pending expirations that are not reached while the test runs foreach (var key in largeExpireKeys) { db.HashSet(key, [new HashEntry("Field1", "StringValue"), new HashEntry("Field2", "1")]); - db.Execute("HEXPIRE", key, "4", "FIELDS", "1", "Field1"); + db.Execute("HPEXPIREAT", key, MemberExpiry.Pending(), "FIELDS", "1", "Field1"); } // Create LTM (larger than memory) DB by inserting 1000 keys @@ -1137,7 +1226,7 @@ public async Task CanDoHashExpireLTM() // Ensure data has spilled to disk ClassicAssert.Greater(info.HeadAddress, info.BeginAddress); - await Task.Delay(2000).ConfigureAwait(false); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); var result = db.HashExists(smallExpireKeys[0], "Field1"); ClassicAssert.IsFalse(result); @@ -1150,13 +1239,18 @@ public async Task CanDoHashExpireLTM() var ttl = db.HashFieldGetTimeToLive(largeExpireKeys[0], ["Field1"]); ClassicAssert.AreEqual(ttl.Length, 1); ClassicAssert.Greater(ttl[0], 0); - ClassicAssert.LessOrEqual(ttl[0], 2000); + ClassicAssert.LessOrEqual(ttl[0], MemberExpiry.PendingTtlMs); ttl = db.HashFieldGetTimeToLive(largeExpireKeys[1], ["Field1"]); ClassicAssert.AreEqual(ttl.Length, 1); ClassicAssert.Greater(ttl[0], 0); - ClassicAssert.LessOrEqual(ttl[0], 2000); + ClassicAssert.LessOrEqual(ttl[0], MemberExpiry.PendingTtlMs); - await Task.Delay(2000).ConfigureAwait(false); + // Expire the remaining fields. These hashes carry field expirations and have been evicted, so + // the expiration is applied by copy-updating the record read back from disk. + deadline = MemberExpiry.Imminent(); + foreach (var key in largeExpireKeys) + db.Execute("HPEXPIREAT", key, deadline, "FIELDS", "1", "Field1"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); result = db.HashExists(largeExpireKeys[0], "Field1"); ClassicAssert.IsFalse(result); @@ -1183,7 +1277,7 @@ public async Task CanDoHashExpireWithAofRecoveryAsync() HashEntry[] values2_1 = [new HashEntry("key2_1", "val2_1"), new HashEntry("key2_2", "val2_2")]; HashEntry[] values2_2 = [new HashEntry("key2_3", "val2_3"), new HashEntry("key2_4", "val2_4")]; - var expireTime = DateTimeOffset.UtcNow + TimeSpan.FromMinutes(1); + var expireTime = DateTimeOffset.FromUnixTimeMilliseconds(MemberExpiry.Pending()); using (var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig())) { @@ -1195,18 +1289,18 @@ public async Task CanDoHashExpireWithAofRecoveryAsync() // Set 1st hash set, add short expiry to 1 entry db.HashSet(key2, values2_1); - db.Execute("HEXPIRE", key2, 1, "FIELDS", 1, "key2_1"); + var deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", key2, deadline, "FIELDS", 1, "key2_1"); // Wait for short expiry to pass - Thread.Sleep(2000); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); // Add more entries to 1st and 2nd hash sets db.HashSet(key1, values1_2); db.HashSet(key2, values2_2); // Add longer expiry to entry in 2nd hash set - db.Execute("HPEXPIRE", key2, 15000, "FIELDS", 1, "key2_2"); - Thread.Sleep(2000); + db.Execute("HPEXPIREAT", key2, MemberExpiry.Pending(), "FIELDS", 1, "key2_2"); // Verify 1st hash set contains all added entries var recoveredValues = db.HashGetAll(key1); @@ -1228,7 +1322,7 @@ public async Task CanDoHashExpireWithAofRecoveryAsync() ClassicAssert.IsNotNull(recoveredValuesTtl); ClassicAssert.AreEqual(4, recoveredValuesTtl!.Length); ClassicAssert.AreEqual(-2, (long)recoveredValuesTtl[0]); - ClassicAssert.LessOrEqual((long)recoveredValuesTtl[1], 13); + ClassicAssert.LessOrEqual((long)recoveredValuesTtl[1], MemberExpiry.PendingTtlMs / 1000); ClassicAssert.Greater((long)recoveredValuesTtl[1], 0); ClassicAssert.AreEqual(-1, (long)recoveredValuesTtl[2]); ClassicAssert.AreEqual(-1, (long)recoveredValuesTtl[3]); @@ -1264,7 +1358,7 @@ public async Task CanDoHashExpireWithAofRecoveryAsync() ClassicAssert.IsNotNull(recoveredValuesTtl); ClassicAssert.AreEqual(4, recoveredValuesTtl!.Length); ClassicAssert.AreEqual(-2, (long)recoveredValuesTtl[0]); - ClassicAssert.Less((long)recoveredValuesTtl[1], 13000); + ClassicAssert.LessOrEqual((long)recoveredValuesTtl[1], MemberExpiry.PendingTtlMs); ClassicAssert.Greater((long)recoveredValuesTtl[1], 0); ClassicAssert.AreEqual(-1, (long)recoveredValuesTtl[2]); ClassicAssert.AreEqual(-1, (long)recoveredValuesTtl[3]); @@ -1291,13 +1385,18 @@ public async Task CanDoHashCollect() var server = redis.GetServers().First(); db.HashSet("myhash", [new HashEntry("field1", "hello"), new HashEntry("field2", "world"), new HashEntry("field3", "value3"), new HashEntry("field4", "value4"), new HashEntry("field5", "value5"), new HashEntry("field6", "value6")]); - var result = db.Execute("HPEXPIRE", "myhash", "500", "FIELDS", "2", "field1", "field2"); + // Set the pending expirations first: HEXPIRE-family commands purge already-expired fields, so no + // mutating command may run between arming field1/field2 and the HCOLLECT that must reclaim them. + var result = db.Execute("HPEXPIREAT", "myhash", MemberExpiry.Pending(), "FIELDS", "2", "field3", "field4"); var results = (RedisResult[])result; ClassicAssert.AreEqual(2, results.Length); ClassicAssert.AreEqual(1, (long)results[0]); ClassicAssert.AreEqual(1, (long)results[1]); - result = db.Execute("HPEXPIRE", "myhash", "1500", "FIELDS", "2", "field3", "field4"); + // Expire field1 and field2, leaving field3 and field4 with a pending expiration that is + // not reached while the test runs + var deadline = MemberExpiry.Imminent(); + result = db.Execute("HPEXPIREAT", "myhash", deadline, "FIELDS", "2", "field1", "field2"); results = (RedisResult[])result; ClassicAssert.AreEqual(2, results.Length); ClassicAssert.AreEqual(1, (long)results[0]); @@ -1305,7 +1404,7 @@ public async Task CanDoHashCollect() var orginalMemory = (long)db.Execute("MEMORY", "USAGE", "myhash"); - await Task.Delay(600).ConfigureAwait(false); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); var newMemory = (long)db.Execute("MEMORY", "USAGE", "myhash"); ClassicAssert.AreEqual(newMemory, orginalMemory); @@ -1315,9 +1414,13 @@ public async Task CanDoHashCollect() newMemory = (long)db.Execute("MEMORY", "USAGE", "myhash"); ClassicAssert.Less(newMemory, orginalMemory); - orginalMemory = newMemory; - await Task.Delay(1100).ConfigureAwait(false); + // Expire field3 and field4. Re-setting a deadline enqueues another expiration entry, so the + // baseline for the lazy-expiration comparison is taken after the command. + deadline = MemberExpiry.Imminent(); + db.Execute("HPEXPIREAT", "myhash", deadline, "FIELDS", "2", "field3", "field4"); + orginalMemory = (long)db.Execute("MEMORY", "USAGE", "myhash"); + await MemberExpiry.WaitUntilPastAsync(deadline).ConfigureAwait(false); newMemory = (long)db.Execute("MEMORY", "USAGE", "myhash"); ClassicAssert.AreEqual(newMemory, orginalMemory); @@ -1355,10 +1458,10 @@ public void CanDoHashExpireWithOptions(string command, string option) (var expireTimeField1, var expireTimeField3, var newExpireTimeField) = command switch { - "HEXPIRE" => ("2", "6", "4"), - "HPEXPIRE" => ("2000", "6000", "4000"), - "HEXPIREAT" => (DateTimeOffset.UtcNow.AddSeconds(2).ToUnixTimeSeconds().ToString(), DateTimeOffset.UtcNow.AddSeconds(6).ToUnixTimeSeconds().ToString(), DateTimeOffset.UtcNow.AddSeconds(4).ToUnixTimeSeconds().ToString()), - "HPEXPIREAT" => (DateTimeOffset.UtcNow.AddSeconds(2).ToUnixTimeMilliseconds().ToString(), DateTimeOffset.UtcNow.AddSeconds(6).ToUnixTimeMilliseconds().ToString(), DateTimeOffset.UtcNow.AddSeconds(4).ToUnixTimeMilliseconds().ToString()), + "HEXPIRE" => ("3600", "10800", "7200"), + "HPEXPIRE" => ("3600000", "10800000", "7200000"), + "HEXPIREAT" => (DateTimeOffset.UtcNow.AddHours(1).ToUnixTimeSeconds().ToString(), DateTimeOffset.UtcNow.AddHours(3).ToUnixTimeSeconds().ToString(), DateTimeOffset.UtcNow.AddHours(2).ToUnixTimeSeconds().ToString()), + "HPEXPIREAT" => (DateTimeOffset.UtcNow.AddHours(1).ToUnixTimeMilliseconds().ToString(), DateTimeOffset.UtcNow.AddHours(3).ToUnixTimeMilliseconds().ToString(), DateTimeOffset.UtcNow.AddHours(2).ToUnixTimeMilliseconds().ToString()), _ => throw new ArgumentException("Invalid command") }; diff --git a/test/standalone/Garnet.test.collections/RespSortedSetTests.cs b/test/standalone/Garnet.test.collections/RespSortedSetTests.cs index 44cc545e422..a836c072ec0 100644 --- a/test/standalone/Garnet.test.collections/RespSortedSetTests.cs +++ b/test/standalone/Garnet.test.collections/RespSortedSetTests.cs @@ -548,10 +548,10 @@ public void ZCountAndZCardWithExpiredAndExpiringItems() db.SortedSetAdd("key1", "d", 4); db.SortedSetAdd("key1", "e", 5); - // Set expiration for the minimum, maximum and middle items - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "3", "a", "e", "c"); - - Thread.Sleep(300); + // Expire the minimum, maximum and middle items + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "3", "a", "e", "c"); + MemberExpiry.WaitUntilPast(deadline); // ZCARD var count = db.SortedSetLength("key1"); @@ -2487,14 +2487,14 @@ public void ZDiffWithExpiredAndExpiringItems() db.SortedSetAdd("key2", "b", 2); db.SortedSetAdd("key2", "c", 3); - // Set expiration for some items in key1 - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "2", "a", "c"); - db.Execute("ZPEXPIRE", "key1", "500", "MEMBERS", "1", "d"); - - // Set expiration for matching items in key2 - db.Execute("ZPEXPIRE", "key2", "200", "MEMBERS", "1", "a"); + // Give "d" a pending expiration that is not reached while the test runs + db.Execute("ZPEXPIREAT", "key1", MemberExpiry.Pending(), "MEMBERS", "1", "d"); - Thread.Sleep(300); + // Expire some items in key1 and the matching item in key2 + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "2", "a", "c"); + db.Execute("ZPEXPIREAT", "key2", deadline, "MEMBERS", "1", "a"); + MemberExpiry.WaitUntilPast(deadline); // Perform ZDIFF var diff = db.SortedSetCombine(SetOperation.Difference, ["key1", "key2"]); @@ -2508,7 +2508,10 @@ public void ZDiffWithExpiredAndExpiringItems() ClassicAssert.AreEqual("e", diffWithScores[1].Element.ToString()); ClassicAssert.AreEqual(5, diffWithScores[1].Score); - Thread.Sleep(300); + // Expire "d" + deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "d"); + MemberExpiry.WaitUntilPast(deadline); // Perform ZDIFF again after more items have expired diff = db.SortedSetCombine(SetOperation.Difference, ["key1", "key2"]); @@ -2538,14 +2541,14 @@ public void ZDiffStoreWithExpiredAndExpiringItems() db.SortedSetAdd("key2", "b", 2); db.SortedSetAdd("key2", "c", 3); - // Set expiration for some items in key1 - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "2", "a", "c"); - db.Execute("ZPEXPIRE", "key1", "500", "MEMBERS", "1", "d"); + // Give "d" a pending expiration that is not reached while the test runs + db.Execute("ZPEXPIREAT", "key1", MemberExpiry.Pending(), "MEMBERS", "1", "d"); - // Set expiration for matching items in key2 - db.Execute("ZPEXPIRE", "key2", "200", "MEMBERS", "1", "a"); - - Thread.Sleep(300); + // Expire some items in key1 and the matching item in key2 + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "2", "a", "c"); + db.Execute("ZPEXPIREAT", "key2", deadline, "MEMBERS", "1", "a"); + MemberExpiry.WaitUntilPast(deadline); // Perform ZDIFFSTORE var diffStoreCount = db.SortedSetCombineAndStore(SetOperation.Difference, "key3", ["key1", "key2"]); @@ -2559,7 +2562,10 @@ public void ZDiffStoreWithExpiredAndExpiringItems() ClassicAssert.AreEqual("e", diffStoreResult[1].Element.ToString()); ClassicAssert.AreEqual(5, diffStoreResult[1].Score); - Thread.Sleep(300); + // Expire "d" + deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "d"); + MemberExpiry.WaitUntilPast(deadline); // Perform ZDIFFSTORE again after more items have expired diffStoreCount = db.SortedSetCombineAndStore(SetOperation.Difference, "key3", ["key1", "key2"]); @@ -2583,10 +2589,9 @@ public void ZIncrByWithExpiredAndExpiringItems() db.SortedSetAdd("key1", "b", 2); db.SortedSetAdd("key1", "c", 3); - // Set expiration for some items in key1 - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "1", "a"); - - Thread.Sleep(10); + // Give "a" a pending expiration that is not reached while the test runs + var pending = MemberExpiry.Pending(); + db.Execute("ZPEXPIREAT", "key1", pending, "MEMBERS", "1", "a"); // Try to increment the score of an expiring item var newScore = db.SortedSetIncrement("key1", "a", 5); @@ -2594,10 +2599,17 @@ public void ZIncrByWithExpiredAndExpiringItems() // Check the TTL of the expiring item var ttl = db.Execute("ZPTTL", "key1", "MEMBERS", "1", "a"); - ClassicAssert.LessOrEqual((long)ttl, 200); + ClassicAssert.LessOrEqual((long)ttl, MemberExpiry.PendingTtlMs); ClassicAssert.Greater((long)ttl, 0); - Thread.Sleep(200); + // The increment leaves the deadline of the item untouched + var expireTime = db.Execute("ZPEXPIRETIME", "key1", "MEMBERS", "1", "a"); + ClassicAssert.AreEqual(pending, (long)expireTime); + + // Expire "a" + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "a"); + MemberExpiry.WaitUntilPast(deadline); // Check the item has expired ttl = db.Execute("ZPTTL", "key1", "MEMBERS", "1", "a"); @@ -2629,21 +2641,30 @@ public void ZInterWithExpiredAndExpiringItems() db.SortedSetAdd("key2", "b", 2); db.SortedSetAdd("key2", "c", 3); - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "1", "c"); - db.Execute("ZPEXPIRE", "key1", "500", "MEMBERS", "1", "b"); - db.Execute("ZPEXPIRE", "key2", "200", "MEMBERS", "1", "a"); + // Give the items that must survive the first two phases a pending expiration that is not + // reached while the test runs + var pending = MemberExpiry.Pending(); + db.Execute("ZPEXPIREAT", "key1", pending, "MEMBERS", "2", "c", "b"); + db.Execute("ZPEXPIREAT", "key2", pending, "MEMBERS", "1", "a"); var inter = db.SortedSetCombine(SetOperation.Intersect, ["key1", "key2"]); ClassicAssert.AreEqual(3, inter.Length); - Thread.Sleep(300); + // Expire "c" in key1 and "a" in key2 + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "c"); + db.Execute("ZPEXPIREAT", "key2", deadline, "MEMBERS", "1", "a"); + MemberExpiry.WaitUntilPast(deadline); var interWithScores = db.SortedSetCombineWithScores(SetOperation.Intersect, ["key1", "key2"]); ClassicAssert.AreEqual(1, interWithScores.Length); // Only "b" should remain ClassicAssert.AreEqual("b", interWithScores[0].Element.ToString()); ClassicAssert.AreEqual(4, interWithScores[0].Score); // Sum of scores - Thread.Sleep(300); + // Expire "b" in key1 + deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "b"); + MemberExpiry.WaitUntilPast(deadline); inter = db.SortedSetCombine(SetOperation.Intersect, ["key1", "key2"]); ClassicAssert.AreEqual(0, inter.Length); @@ -2665,16 +2686,22 @@ public void ZInterCardWithExpiredAndExpiringItems() db.SortedSetAdd("key2", "b", 2); db.SortedSetAdd("key2", "c", 3); - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "1", "c"); - db.Execute("ZPEXPIRE", "key1", "500", "MEMBERS", "1", "b"); - db.Execute("ZPEXPIRE", "key2", "200", "MEMBERS", "1", "a"); + // Give "b" a pending expiration that is not reached while the test runs + db.Execute("ZPEXPIREAT", "key1", MemberExpiry.Pending(), "MEMBERS", "1", "b"); - Thread.Sleep(300); + // Expire "c" in key1 and "a" in key2 + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "c"); + db.Execute("ZPEXPIREAT", "key2", deadline, "MEMBERS", "1", "a"); + MemberExpiry.WaitUntilPast(deadline); var interCardCount = (long)db.Execute("ZINTERCARD", "2", "key1", "key2"); ClassicAssert.AreEqual(1, interCardCount); // Only "b" should remain - Thread.Sleep(300); + // Expire "b" in key1 + deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "b"); + MemberExpiry.WaitUntilPast(deadline); interCardCount = (long)db.Execute("ZINTERCARD", "2", "key1", "key2"); ClassicAssert.AreEqual(0, interCardCount); // No items should remain @@ -2696,11 +2723,14 @@ public void ZInterStoreWithExpiredAndExpiringItems() db.SortedSetAdd("key2", "b", 2); db.SortedSetAdd("key2", "c", 3); - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "1", "c"); - db.Execute("ZPEXPIRE", "key1", "500", "MEMBERS", "1", "b"); - db.Execute("ZPEXPIRE", "key2", "200", "MEMBERS", "1", "a"); + // Give "b" a pending expiration that is not reached while the test runs + db.Execute("ZPEXPIREAT", "key1", MemberExpiry.Pending(), "MEMBERS", "1", "b"); - Thread.Sleep(300); + // Expire "c" in key1 and "a" in key2 + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "c"); + db.Execute("ZPEXPIREAT", "key2", deadline, "MEMBERS", "1", "a"); + MemberExpiry.WaitUntilPast(deadline); var interStoreCount = db.SortedSetCombineAndStore(SetOperation.Intersect, "key3", ["key1", "key2"]); ClassicAssert.AreEqual(1, interStoreCount); // Only "b" should remain @@ -2710,7 +2740,10 @@ public void ZInterStoreWithExpiredAndExpiringItems() ClassicAssert.AreEqual("b", interStoreResult[0].Element.ToString()); ClassicAssert.AreEqual(4, interStoreResult[0].Score); // Sum of scores - Thread.Sleep(300); + // Expire "b" in key1 + deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "b"); + MemberExpiry.WaitUntilPast(deadline); interStoreCount = db.SortedSetCombineAndStore(SetOperation.Intersect, "key3", ["key1", "key2"]); ClassicAssert.AreEqual(0, interStoreCount); // No items should remain @@ -2731,10 +2764,13 @@ public void ZLexCountWithExpiredAndExpiringItems() db.SortedSetAdd("key1", "d", 4); db.SortedSetAdd("key1", "e", 5); - db.Execute("ZPEXPIRE", "key1", "500", "MEMBERS", "3", "a", "e", "c"); - db.Execute("ZPEXPIRE", "key1", "2000", "MEMBERS", "1", "b"); + // Give "b" a pending expiration that is not reached while the test runs + db.Execute("ZPEXPIREAT", "key1", MemberExpiry.Pending(), "MEMBERS", "1", "b"); - Thread.Sleep(1000); + // Expire "a", "e" and "c" + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "3", "a", "e", "c"); + MemberExpiry.WaitUntilPast(deadline); var lexCount = (int)db.Execute("ZLEXCOUNT", "key1", "-", "+"); // SortedSetLengthByValue will check - and + to [- and [+ ClassicAssert.AreEqual(2, lexCount); // Only "b" and "d" should remain @@ -2742,7 +2778,10 @@ public void ZLexCountWithExpiredAndExpiringItems() var lexCountRange = db.SortedSetLengthByValue("key1", "b", "d", Exclude.Stop); ClassicAssert.AreEqual(1, lexCountRange); // Only "b" should remain within the range - Thread.Sleep(1500); + // Expire "b" + deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "b"); + MemberExpiry.WaitUntilPast(deadline); lexCount = (int)db.Execute("ZLEXCOUNT", "key1", "-", "+"); ClassicAssert.AreEqual(1, lexCount); // Only "d" should remain @@ -3379,10 +3418,9 @@ public void ZScoreWithExpiredAndExpiringItems() db.SortedSetAdd("key1", "b", 2); db.SortedSetAdd("key1", "c", 3); - // Set expiration for some items in key1 - db.Execute("ZPEXPIRE", "key1", "200", "MEMBERS", "1", "a"); - - Thread.Sleep(10); + // Give "a" a pending expiration that is not reached while the test runs + var pending = MemberExpiry.Pending(); + db.Execute("ZPEXPIREAT", "key1", pending, "MEMBERS", "1", "a"); // Check the score of an expiring item var score = db.SortedSetScore("key1", "a"); @@ -3390,10 +3428,13 @@ public void ZScoreWithExpiredAndExpiringItems() // Check the TTL of the expiring item var ttl = db.Execute("ZPTTL", "key1", "MEMBERS", "1", "a"); - ClassicAssert.LessOrEqual((long)ttl, 200); + ClassicAssert.LessOrEqual((long)ttl, MemberExpiry.PendingTtlMs); ClassicAssert.Greater((long)ttl, 0); - Thread.Sleep(200); + // Expire "a" + var deadline = MemberExpiry.Imminent(); + db.Execute("ZPEXPIREAT", "key1", deadline, "MEMBERS", "1", "a"); + MemberExpiry.WaitUntilPast(deadline); // Check the item has expired ttl = db.Execute("ZPTTL", "key1", "MEMBERS", "1", "a"); diff --git a/test/standalone/Garnet.test.vectorset/ExceptionInjectionShutdownTests.cs b/test/standalone/Garnet.test.vectorset/ExceptionInjectionShutdownTests.cs new file mode 100644 index 00000000000..98f847c3ce4 --- /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 the same way. + /// + [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 2d0c279514e..8529e777613 100644 --- a/test/standalone/Garnet.test.vectorset/RespVectorSetTests.cs +++ b/test/standalone/Garnet.test.vectorset/RespVectorSetTests.cs @@ -3402,6 +3402,11 @@ static void AssertExpectedHashSlot(GarnetServer server, string indexKey, ushort var store = server.Provider.StoreWrapper; var vectorManager = store.DefaultDatabase.VectorManager; + // A context released by a delete or an overwrite is only marked for cleanup on a background + // task, so it still reads as live until that task runs. Settle that work before sampling the + // slot, otherwise the released context is counted alongside the live one. + vectorManager.WaitForQuiescence(); + unsafe { fixed (byte* indexKeyPtr = Encoding.ASCII.GetBytes(indexKey)) @@ -4295,6 +4300,163 @@ public async Task VSETATTRAsync() ClassicAssert.IsNull(res8); } + /// + /// A Vector Set whose index record was flushed to the main log before the checkpoint was taken must + /// survive recovery: the checkpoint snapshot only reports records in its own range, so the index + /// record is recovered from the main log without ever being reported, and the recovered context must + /// not be mistaken for an abandoned one and swept by the cleanup task. + /// + [Test] + public async Task RecoverVectorSetFlushedBeforeCheckpointAsync() + { + // This test drives the log through a real flush, so it needs its own small-page server. + server.Dispose(deleteDir: true); + TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); + server = CreateSmallPageGarnetServer(tryRecover: false); + server.Start(); + server.Provider.StoreWrapper.DefaultDatabase.VectorManager.AllocateTestContexts(preAllocatedContexts); + + var vector = Enumerable.Range(0, 8).Select(static x => (byte)x).ToArray(); + var element = new byte[] { 0, 0, 0, 0 }; + + using (var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(allowAdmin: true))) + { + var db = redis.GetDatabase(0); + + ClassicAssert.IsTrue(db.StringSet("control", "controlvalue")); + _ = db.Execute("VADD", ["flushed", "XB8", vector, element, "CAS", "NOQUANT", "EF", "16", "M", "32"]); + + // Fill past the tail page so the product's own page-fill path moves the read-only boundary and + // flushes "flushed" to the main log well before the checkpoint below starts. + for (var i = 0; i < 4_000; i++) + { + ClassicAssert.IsTrue(db.StringSet($"filler:{i}", new string('x', 64))); + } + + var log = server.Provider.StoreWrapper.DefaultDatabase.Store.Log; + ClassicAssert.Greater(log.FlushedUntilAddress, log.BeginAddress, "the filler writes did not flush any of the log, so this test would not cover the flushed-before-checkpoint case"); + + _ = db.Execute("VADD", ["snapshotted", "XB8", vector, element, "CAS", "NOQUANT", "EF", "16", "M", "32"]); + +#pragma warning disable CS0618 // ForegroundSave is deprecated in the client, but is what this test needs + redis.GetServers()[0].Save(SaveType.ForegroundSave); +#pragma warning restore CS0618 + _ = await server.Store.WaitForCommitAsync(); + } + + server.Dispose(deleteDir: false); + server = CreateSmallPageGarnetServer(tryRecover: true); + server.Start(); + + using (var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig())) + { + var db = redis.GetDatabase(0); + + // Plain keys prove recovery itself worked, isolating any Vector Set loss below. + ClassicAssert.AreEqual("controlvalue", (string)db.StringGet("control")); + ClassicAssert.AreEqual(new string('x', 64), (string)db.StringGet("filler:0")); + + foreach (var key in new[] { "flushed", "snapshotted" }) + { + var embedding = (string[])db.Execute("VEMB", [key, element]); + ClassicAssert.AreEqual(8, embedding?.Length ?? 0, $"VEMB {key} did not return the recovered embedding"); + } + } + } + +#if DEBUG + /// + /// A ContextMetadata update that lands inside a checkpoint's fuzzy region is copied to the tail, so the + /// snapshot holds both the old and the new version of that record. Recovery must take the newest + /// version rather than treating the pair as corruption and aborting. + /// + [Test] + public async Task RecoverContextMetadataUpdatedInsideFuzzyRegionAsync() + { + TestUtils.IgnoreIfExceptionInjectionDisabled(); + + const ExceptionInjectionType MetadataPause = ExceptionInjectionType.VectorSet_Pause_Before_Context_Metadata_Rmw; + const ExceptionInjectionType CheckpointPause = ExceptionInjectionType.Checkpoint_Pause_At_Flush_Begin; + + var vector = Enumerable.Range(0, 8).Select(static x => (byte)x).ToArray(); + var element = new byte[] { 0, 0, 0, 0 }; + + using (var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(allowAdmin: true))) + { + var db = redis.GetDatabase(0); + + // First version of the ContextMetadata record. + _ = db.Execute("VADD", ["before", "XB8", vector, element, "CAS", "NOQUANT", "EF", "16", "M", "32"]); + + ExceptionInjectionHelper.EnableException(MetadataPause); + try + { + // Park a second VADD just before it persists its ContextMetadata update. + var vaddTask = Task.Run(() => + { + using var vaddRedis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + return vaddRedis.GetDatabase(0).Execute("VADD", ["during", "XB8", vector, element, "CAS", "NOQUANT", "EF", "16", "M", "32"]); + }); + await ExceptionInjectionHelper.WaitOnClearAsync(MetadataPause).WaitAsync(TimeSpan.FromSeconds(30)); + + // Park a checkpoint at the end of its fuzzy region, before it captures the last address the + // snapshot covers. + ExceptionInjectionHelper.EnableException(CheckpointPause); + try + { +#pragma warning disable CS0618 // ForegroundSave is deprecated in the client, but is what this test needs + var saveTask = Task.Run(() => redis.GetServers()[0].Save(SaveType.ForegroundSave)); +#pragma warning restore CS0618 + await ExceptionInjectionHelper.WaitOnClearAsync(CheckpointPause).WaitAsync(TimeSpan.FromSeconds(30)); + + // Release the metadata update into the new version, forcing it to be copied to the tail + // while the old version stays in the snapshot's range. + ExceptionInjectionHelper.EnableException(MetadataPause); + _ = await vaddTask.WaitAsync(TimeSpan.FromSeconds(30)); + ExceptionInjectionHelper.DisableException(MetadataPause); + + ExceptionInjectionHelper.EnableException(CheckpointPause); + await saveTask.WaitAsync(TimeSpan.FromSeconds(60)); + } + finally + { + ExceptionInjectionHelper.DisableException(CheckpointPause); + } + } + finally + { + ExceptionInjectionHelper.DisableException(MetadataPause); + } + + _ = await server.Store.WaitForCommitAsync(); + } + + server.Dispose(deleteDir: false); + + // failOnRecoveryError surfaces a recovery failure that the server would otherwise log and continue + // past, leaving a partially recovered store behind. + server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, enableAOF: true, tryRecover: true, aofMemorySize: DefaultAOFMemorySize, enableVectorSetPreview: true, failOnRecoveryError: true); + server.Start(); + + using (var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig())) + { + var db = redis.GetDatabase(0); + + foreach (var key in new[] { "before", "during" }) + { + var embedding = (string[])db.Execute("VEMB", [key, element]); + ClassicAssert.AreEqual(8, embedding?.Length ?? 0, $"VEMB {key} did not return the recovered embedding"); + } + } + } +#endif + + /// + /// Create a new GarnetServer instance with a small enough log that ordinary writes flush pages. + /// + private static GarnetServer CreateSmallPageGarnetServer(bool tryRecover) + => TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, enableAOF: true, tryRecover: tryRecover, aofMemorySize: DefaultAOFMemorySize, enableVectorSetPreview: true, lowMemory: true, memorySize: "64m", pageSize: "16k", failOnRecoveryError: true); + /// /// Create a new GarnetServer instance with common parameters. /// diff --git a/test/standalone/Garnet.test.vectorset/VectorSetMigratedContextRemapTests.cs b/test/standalone/Garnet.test.vectorset/VectorSetMigratedContextRemapTests.cs new file mode 100644 index 00000000000..6b146dc2064 --- /dev/null +++ b/test/standalone/Garnet.test.vectorset/VectorSetMigratedContextRemapTests.cs @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using Garnet.server; +using NUnit.Framework; +using NUnit.Framework.Legacy; + +namespace Garnet.test +{ + /// + /// Contexts are chosen independently by each node, so a context that was free on the PRIMARY which + /// picked it can be occupied on the node receiving the migration, and that migration is steered onto a + /// context that is free locally. The context it is steered onto gets marked migrating, which is also how + /// a migration that was interrupted and resumed looks, so a later incoming context whose own value + /// happens to equal that one must not read that mark as its own reservation and adopt it. + /// + [TestFixture] + public class VectorSetMigratedContextRemapTests : TestBase + { + private global::Garnet.GarnetServer server; + + [SetUp] + public void Setup() + { + TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); + + server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, enableVectorSetPreview: true); + server.Start(); + } + + [TearDown] + public void TearDown() + { + server?.Dispose(); + server = null; + + TestUtils.DeleteDirectory(TestUtils.MethodTestDir); + TestUtils.OnTearDown(); + } + + [Test] + public void RemappedMigrationTargetIsNotAdoptedByALaterMigration() + { + var vectorManager = server.Provider.StoreWrapper.DefaultDatabase.VectorManager; + + // Occupy the low contexts, so the first incoming context cannot be adopted as-is + vectorManager.AllocateTestContexts(4); + + var occupiedLocally = VectorManager.ContextStep; + + var firstLocal = vectorManager.ResolveMigratedContextForTest(occupiedLocally); + ClassicAssert.AreNotEqual(occupiedLocally, firstLocal, "A context that is in use locally must be steered elsewhere"); + + // The PRIMARY assigns contexts independently, so the value the first migration was steered onto + // can equal the value a second, unrelated migration arrives with + var secondLocal = vectorManager.ResolveMigratedContextForTest(firstLocal); + + ClassicAssert.AreNotEqual(firstLocal, secondLocal, "Two migrated Vector Sets were mapped onto a single context"); + + // Resolution is stable, so replaying either migration's records keeps landing in the same place + ClassicAssert.AreEqual(firstLocal, vectorManager.ResolveMigratedContextForTest(occupiedLocally)); + ClassicAssert.AreEqual(secondLocal, vectorManager.ResolveMigratedContextForTest(firstLocal)); + } + } +} \ No newline at end of file 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..2a19b19b45c --- /dev/null +++ b/test/standalone/Garnet.test.vectorset/VectorSetRecoveredContextReservationTests.cs @@ -0,0 +1,206 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Garnet.server; +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"); + } + } + + /// + /// A cluster node with AOF enabled reconciles twice while starting up, once from + /// RecoverCheckpointAndAOFAsync and again from StoreWrapper, and the first pass consumes the + /// recovered metadata. The second pass therefore rebuilds the reservations from the index records + /// alone, and a context that is in use without an index record - a Vector Set that was deleted but + /// whose data has not finished being cleaned up - must still be marked for cleanup rather than + /// silently dropped, or it is handed to the next Vector Set while its old data is still present. + /// + [Test] + public async Task ReconcileWithoutRecoveredMetadataStillMarksUnbackedContextsAsync() + { + const string Live = "live-vs"; + const int Elements = 50; + const int Dim = 32; + const int UnbackedContexts = 4; + + StartServer(tryRecover: false); + + var vectorManager = server.Provider.StoreWrapper.DefaultDatabase.VectorManager; + + // Reservations with no index record of their own, which is what a Vector Set whose data is still + // being cleaned up looks like + vectorManager.AllocateTestContexts(UnbackedContexts); + + 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(Live, Dim, i, $"element_{i}"))); + } + } + + var unbacked = new List(); + for (var context = VectorManager.ContextStep; context <= UnbackedContexts * VectorManager.ContextStep; context += VectorManager.ContextStep) + { + vectorManager.GetContextState(context, out var isInUse, out _, out _); + if (isInUse) + { + unbacked.Add(context); + } + } + + ClassicAssert.AreEqual(UnbackedContexts, unbacked.Count, "expected every allocated context to be reserved"); + + // Hold cleanup so the reconcile's decision stays observable instead of being processed away + await vectorManager.PauseCleanupAsync(); + try + { + // recoveredMetadata is empty here, exactly as it is on the second reconcile of a startup + vectorManager.ReconcileRecoveredState(); + + foreach (var context in unbacked) + { + vectorManager.GetContextState(context, out var isInUse, out var isCleaningUp, out _); + + ClassicAssert.IsTrue( + isInUse && isCleaningUp, + $"context {context} was dropped rather than marked for cleanup, so it can be reused while its data is still present"); + } + } + finally + { + vectorManager.ResumeCleanup(); + } + } + + 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/RespTests.cs b/test/standalone/Garnet.test/RespTests.cs index ed1b2c60576..8fd815aafab 100644 --- a/test/standalone/Garnet.test/RespTests.cs +++ b/test/standalone/Garnet.test/RespTests.cs @@ -4657,12 +4657,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); } @@ -5551,18 +5564,23 @@ public async Task MultipleClientsUnblockAndAddTest(int numberOfItems) // Wait for client to enter blocking state await Task.Delay(1000).ConfigureAwait(false); - // Start parallel unblock and add tasks - var unblockTasks = new List>(); + // Start parallel unblock and add tasks. + // These use the asynchronous StackExchange.Redis APIs deliberately: the synchronous overloads block a + // thread pool thread for the duration of the round trip, and issuing numberOfItems + 3 of them at once + // exhausts the pool on a 2-core machine. The pool then injects replacement threads at roughly one per + // second, so the commands reach the server several seconds late - after the blocking command has already + // timed out - and the concurrency this test exists to exercise never actually happens. + var unblockTasks = new List>(); var addTasks = new List(); for (int i = 0; i < numberOfItems; i++) { var _i = i; - addTasks.Add(Task.Run(() => redis.GetDatabase(0).ListLeftPush(key, $"{value}{_i}"))); + addTasks.Add(db.ListLeftPushAsync(key, $"{value}{_i}")); } for (int i = 0; i < 3; i++) { - unblockTasks.Add(Task.Run(() => (int)redis.GetDatabase(0).Execute("CLIENT", "UNBLOCK", clientId, "ERROR"))); + unblockTasks.Add(db.ExecuteAsync("CLIENT", "UNBLOCK", clientId, "ERROR")); } await Task.WhenAll(unblockTasks).ConfigureAwait(false); @@ -5577,11 +5595,11 @@ public async Task MultipleClientsUnblockAndAddTest(int numberOfItems) if (numberOfItemsReturned == 0) { ClassicAssert.IsTrue(blockingResult.StartsWith("-UNBLOCKED")); - ClassicAssert.IsTrue(unblockTasks.Any(x => x.Result == 1)); + ClassicAssert.IsTrue(unblockTasks.Any(x => (int)x.Result == 1)); } else { - ClassicAssert.IsTrue(unblockTasks.All(x => x.Result == 0)); + ClassicAssert.IsTrue(unblockTasks.All(x => (int)x.Result == 0)); } } diff --git a/test/standalone/Garnet.test/RespTlsTests.cs b/test/standalone/Garnet.test/RespTlsTests.cs index 87d3ce31b61..bbe226e8a62 100644 --- a/test/standalone/Garnet.test/RespTlsTests.cs +++ b/test/standalone/Garnet.test/RespTlsTests.cs @@ -4,6 +4,9 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net; +using System.Net.Security; +using System.Net.Sockets; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -523,5 +526,31 @@ public void TlsCanSetGetCommandsChunks(int bytesSent) response = lightClientRequest.Execute($"MGET{sb}", expectedResponse.Length, bytesSent); ClassicAssert.AreEqual(expectedResponse, response); } + + [Test] + public async Task PeerThatNeverNegotiatesDoesNotBlockOtherConnections() + { + // A TLS handshake is driven by the peer and takes several network round trips. If the server + // completes it on the accept loop, a peer that connects without negotiating holds that loop and + // no other connection is ever accepted. + var endpoint = new IPEndPoint(IPAddress.Loopback, TestUtils.TestPort); + + ClassicAssert.IsTrue(await TryHandshakeAsync(endpoint), "TLS handshake failed before opening the silent connection"); + + using var silent = new TcpClient(); + await silent.ConnectAsync(endpoint); + + ClassicAssert.IsTrue(await TryHandshakeAsync(endpoint), "TLS handshake did not complete while a peer that never negotiates was connected"); + } + + static async Task TryHandshakeAsync(IPEndPoint endpoint) + { + using var client = new TcpClient(); + await client.ConnectAsync(endpoint); + using var sslStream = new SslStream(client.GetStream(), leaveInnerStreamOpen: false, (_, _, _, _) => true); + var handshake = sslStream.AuthenticateAsClientAsync("GarnetTest"); + return await Task.WhenAny(handshake, Task.Delay(TimeSpan.FromSeconds(15))).ConfigureAwait(false) == handshake + && handshake.IsCompletedSuccessfully; + } } } \ No newline at end of file diff --git a/test/standalone/Garnet.test/TestUtils.cs b/test/standalone/Garnet.test/TestUtils.cs index 203c3ea7a4a..d1c2f649aba 100644 --- a/test/standalone/Garnet.test/TestUtils.cs +++ b/test/standalone/Garnet.test/TestUtils.cs @@ -146,6 +146,22 @@ internal static string AzureTestContainer public const string certFile = "testcert.pfx"; public const string certPassword = "placeholder"; + static X509Certificate2 clientCertificate; + static readonly object clientCertificateLock = new(); + + /// + /// Returns the client certificate used by TLS-enabled tests, importing it from disk on first use. + /// Importing a PKCS#12 file is expensive - on Windows it materializes a key container - and the + /// options factory a client calls runs once per connection attempt, so importing per connection + /// spends that cost inside the client's connect timeout on every attempt and retry. + /// + public static X509Certificate2 GetClientCertificate() + { + if (clientCertificate is not null) return clientCertificate; + lock (clientCertificateLock) + return clientCertificate ??= CertificateUtils.GetMachineCertificateByFile(certFile, certPassword); + } + public const string pemCertFile = "testcert.pem"; public const string pemCertKeyFile = "testcert.key.pem"; @@ -858,7 +874,7 @@ public static GarnetServerOptions GetGarnetServerOptions( tlsServerOptionsOverride: null, clusterTlsClientOptionsOverride: new SslClientAuthenticationOptions { - ClientCertificates = certificates ?? [CertificateUtils.GetMachineCertificateByFile(certFile, certPassword)], + ClientCertificates = certificates ?? [GetClientCertificate()], TargetHost = "GarnetTest", AllowRenegotiation = false, RemoteCertificateValidationCallback = ValidateServerCertificate, @@ -968,7 +984,10 @@ public static ConfigurationOptions GetConfig( SyncTimeout = (int)TimeSpan.FromSeconds(30).TotalMilliseconds, AsyncTimeout = (int)TimeSpan.FromSeconds(30).TotalMilliseconds, AllowAdmin = allowAdmin, - ReconnectRetryPolicy = new LinearRetry((int)TimeSpan.FromSeconds(10).TotalMilliseconds), + // Gates how often the multiplexer may retry a dropped connection. Tests restart nodes + // routinely, and the first command issued afterwards blocks until the next retry is + // allowed, so a long interval is dead time added to every such test. + ReconnectRetryPolicy = new LinearRetry((int)TimeSpan.FromMilliseconds(250).TotalMilliseconds), ConnectRetry = 5, IncludeDetailInExceptions = true, AbortOnConnectFail = true, @@ -992,7 +1011,7 @@ public static ConfigurationOptions GetConfig( ( new SslClientAuthenticationOptions { - ClientCertificates = certificates ?? [CertificateUtils.GetMachineCertificateByFile(certFile, certPassword)], + ClientCertificates = certificates ?? [GetClientCertificate()], TargetHost = "GarnetTest", AllowRenegotiation = false, RemoteCertificateValidationCallback = ValidateServerCertificate, @@ -1009,7 +1028,7 @@ public static GarnetClient GetGarnetClient(EndPoint endpoint = null, bool useTLS { sslOptions = new SslClientAuthenticationOptions { - ClientCertificates = [CertificateUtils.GetMachineCertificateByFile(certFile, certPassword)], + ClientCertificates = [GetClientCertificate()], TargetHost = "GarnetTest", AllowRenegotiation = false, RemoteCertificateValidationCallback = ValidateServerCertificate, @@ -1025,7 +1044,7 @@ public static GarnetClientSession GetGarnetClientSession(bool useTLS = false, bo { sslOptions = new SslClientAuthenticationOptions { - ClientCertificates = [CertificateUtils.GetMachineCertificateByFile(certFile, certPassword)], + ClientCertificates = [GetClientCertificate()], TargetHost = "GarnetTest", AllowRenegotiation = false, RemoteCertificateValidationCallback = ValidateServerCertificate, @@ -1041,7 +1060,7 @@ public static LightClientRequest CreateRequest(LightClient.OnResponseDelegateUns { sslOptions = new SslClientAuthenticationOptions { - ClientCertificates = [CertificateUtils.GetMachineCertificateByFile(certFile, certPassword)], + ClientCertificates = [GetClientCertificate()], TargetHost = "GarnetTest", AllowRenegotiation = false, RemoteCertificateValidationCallback = ValidateServerCertificate,