From d995b1139de0546df7322a5b072018c6d236b71e Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Wed, 8 Jan 2025 17:02:23 -0800 Subject: [PATCH 01/31] skip Interlocked.Exchange and FlushConfig when no update detected on merge --- libs/cluster/Server/ClusterConfig.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/cluster/Server/ClusterConfig.cs b/libs/cluster/Server/ClusterConfig.cs index c45320b0ce..6e3cf0fba0 100644 --- a/libs/cluster/Server/ClusterConfig.cs +++ b/libs/cluster/Server/ClusterConfig.cs @@ -998,6 +998,9 @@ public ClusterConfig MergeSlotMap(ClusterConfig senderConfig, ILogger logger = n // Update ownership of node newSlotMap[i]._workerId = senderWorkerId; newSlotMap[i]._state = SlotState.STABLE; + + // Update happened, need to merge and FlushConfig + updated = true; } return updated ? new(newSlotMap, workers) : this; From 4afc0db2f0f6e4efdb9e832e8cc985cf7b0f3e80 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Wed, 8 Jan 2025 17:54:10 -0800 Subject: [PATCH 02/31] special case of zero epoch necessitates tracking of explicit slot update --- libs/cluster/Server/ClusterConfig.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/libs/cluster/Server/ClusterConfig.cs b/libs/cluster/Server/ClusterConfig.cs index 6e3cf0fba0..c45320b0ce 100644 --- a/libs/cluster/Server/ClusterConfig.cs +++ b/libs/cluster/Server/ClusterConfig.cs @@ -998,9 +998,6 @@ public ClusterConfig MergeSlotMap(ClusterConfig senderConfig, ILogger logger = n // Update ownership of node newSlotMap[i]._workerId = senderWorkerId; newSlotMap[i]._state = SlotState.STABLE; - - // Update happened, need to merge and FlushConfig - updated = true; } return updated ? new(newSlotMap, workers) : this; From 86cf1d4910cca005d350cd27c9303c547950ec38 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 26 Nov 2024 16:39:01 -0800 Subject: [PATCH 03/31] move key spec extraction to clusterProvider --- libs/cluster/Server/ClusterProvider.cs | 67 +++++++++++++++++++ libs/server/Cluster/IClusterProvider.cs | 10 ++- .../Resp/RespServerSessionSlotVerify.cs | 64 +----------------- 3 files changed, 77 insertions(+), 64 deletions(-) diff --git a/libs/cluster/Server/ClusterProvider.cs b/libs/cluster/Server/ClusterProvider.cs index b1e9abc1ee..398f3bc93e 100644 --- a/libs/cluster/Server/ClusterProvider.cs +++ b/libs/cluster/Server/ClusterProvider.cs @@ -288,6 +288,73 @@ public void PurgeBufferPool(ManagerType managerType) throw new GarnetException(); } + public void ExtractKeySpecs(RespCommandsInfo commandInfo, RespCommand cmd, ref SessionParseState parseState, ref ClusterSlotVerificationInput csvi) + { + var specs = commandInfo.KeySpecifications; + switch (specs.Length) + { + case 1: + var searchIndex = (BeginSearchIndex)specs[0].BeginSearch; + csvi.readOnly = specs[0].Flags.HasFlag(KeySpecificationFlags.RO); + switch (specs[0].FindKeys) + { + case FindKeysRange: + var findRange = (FindKeysRange)specs[0].FindKeys; + csvi.firstKey = searchIndex.Index - 1; + csvi.lastKey = findRange.LastKey < 0 ? findRange.LastKey + parseState.Count + 1 : findRange.LastKey - searchIndex.Index + 1; + csvi.step = findRange.KeyStep; + csvi.readOnly = !specs[0].Flags.HasFlag(KeySpecificationFlags.RW); + break; + case FindKeysKeyNum: + var findKeysKeyNum = (FindKeysKeyNum)specs[0].FindKeys; + csvi.firstKey = searchIndex.Index + findKeysKeyNum.FirstKey - 1; + csvi.lastKey = csvi.firstKey + parseState.GetInt(searchIndex.Index + findKeysKeyNum.KeyNumIdx - 1); + csvi.step = findKeysKeyNum.KeyStep; + break; + case FindKeysUnknown: + default: + throw new GarnetException("FindKeys spec not known"); + } + + break; + case 2: + searchIndex = (BeginSearchIndex)specs[0].BeginSearch; + switch (specs[0].FindKeys) + { + case FindKeysRange: + csvi.firstKey = RespCommand.BITOP == cmd ? searchIndex.Index - 2 : searchIndex.Index - 1; + break; + case FindKeysKeyNum: + case FindKeysUnknown: + default: + throw new GarnetException("FindKeys spec not known"); + } + + var searchIndex1 = (BeginSearchIndex)specs[1].BeginSearch; + switch (specs[1].FindKeys) + { + case FindKeysRange: + var findRange = (FindKeysRange)specs[1].FindKeys; + csvi.lastKey = findRange.LastKey < 0 ? findRange.LastKey + parseState.Count + 1 : findRange.LastKey + searchIndex1.Index - searchIndex.Index + 1; + csvi.step = findRange.KeyStep; + break; + case FindKeysKeyNum: + var findKeysKeyNum = (FindKeysKeyNum)specs[1].FindKeys; + csvi.keyNumOffset = searchIndex1.Index + findKeysKeyNum.KeyNumIdx - 1; + csvi.lastKey = searchIndex1.Index + parseState.GetInt(csvi.keyNumOffset); + csvi.step = findKeysKeyNum.KeyStep; + break; + case FindKeysUnknown: + default: + throw new GarnetException("FindKeys spec not known"); + } + + break; + default: + throw new GarnetException("KeySpecification not supported count"); + } + } + internal ReplicationLogCheckpointManager GetReplicationLogCheckpointManager(StoreType storeType) { Debug.Assert(serverOptions.EnableCluster); diff --git a/libs/server/Cluster/IClusterProvider.cs b/libs/server/Cluster/IClusterProvider.cs index 738a86c63c..bf664141e1 100644 --- a/libs/server/Cluster/IClusterProvider.cs +++ b/libs/server/Cluster/IClusterProvider.cs @@ -50,11 +50,19 @@ public interface IClusterProvider : IDisposable MetricsItem[] GetBufferPoolStats(); /// - /// Purger buffer pool for provided manager + /// Purge buffer pool for provided manager /// /// void PurgeBufferPool(ManagerType managerType); + /// + /// Extract key specs + /// + /// + /// + /// + void ExtractKeySpecs(RespCommandsInfo commandInfo, RespCommand cmd, ref SessionParseState parseState, ref ClusterSlotVerificationInput csvi); + /// /// Is replica /// diff --git a/libs/server/Resp/RespServerSessionSlotVerify.cs b/libs/server/Resp/RespServerSessionSlotVerify.cs index 323fb50be0..9de8ee1c18 100644 --- a/libs/server/Resp/RespServerSessionSlotVerify.cs +++ b/libs/server/Resp/RespServerSessionSlotVerify.cs @@ -3,7 +3,6 @@ using System; using System.Diagnostics; -using Garnet.common; namespace Garnet.server { @@ -41,68 +40,7 @@ bool CanServeSlot(RespCommand cmd) return true; csvi.keyNumOffset = -1; - var specs = commandInfo.KeySpecifications; - switch (specs.Length) - { - case 1: - var searchIndex = (BeginSearchIndex)specs[0].BeginSearch; - - switch (specs[0].FindKeys) - { - case FindKeysRange: - var findRange = (FindKeysRange)specs[0].FindKeys; - csvi.firstKey = searchIndex.Index - 1; - csvi.lastKey = findRange.LastKey < 0 ? findRange.LastKey + parseState.Count + 1 : findRange.LastKey - searchIndex.Index + 1; - csvi.step = findRange.KeyStep; - break; - case FindKeysKeyNum: - var findKeysKeyNum = (FindKeysKeyNum)specs[0].FindKeys; - csvi.firstKey = searchIndex.Index + findKeysKeyNum.FirstKey - 1; - csvi.lastKey = csvi.firstKey + parseState.GetInt(searchIndex.Index + findKeysKeyNum.KeyNumIdx - 1); - csvi.step = findKeysKeyNum.KeyStep; - break; - case FindKeysUnknown: - default: - throw new GarnetException("FindKeys spec not known"); - } - - break; - case 2: - searchIndex = (BeginSearchIndex)specs[0].BeginSearch; - switch (specs[0].FindKeys) - { - case FindKeysRange: - csvi.firstKey = RespCommand.BITOP == cmd ? searchIndex.Index - 2 : searchIndex.Index - 1; - break; - case FindKeysKeyNum: - case FindKeysUnknown: - default: - throw new GarnetException("FindKeys spec not known"); - } - - var searchIndex1 = (BeginSearchIndex)specs[1].BeginSearch; - switch (specs[1].FindKeys) - { - case FindKeysRange: - var findRange = (FindKeysRange)specs[1].FindKeys; - csvi.lastKey = findRange.LastKey < 0 ? findRange.LastKey + parseState.Count + 1 : findRange.LastKey + searchIndex1.Index - searchIndex.Index + 1; - csvi.step = findRange.KeyStep; - break; - case FindKeysKeyNum: - var findKeysKeyNum = (FindKeysKeyNum)specs[1].FindKeys; - csvi.keyNumOffset = searchIndex1.Index + findKeysKeyNum.KeyNumIdx - 1; - csvi.lastKey = searchIndex1.Index + parseState.GetInt(csvi.keyNumOffset); - csvi.step = findKeysKeyNum.KeyStep; - break; - case FindKeysUnknown: - default: - throw new GarnetException("FindKeys spec not known"); - } - - break; - default: - throw new GarnetException("KeySpecification not supported count"); - } + storeWrapper.clusterProvider.ExtractKeySpecs(commandInfo, cmd, ref parseState, ref csvi); csvi.readOnly = cmd.IsReadOnly(); csvi.sessionAsking = SessionAsking; return !clusterSession.NetworkMultiKeySlotVerify(ref parseState, ref csvi, ref dcurr, ref dend); From 957f03cba402b09eac18c083c120cb621d68c564 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 26 Nov 2024 16:40:35 -0800 Subject: [PATCH 04/31] implement CLUSTER PUBLISH api --- libs/cluster/Session/ClusterCommands.cs | 2 +- libs/cluster/Session/ClusterSession.cs | 9 +++++ .../Session/RespClusterBasicCommands.cs | 40 +++++++++++++++++++ libs/host/GarnetServer.cs | 2 +- libs/resources/RespCommandsInfo.json | 26 ++++++++++++ libs/server/AOF/AofProcessor.cs | 1 + libs/server/Resp/CmdStrings.cs | 1 + libs/server/Resp/Parser/RespCommand.cs | 5 +++ libs/server/StoreWrapper.cs | 8 ++++ .../CommandInfoUpdater/SupportedCommand.cs | 3 +- test/Garnet.test/Resp/ACL/RespCommandTests.cs | 29 ++++++++++++++ 11 files changed, 123 insertions(+), 3 deletions(-) diff --git a/libs/cluster/Session/ClusterCommands.cs b/libs/cluster/Session/ClusterCommands.cs index 33469ce790..e9e6d2c116 100644 --- a/libs/cluster/Session/ClusterCommands.cs +++ b/libs/cluster/Session/ClusterCommands.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Text; -using Garnet.common; using Garnet.server; namespace Garnet.cluster @@ -165,6 +164,7 @@ private void ProcessClusterCommands(RespCommand command, out bool invalidParamet RespCommand.CLUSTER_MYID => NetworkClusterMyId(out invalidParameters), RespCommand.CLUSTER_MYPARENTID => NetworkClusterMyParentId(out invalidParameters), RespCommand.CLUSTER_NODES => NetworkClusterNodes(out invalidParameters), + RespCommand.CLUSTER_PUBLISH => NetworkClusterPublish(out invalidParameters), RespCommand.CLUSTER_REPLICAS => NetworkClusterReplicas(out invalidParameters), RespCommand.CLUSTER_REPLICATE => NetworkClusterReplicate(out invalidParameters), RespCommand.CLUSTER_RESET => NetworkClusterReset(out invalidParameters), diff --git a/libs/cluster/Session/ClusterSession.cs b/libs/cluster/Session/ClusterSession.cs index b4e863e273..11253d644a 100644 --- a/libs/cluster/Session/ClusterSession.cs +++ b/libs/cluster/Session/ClusterSession.cs @@ -27,6 +27,7 @@ internal sealed unsafe partial class ClusterSession : IClusterSession BasicGarnetApi basicGarnetApi; readonly INetworkSender networkSender; readonly ILogger logger; + ClusterSlotVerificationInput csvi; // Authenticator used to validate permissions for cluster commands readonly IGarnetAuthenticator authenticator; @@ -74,6 +75,14 @@ public void ProcessClusterCommands(RespCommand command, ref SessionParseState pa { if (command.IsClusterSubCommand()) { + if (RespCommandsInfo.TryGetRespCommandInfo(command, out var commandInfo) && commandInfo.KeySpecifications != null) + { + csvi.keyNumOffset = -1; + clusterProvider.ExtractKeySpecs(commandInfo, command, ref parseState, ref csvi); + if (NetworkMultiKeySlotVerify(ref parseState, ref csvi, ref this.dcurr, ref this.dend)) + return; + } + ProcessClusterCommands(command, out invalidParameters); if (invalidParameters) diff --git a/libs/cluster/Session/RespClusterBasicCommands.cs b/libs/cluster/Session/RespClusterBasicCommands.cs index c364543d8c..387c3eebd1 100644 --- a/libs/cluster/Session/RespClusterBasicCommands.cs +++ b/libs/cluster/Session/RespClusterBasicCommands.cs @@ -450,5 +450,45 @@ private bool NetworkClusterReset(out bool invalidParameters) return true; } + + /// + /// Implement CLUSTER PUBLISH command + /// + /// + /// + private bool NetworkClusterPublish(out bool invalidParameters) + { + invalidParameters = false; + + // CLUSTER PUBLISH|SPUBLISH channel message + // Expecting exactly 2 arguments + if (parseState.Count != 2) + { + invalidParameters = true; + return true; + } + + if (clusterProvider.storeWrapper.subscribeBroker == null) + { + while (!RespWriteUtils.WriteError("ERR PUBLISH is disabled, enable it with --pubsub option."u8, ref dcurr, dend)) + SendAndReset(); + return true; + } + + var key = parseState.GetArgSliceByRef(0).SpanByte; + var val = parseState.GetArgSliceByRef(1).SpanByte; + + var keyPtr = key.ToPointer() - sizeof(int); + var valPtr = val.ToPointer() - sizeof(int); + var kSize = key.Length; + var vSize = val.Length; + + var numClients = clusterProvider.storeWrapper.subscribeBroker.PublishNow(keyPtr, valPtr, vSize + sizeof(int), true); + + while (!RespWriteUtils.WriteInteger(numClients, ref dcurr, dend)) + SendAndReset(); + + return true; + } } } \ No newline at end of file diff --git a/libs/host/GarnetServer.cs b/libs/host/GarnetServer.cs index c4af9b8dfe..973399ad80 100644 --- a/libs/host/GarnetServer.cs +++ b/libs/host/GarnetServer.cs @@ -246,7 +246,7 @@ private void InitializeServer() this.server ??= new GarnetServerTcp(opts.Address, opts.Port, 0, opts.TlsOptions, opts.NetworkSendThrottleMax, opts.NetworkConnectionLimit, logger); storeWrapper = new StoreWrapper(version, redisProtocolVersion, server, store, objectStore, objectStoreSizeTracker, - customCommandManager, appendOnlyFile, opts, clusterFactory: clusterFactory, loggerFactory: loggerFactory); + customCommandManager, appendOnlyFile, opts, subscribeBroker, clusterFactory: clusterFactory, loggerFactory: loggerFactory); // Create session provider for Garnet Provider = new GarnetProvider(storeWrapper, subscribeBroker); diff --git a/libs/resources/RespCommandsInfo.json b/libs/resources/RespCommandsInfo.json index 1181f264f6..196aeccd2d 100644 --- a/libs/resources/RespCommandsInfo.json +++ b/libs/resources/RespCommandsInfo.json @@ -610,6 +610,32 @@ "Flags": "Admin, NoMulti, NoScript", "AclCategories": "Admin, Dangerous, Slow, Garnet" }, + { + "Command": "CLUSTER_PUBLISH", + "Name": "CLUSTER|PUBLISH", + "IsInternal": true, + "Arity": 4, + "Flags": "Loading, NoScript, PubSub, Stale", + "FirstKey": 1, + "LastKey": 1, + "Step": 1, + "AclCategories": "Admin, PubSub, Slow, Garnet, Read", + "KeySpecifications": [ + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 1 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": 0, + "KeyStep": 1, + "Limit": 0 + }, + "Flags": "RO" + } + ] + }, { "Command": "CLUSTER_BUMPEPOCH", "Name": "CLUSTER|BUMPEPOCH", diff --git a/libs/server/AOF/AofProcessor.cs b/libs/server/AOF/AofProcessor.cs index 2a916e3689..aed7b5d27f 100644 --- a/libs/server/AOF/AofProcessor.cs +++ b/libs/server/AOF/AofProcessor.cs @@ -78,6 +78,7 @@ public AofProcessor( storeWrapper.customCommandManager, recordToAof ? storeWrapper.appendOnlyFile : null, storeWrapper.serverOptions, + storeWrapper.subscribeBroker, accessControlList: storeWrapper.accessControlList, loggerFactory: storeWrapper.loggerFactory); diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index 9df9bc429d..6304d93565 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -350,6 +350,7 @@ static partial class CmdStrings public static ReadOnlySpan delkeysinslotrange => "DELKEYSINSLOTRANGE"u8; public static ReadOnlySpan setslotsrange => "SETSLOTSRANGE"u8; public static ReadOnlySpan slotstate => "SLOTSTATE"u8; + public static ReadOnlySpan publish => "PUBLISH"u8; public static ReadOnlySpan mtasks => "MTASKS"u8; public static ReadOnlySpan aofsync => "AOFSYNC"u8; public static ReadOnlySpan appendlog => "APPENDLOG"u8; diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index d9b50c9716..6e62c56cb5 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -331,6 +331,7 @@ public enum RespCommand : ushort CLUSTER_MYID, CLUSTER_MYPARENTID, CLUSTER_NODES, + CLUSTER_PUBLISH, CLUSTER_REPLICAS, CLUSTER_REPLICATE, CLUSTER_RESET, @@ -1945,6 +1946,10 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.CLUSTER_SLOTSTATE; } + else if (subCommand.SequenceEqual(CmdStrings.publish)) + { + return RespCommand.CLUSTER_PUBLISH; + } else if (subCommand.SequenceEqual(CmdStrings.MIGRATE)) { return RespCommand.CLUSTER_MIGRATE; diff --git a/libs/server/StoreWrapper.cs b/libs/server/StoreWrapper.cs index 6da4f7fca5..0bef69a596 100644 --- a/libs/server/StoreWrapper.cs +++ b/libs/server/StoreWrapper.cs @@ -47,6 +47,12 @@ public sealed class StoreWrapper /// Server options /// public readonly GarnetServerOptions serverOptions; + + /// + /// Subscribe broker + /// + public readonly SubscribeBroker> subscribeBroker; + internal readonly IClusterProvider clusterProvider; /// @@ -124,6 +130,7 @@ public StoreWrapper( CustomCommandManager customCommandManager, TsavoriteLog appendOnlyFile, GarnetServerOptions serverOptions, + SubscribeBroker> subscribeBroker, AccessControlList accessControlList = null, IClusterFactory clusterFactory = null, ILoggerFactory loggerFactory = null @@ -137,6 +144,7 @@ public StoreWrapper( this.objectStore = objectStore; this.appendOnlyFile = appendOnlyFile; this.serverOptions = serverOptions; + this.subscribeBroker = subscribeBroker; lastSaveTime = DateTimeOffset.FromUnixTimeSeconds(0); this.customCommandManager = customCommandManager; this.monitor = serverOptions.MetricsSamplingFrequency > 0 ? new GarnetServerMonitor(this, serverOptions, server, loggerFactory?.CreateLogger("GarnetServerMonitor")) : null; diff --git a/playground/CommandInfoUpdater/SupportedCommand.cs b/playground/CommandInfoUpdater/SupportedCommand.cs index 3252c60e3a..bced6830a0 100644 --- a/playground/CommandInfoUpdater/SupportedCommand.cs +++ b/playground/CommandInfoUpdater/SupportedCommand.cs @@ -82,6 +82,7 @@ public class SupportedCommand new("CLUSTER|MYID", RespCommand.CLUSTER_MYID), new("CLUSTER|MYPARENTID", RespCommand.CLUSTER_MYPARENTID), new("CLUSTER|NODES", RespCommand.CLUSTER_NODES), + new("CLUSTER|PUBLISH", RespCommand.CLUSTER_PUBLISH), new("CLUSTER|REPLICAS", RespCommand.CLUSTER_REPLICAS), new("CLUSTER|REPLICATE", RespCommand.CLUSTER_REPLICATE), new("CLUSTER|RESET", RespCommand.CLUSTER_RESET), @@ -92,7 +93,7 @@ public class SupportedCommand new("CLUSTER|SETSLOTSRANGE", RespCommand.CLUSTER_SETSLOTSRANGE), new("CLUSTER|SHARDS", RespCommand.CLUSTER_SHARDS), new("CLUSTER|SLOTS", RespCommand.CLUSTER_SLOTS), - new("CLUSTER|SLOTSTATE", RespCommand.CLUSTER_SLOTSTATE), + new("CLUSTER|SLOTSTATE", RespCommand.CLUSTER_SLOTSTATE) ]), new("COMMAND", RespCommand.COMMAND, [ diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs index f9a72c56fc..c19670c3c9 100644 --- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs +++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs @@ -2174,6 +2174,35 @@ static async Task DoClusterSlotStateAsync(GarnetClient client) } } + [Test] + public async Task ClusterPublishACLsAsync() + { + // All cluster command "success" is a thrown exception, because clustering is disabled + + await CheckCommandsAsync( + "CLUSTER PUBLISH", + [DoClusterPublishAsync] + ); + + static async Task DoClusterPublishAsync(GarnetClient client) + { + try + { + await client.ExecuteForStringResultAsync("CLUSTER", ["PUBLISH", "channel", "message"]); + Assert.Fail("Shouldn't be reachable, cluster isn't enabled"); + } + catch (Exception e) + { + if (e.Message == "ERR This instance has cluster support disabled") + { + return; + } + + throw; + } + } + } + [Test] public async Task CommandACLsAsync() { From 82951d88982095b9b638d0baaa14ae32efa32f3a Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 19 Nov 2024 19:45:32 -0800 Subject: [PATCH 05/31] implement SSUBSCRIBE api --- libs/resources/RespCommandsInfo.json | 25 ++++++++++++ libs/server/Resp/CmdStrings.cs | 1 + libs/server/Resp/Parser/RespCommand.cs | 11 +++++ libs/server/Resp/PubSubCommands.cs | 40 +++++++++++++++++-- libs/server/Resp/RespServerSession.cs | 3 +- .../CommandInfoUpdater/SupportedCommand.cs | 1 + test/Garnet.test/Resp/ACL/RespCommandTests.cs | 29 ++++++++++++++ 7 files changed, 105 insertions(+), 5 deletions(-) diff --git a/libs/resources/RespCommandsInfo.json b/libs/resources/RespCommandsInfo.json index 196aeccd2d..86e93b7dd4 100644 --- a/libs/resources/RespCommandsInfo.json +++ b/libs/resources/RespCommandsInfo.json @@ -4366,6 +4366,31 @@ "Flags": "Loading, NoScript, PubSub, Stale", "AclCategories": "PubSub, Slow" }, + { + "Command": "SSUBSCRIBE", + "Name": "SSUBSCRIBE", + "Arity": -2, + "Flags": "Loading, NoScript, PubSub, Stale", + "FirstKey": 1, + "LastKey": -1, + "Step": 1, + "AclCategories": "PubSub, Slow, Read", + "KeySpecifications": [ + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 1 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": -1, + "KeyStep": 1, + "Limit": 0 + }, + "Flags": "RO" + } + ] + }, { "Command": "SUBSTR", "Name": "SUBSTR", diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index 6304d93565..798e114251 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -15,6 +15,7 @@ static partial class CmdStrings /// public static ReadOnlySpan CLIENT => "CLIENT"u8; public static ReadOnlySpan SUBSCRIBE => "SUBSCRIBE"u8; + public static ReadOnlySpan SSUBSCRIBE => "SSUBSCRIBE"u8; public static ReadOnlySpan RUNTXP => "RUNTXP"u8; public static ReadOnlySpan GET => "GET"u8; public static ReadOnlySpan get => "get"u8; diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index 6e62c56cb5..b287b848a2 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -72,6 +72,7 @@ public enum RespCommand : ushort SMISMEMBER, SRANDMEMBER, SSCAN, + SSUBSCRIBE, STRLEN, SUBSTR, SUNION, @@ -1448,6 +1449,12 @@ private RespCommand FastParseArrayCommand(ref int count, ref ReadOnlySpan return RespCommand.HEXPIREAT; } break; + case 10: + if (*(ulong*)(ptr + 4) == MemoryMarshal.Read("SSUBSCRI"u8) && *(uint*)(ptr + 11) == MemoryMarshal.Read("BE\r\n"u8)) + { + return RespCommand.SSUBSCRIBE; + } + break; } // Reset optimistically changed state, if no matching command was found @@ -1666,6 +1673,10 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.SUBSCRIBE; } + else if (command.SequenceEqual(CmdStrings.SSUBSCRIBE)) + { + return RespCommand.SSUBSCRIBE; + } else if (command.SequenceEqual(CmdStrings.RUNTXP)) { return RespCommand.RUNTXP; diff --git a/libs/server/Resp/PubSubCommands.cs b/libs/server/Resp/PubSubCommands.cs index 89a9dc526d..d4ac690475 100644 --- a/libs/server/Resp/PubSubCommands.cs +++ b/libs/server/Resp/PubSubCommands.cs @@ -129,15 +129,36 @@ private bool NetworkPUBLISH() return true; } - private bool NetworkSUBSCRIBE() + private bool NetworkSUBSCRIBE(RespCommand cmd) { if (parseState.Count < 1) { - return AbortWithWrongNumberOfArguments(nameof(RespCommand.SUBSCRIBE)); + var cmdName = cmd switch + { + RespCommand.SUBSCRIBE => nameof(RespCommand.SUBSCRIBE), + RespCommand.SSUBSCRIBE => nameof(RespCommand.SSUBSCRIBE), + _ => throw new NotImplementedException() + }; + return AbortWithWrongNumberOfArguments(cmdName); + } + + if (cmd == RespCommand.SSUBSCRIBE && clusterSession == null) + { + // Print error message + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_CLUSTER_DISABLED, ref dcurr, dend)) + SendAndReset(); + return true; } - // SUBSCRIBE channel1 channel2.. ==> [$9\r\nSUBSCRIBE\r\n$]8\r\nchannel1\r\n$8\r\nchannel2\r\n => Subscribe to channel1 and channel2 var disabledBroker = subscribeBroker == null; + var header = cmd switch + { + RespCommand.SUBSCRIBE => "subscribe"u8, + RespCommand.SSUBSCRIBE => "ssubscribe"u8, + _ => throw new NotImplementedException() + }; + + // SUBSCRIBE|SUBSCRIBE channel1 channel2.. ==> [$9\r\nSUBSCRIBE\r\n$]8\r\nchannel1\r\n$8\r\nchannel2\r\n => Subscribe to channel1 and channel2 for (var c = 0; c < parseState.Count; c++) { var key = parseState.GetArgSliceByRef(c).SpanByte; @@ -150,8 +171,9 @@ private bool NetworkSUBSCRIBE() while (!RespWriteUtils.WriteArrayLength(3, ref dcurr, dend)) SendAndReset(); - while (!RespWriteUtils.WriteBulkString("subscribe"u8, ref dcurr, dend)) + while (!RespWriteUtils.WriteBulkString(header, ref dcurr, dend)) SendAndReset(); + while (!RespWriteUtils.WriteBulkString(new Span(keyPtr + sizeof(int), kSize), ref dcurr, dend)) SendAndReset(); @@ -452,5 +474,15 @@ private bool NetworkPUBSUB_NUMSUB() return true; } + + private bool NetworkSSUBSCRIBE() + { + if (parseState.Count < 1) + { + return AbortWithWrongNumberOfArguments(nameof(RespCommand.SSUBSCRIBE)); + } + + return true; + } } } \ No newline at end of file diff --git a/libs/server/Resp/RespServerSession.cs b/libs/server/Resp/RespServerSession.cs index 60b7009bfb..eebb3f91c6 100644 --- a/libs/server/Resp/RespServerSession.cs +++ b/libs/server/Resp/RespServerSession.cs @@ -604,7 +604,8 @@ private bool ProcessArrayCommands(RespCommand cmd, ref TGarnetApi st RespCommand.WATCHMS => NetworkWATCH_MS(), RespCommand.WATCHOS => NetworkWATCH_OS(), // Pub/sub commands - RespCommand.SUBSCRIBE => NetworkSUBSCRIBE(), + RespCommand.SUBSCRIBE => NetworkSUBSCRIBE(cmd), + RespCommand.SSUBSCRIBE => NetworkSUBSCRIBE(cmd), RespCommand.PSUBSCRIBE => NetworkPSUBSCRIBE(), RespCommand.UNSUBSCRIBE => NetworkUNSUBSCRIBE(), RespCommand.PUNSUBSCRIBE => NetworkPUNSUBSCRIBE(), diff --git a/playground/CommandInfoUpdater/SupportedCommand.cs b/playground/CommandInfoUpdater/SupportedCommand.cs index bced6830a0..9181b1e478 100644 --- a/playground/CommandInfoUpdater/SupportedCommand.cs +++ b/playground/CommandInfoUpdater/SupportedCommand.cs @@ -270,6 +270,7 @@ public class SupportedCommand new("SSCAN", RespCommand.SSCAN), new("STRLEN", RespCommand.STRLEN), new("SUBSCRIBE", RespCommand.SUBSCRIBE), + new("SSUBSCRIBE", RespCommand.SSUBSCRIBE), new("SUBSTR", RespCommand.SUBSTR), new("SUNION", RespCommand.SUNION), new("SUNIONSTORE", RespCommand.SUNIONSTORE), diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs index c19670c3c9..c8301a2170 100644 --- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs +++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs @@ -780,6 +780,35 @@ static async Task DoClientSetInfoAsync(GarnetClient client) } } + [Test] + public async Task SSubscribeACLsAsync() + { + // SUBSCRIBE is sufficient weird that all we care to test is forbidding it + await CheckCommandsAsync( + "SSUBSCRIBE", + [DoSSubscribeAsync], + skipPermitted: true + ); + + static async Task DoSSubscribeAsync(GarnetClient client) + { + try + { + await client.ExecuteForStringResultAsync("SSUBSCRIBE", ["channel"]); + Assert.Fail("Shouldn't be reachable, cluster isn't enabled"); + } + catch (Exception e) + { + if (e.Message == "ERR This instance has cluster support disabled") + { + return; + } + + throw; + } + } + } + [Test] public async Task ClusterAddSlotsACLsAsync() { From e80c3f92cfeba020028aec0d35d65eaafd854e39 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 21 Nov 2024 13:40:19 -0800 Subject: [PATCH 06/31] implement SPUBLISH api --- libs/resources/RespCommandsInfo.json | 25 ++++++++++++++++ libs/server/PubSub/SubscribeBroker.cs | 29 ------------------- libs/server/Resp/Parser/RespCommand.cs | 3 +- libs/server/Resp/PubSubCommands.cs | 28 ++++++++++-------- libs/server/Resp/RespServerSession.cs | 3 +- libs/server/Transaction/TxnKeyManager.cs | 1 + .../CommandInfoUpdater/SupportedCommand.cs | 1 + test/Garnet.test/Resp/ACL/RespCommandTests.cs | 29 +++++++++++++++++++ 8 files changed, 76 insertions(+), 43 deletions(-) diff --git a/libs/resources/RespCommandsInfo.json b/libs/resources/RespCommandsInfo.json index 86e93b7dd4..f9651230a5 100644 --- a/libs/resources/RespCommandsInfo.json +++ b/libs/resources/RespCommandsInfo.json @@ -4391,6 +4391,31 @@ } ] }, + { + "Command": "SPUBLISH", + "Name": "SPUBLISH", + "Arity": 3, + "Flags": "Loading, NoScript, PubSub, Stale", + "FirstKey": 1, + "LastKey": 1, + "Step": 1, + "AclCategories": "PubSub, Slow, Read", + "KeySpecifications": [ + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 1 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": 0, + "KeyStep": 1, + "Limit": 0 + }, + "Flags": "RO" + } + ] + }, { "Command": "SUBSTR", "Name": "SUBSTR", diff --git a/libs/server/PubSub/SubscribeBroker.cs b/libs/server/PubSub/SubscribeBroker.cs index 75afb319a5..3d57b750bf 100644 --- a/libs/server/PubSub/SubscribeBroker.cs +++ b/libs/server/PubSub/SubscribeBroker.cs @@ -380,35 +380,6 @@ public unsafe int PublishNow(byte* key, byte* value, int valueLength, bool ascii return numSubscribedSessions; } - /// - /// Publish the update made to key to all the subscribers, asynchronously - /// - /// key that has been updated - /// value that has been updated - /// value length that has been updated - /// is payload ascii - public unsafe void Publish(byte* key, byte* value, int valueLength, bool ascii = false) - { - if (subscriptions == null && prefixSubscriptions == null) return; - - var start = key; - ref TKey k = ref keySerializer.ReadKeyByRef(ref key); - // TODO: this needs to be a single atomic enqueue - byte[] logEntryBytes = new byte[(key - start) + valueLength + sizeof(bool)]; - fixed (byte* logEntryBytePtr = &logEntryBytes[0]) - { - byte* dst = logEntryBytePtr; - Buffer.MemoryCopy(start, dst, (key - start), (key - start)); - dst += (key - start); - Buffer.MemoryCopy(value, dst, valueLength, valueLength); - dst += valueLength; - byte* asciiPtr = (byte*)&ascii; - Buffer.MemoryCopy(asciiPtr, dst, sizeof(bool), sizeof(bool)); - } - - log.Enqueue(logEntryBytes); - } - /// /// Retrieves the collection of channels that have active subscriptions. /// diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index b287b848a2..c44c265ed8 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -70,6 +70,7 @@ public enum RespCommand : ushort SISMEMBER, SMEMBERS, SMISMEMBER, + SPUBLISH, SRANDMEMBER, SSCAN, SSUBSCRIBE, @@ -706,6 +707,7 @@ private RespCommand FastParseCommand(out int count) (2 << 4) | 6 when lastWord == MemoryMarshal.Read("GETSET\r\n"u8) => RespCommand.GETSET, (2 << 4) | 7 when lastWord == MemoryMarshal.Read("UBLISH\r\n"u8) && ptr[8] == 'P' => RespCommand.PUBLISH, (2 << 4) | 7 when lastWord == MemoryMarshal.Read("FMERGE\r\n"u8) && ptr[8] == 'P' => RespCommand.PFMERGE, + (2 << 4) | 8 when lastWord == MemoryMarshal.Read("UBLISH\r\n"u8) && *(ushort*)(ptr + 8) == MemoryMarshal.Read("SP"u8) => RespCommand.SPUBLISH, (2 << 4) | 5 when lastWord == MemoryMarshal.Read("\nSETNX\r\n"u8) => RespCommand.SETNX, (3 << 4) | 5 when lastWord == MemoryMarshal.Read("\nSETEX\r\n"u8) => RespCommand.SETEX, (3 << 4) | 6 when lastWord == MemoryMarshal.Read("PSETEX\r\n"u8) => RespCommand.PSETEX, @@ -727,7 +729,6 @@ private RespCommand FastParseCommand(out int count) >= ((6 << 4) | 2) and <= ((6 << 4) | 5) when lastWord == MemoryMarshal.Read("BITPOS\r\n"u8) => RespCommand.BITPOS, >= ((7 << 4) | 2) and <= ((7 << 4) | 3) when lastWord == MemoryMarshal.Read("EXPIRE\r\n"u8) && ptr[8] == 'P' => RespCommand.PEXPIRE, >= ((8 << 4) | 1) and <= ((8 << 4) | 4) when lastWord == MemoryMarshal.Read("TCOUNT\r\n"u8) && *(ushort*)(ptr + 8) == MemoryMarshal.Read("BI"u8) => RespCommand.BITCOUNT, - _ => MatchedNone(this, oldReadHead) } }; diff --git a/libs/server/Resp/PubSubCommands.cs b/libs/server/Resp/PubSubCommands.cs index d4ac690475..26d6bceb6d 100644 --- a/libs/server/Resp/PubSubCommands.cs +++ b/libs/server/Resp/PubSubCommands.cs @@ -94,11 +94,25 @@ public override unsafe void PrefixPublish(byte* patternPtr, int patternLength, r /// /// PUBLISH /// - private bool NetworkPUBLISH() + private bool NetworkPUBLISH(RespCommand cmd) { if (parseState.Count != 2) { - return AbortWithWrongNumberOfArguments(nameof(RespCommand.PUBLISH)); + var cmdName = cmd switch + { + RespCommand.PUBLISH => nameof(RespCommand.PUBLISH), + RespCommand.SPUBLISH => nameof(RespCommand.SPUBLISH), + _ => throw new NotImplementedException() + }; + return AbortWithWrongNumberOfArguments(cmdName); + } + + if (cmd == RespCommand.SPUBLISH && clusterSession == null) + { + // Print error message + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_CLUSTER_DISABLED, ref dcurr, dend)) + SendAndReset(); + return true; } Debug.Assert(isSubscriptionSession == false); @@ -474,15 +488,5 @@ private bool NetworkPUBSUB_NUMSUB() return true; } - - private bool NetworkSSUBSCRIBE() - { - if (parseState.Count < 1) - { - return AbortWithWrongNumberOfArguments(nameof(RespCommand.SSUBSCRIBE)); - } - - return true; - } } } \ No newline at end of file diff --git a/libs/server/Resp/RespServerSession.cs b/libs/server/Resp/RespServerSession.cs index eebb3f91c6..f8a0a65391 100644 --- a/libs/server/Resp/RespServerSession.cs +++ b/libs/server/Resp/RespServerSession.cs @@ -564,7 +564,8 @@ private bool ProcessBasicCommands(RespCommand cmd, ref TGarnetApi st RespCommand.GETBIT => NetworkStringGetBit(ref storageApi), RespCommand.BITCOUNT => NetworkStringBitCount(ref storageApi), RespCommand.BITPOS => NetworkStringBitPosition(ref storageApi), - RespCommand.PUBLISH => NetworkPUBLISH(), + RespCommand.PUBLISH => NetworkPUBLISH(RespCommand.PUBLISH), + RespCommand.SPUBLISH => NetworkPUBLISH(RespCommand.SPUBLISH), RespCommand.PING => parseState.Count == 0 ? NetworkPING() : NetworkArrayPING(), RespCommand.ASKING => NetworkASKING(), RespCommand.MULTI => NetworkMULTI(), diff --git a/libs/server/Transaction/TxnKeyManager.cs b/libs/server/Transaction/TxnKeyManager.cs index 7238a9ecb6..2e525eee68 100644 --- a/libs/server/Transaction/TxnKeyManager.cs +++ b/libs/server/Transaction/TxnKeyManager.cs @@ -189,6 +189,7 @@ private static int AdminCommands(RespCommand command) RespCommand.CLIENT => 1, RespCommand.PING => 1, RespCommand.PUBLISH => 1, + RespCommand.SPUBLISH => 1, _ => -1 }; } diff --git a/playground/CommandInfoUpdater/SupportedCommand.cs b/playground/CommandInfoUpdater/SupportedCommand.cs index 9181b1e478..ba1d673957 100644 --- a/playground/CommandInfoUpdater/SupportedCommand.cs +++ b/playground/CommandInfoUpdater/SupportedCommand.cs @@ -265,6 +265,7 @@ public class SupportedCommand new("SMISMEMBER", RespCommand.SMISMEMBER), new("SMOVE", RespCommand.SMOVE), new("SPOP", RespCommand.SPOP), + new("SPUBLISH", RespCommand.SPUBLISH), new("SRANDMEMBER", RespCommand.SRANDMEMBER), new("SREM", RespCommand.SREM), new("SSCAN", RespCommand.SSCAN), diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs index c8301a2170..698649b5a3 100644 --- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs +++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs @@ -809,6 +809,35 @@ static async Task DoSSubscribeAsync(GarnetClient client) } } + [Test] + public async Task SPublishACLsAsync() + { + // SUBSCRIBE is sufficient weird that all we care to test is forbidding it + await CheckCommandsAsync( + "SPUBLISH", + [DoSPublishAsync], + skipPermitted: true + ); + + static async Task DoSPublishAsync(GarnetClient client) + { + try + { + await client.ExecuteForStringResultAsync("SPUBLISH", ["channel", "message"]); + Assert.Fail("Shouldn't be reachable, cluster isn't enabled"); + } + catch (Exception e) + { + if (e.Message == "ERR This instance has cluster support disabled") + { + return; + } + + throw; + } + } + } + [Test] public async Task ClusterAddSlotsACLsAsync() { From fae3ce4626d361673424aa82b307aaaf9f375e18 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Wed, 4 Dec 2024 18:03:27 -0800 Subject: [PATCH 07/31] implement separate code path for CLUSTER SPUBLISH --- libs/cluster/Session/ClusterCommands.cs | 2 +- .../Session/RespClusterBasicCommands.cs | 4 ++- libs/resources/RespCommandsInfo.json | 13 ++++++++- libs/server/Resp/CmdStrings.cs | 1 + libs/server/Resp/Parser/RespCommand.cs | 5 ++++ .../CommandInfoUpdater/SupportedCommand.cs | 1 + test/Garnet.test/Resp/ACL/RespCommandTests.cs | 29 +++++++++++++++++++ 7 files changed, 52 insertions(+), 3 deletions(-) diff --git a/libs/cluster/Session/ClusterCommands.cs b/libs/cluster/Session/ClusterCommands.cs index e9e6d2c116..d8a682519b 100644 --- a/libs/cluster/Session/ClusterCommands.cs +++ b/libs/cluster/Session/ClusterCommands.cs @@ -164,7 +164,7 @@ private void ProcessClusterCommands(RespCommand command, out bool invalidParamet RespCommand.CLUSTER_MYID => NetworkClusterMyId(out invalidParameters), RespCommand.CLUSTER_MYPARENTID => NetworkClusterMyParentId(out invalidParameters), RespCommand.CLUSTER_NODES => NetworkClusterNodes(out invalidParameters), - RespCommand.CLUSTER_PUBLISH => NetworkClusterPublish(out invalidParameters), + RespCommand.CLUSTER_PUBLISH or RespCommand.CLUSTER_SPUBLISH => NetworkClusterPublish(out invalidParameters), RespCommand.CLUSTER_REPLICAS => NetworkClusterReplicas(out invalidParameters), RespCommand.CLUSTER_REPLICATE => NetworkClusterReplicate(out invalidParameters), RespCommand.CLUSTER_RESET => NetworkClusterReset(out invalidParameters), diff --git a/libs/cluster/Session/RespClusterBasicCommands.cs b/libs/cluster/Session/RespClusterBasicCommands.cs index 387c3eebd1..84704b9037 100644 --- a/libs/cluster/Session/RespClusterBasicCommands.cs +++ b/libs/cluster/Session/RespClusterBasicCommands.cs @@ -483,11 +483,13 @@ private bool NetworkClusterPublish(out bool invalidParameters) var kSize = key.Length; var vSize = val.Length; + *(int*)keyPtr = kSize; + *(int*)valPtr = vSize; + var numClients = clusterProvider.storeWrapper.subscribeBroker.PublishNow(keyPtr, valPtr, vSize + sizeof(int), true); while (!RespWriteUtils.WriteInteger(numClients, ref dcurr, dend)) SendAndReset(); - return true; } } diff --git a/libs/resources/RespCommandsInfo.json b/libs/resources/RespCommandsInfo.json index f9651230a5..55339727a5 100644 --- a/libs/resources/RespCommandsInfo.json +++ b/libs/resources/RespCommandsInfo.json @@ -619,7 +619,18 @@ "FirstKey": 1, "LastKey": 1, "Step": 1, - "AclCategories": "Admin, PubSub, Slow, Garnet, Read", + "AclCategories": "Admin, PubSub, Slow, Garnet" + }, + { + "Command": "CLUSTER_SPUBLISH", + "Name": "CLUSTER|SPUBLISH", + "IsInternal": true, + "Arity": 4, + "Flags": "Loading, NoScript, PubSub, Stale", + "FirstKey": 1, + "LastKey": 1, + "Step": 1, + "AclCategories": "Admin, PubSub, Slow, Garnet", "KeySpecifications": [ { "BeginSearch": { diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index 798e114251..c047587991 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -352,6 +352,7 @@ static partial class CmdStrings public static ReadOnlySpan setslotsrange => "SETSLOTSRANGE"u8; public static ReadOnlySpan slotstate => "SLOTSTATE"u8; public static ReadOnlySpan publish => "PUBLISH"u8; + public static ReadOnlySpan spublish => "SPUBLISH"u8; public static ReadOnlySpan mtasks => "MTASKS"u8; public static ReadOnlySpan aofsync => "AOFSYNC"u8; public static ReadOnlySpan appendlog => "APPENDLOG"u8; diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index c44c265ed8..8ba0dfdbbe 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -334,6 +334,7 @@ public enum RespCommand : ushort CLUSTER_MYPARENTID, CLUSTER_NODES, CLUSTER_PUBLISH, + CLUSTER_SPUBLISH, CLUSTER_REPLICAS, CLUSTER_REPLICATE, CLUSTER_RESET, @@ -1962,6 +1963,10 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.CLUSTER_PUBLISH; } + else if (subCommand.SequenceEqual(CmdStrings.spublish)) + { + return RespCommand.CLUSTER_SPUBLISH; + } else if (subCommand.SequenceEqual(CmdStrings.MIGRATE)) { return RespCommand.CLUSTER_MIGRATE; diff --git a/playground/CommandInfoUpdater/SupportedCommand.cs b/playground/CommandInfoUpdater/SupportedCommand.cs index ba1d673957..6a5b0cc106 100644 --- a/playground/CommandInfoUpdater/SupportedCommand.cs +++ b/playground/CommandInfoUpdater/SupportedCommand.cs @@ -83,6 +83,7 @@ public class SupportedCommand new("CLUSTER|MYPARENTID", RespCommand.CLUSTER_MYPARENTID), new("CLUSTER|NODES", RespCommand.CLUSTER_NODES), new("CLUSTER|PUBLISH", RespCommand.CLUSTER_PUBLISH), + new("CLUSTER|SPUBLISH", RespCommand.CLUSTER_SPUBLISH), new("CLUSTER|REPLICAS", RespCommand.CLUSTER_REPLICAS), new("CLUSTER|REPLICATE", RespCommand.CLUSTER_REPLICATE), new("CLUSTER|RESET", RespCommand.CLUSTER_RESET), diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs index 698649b5a3..e43815c987 100644 --- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs +++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs @@ -2261,6 +2261,35 @@ static async Task DoClusterPublishAsync(GarnetClient client) } } + [Test] + public async Task ClusterSPublishACLsAsync() + { + // All cluster command "success" is a thrown exception, because clustering is disabled + + await CheckCommandsAsync( + "CLUSTER SPUBLISH", + [DoClusterSPublishAsync] + ); + + static async Task DoClusterSPublishAsync(GarnetClient client) + { + try + { + await client.ExecuteForStringResultAsync("CLUSTER", ["SPUBLISH", "channel", "message"]); + Assert.Fail("Shouldn't be reachable, cluster isn't enabled"); + } + catch (Exception e) + { + if (e.Message == "ERR This instance has cluster support disabled") + { + return; + } + + throw; + } + } + } + [Test] public async Task CommandACLsAsync() { From 54ae8936908e94bd2fda2427c9a3687701471e3d Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 5 Dec 2024 15:16:22 -0800 Subject: [PATCH 08/31] expose max number of outstanding pubsub forwarding tasks parameter --- libs/host/Configuration/Options.cs | 5 +++++ libs/host/defaults.conf | 3 +++ libs/server/Servers/ServerOptions.cs | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/libs/host/Configuration/Options.cs b/libs/host/Configuration/Options.cs index 331eeea5e3..b217271c96 100644 --- a/libs/host/Configuration/Options.cs +++ b/libs/host/Configuration/Options.cs @@ -156,6 +156,10 @@ internal sealed class Options [Option("pubsub-pagesize", Required = false, HelpText = "Page size of log used for pub/sub (rounds down to power of 2)")] public string PubSubPageSize { get; set; } + [IntRangeValidation(0, int.MaxValue)] + [Option("max-pubsub-tasks", Required = false, HelpText = "Number of outstanding forwarding pubsub tasks at any given time")] + public int MaxPubSubTasks { get; set; } + [OptionValidation] [Option("no-obj", Required = false, HelpText = "Disable support for data structure objects.")] public bool? DisableObjects { get; set; } @@ -662,6 +666,7 @@ public GarnetServerOptions GetServerOptions(ILogger logger = null) EnableIncrementalSnapshots = EnableIncrementalSnapshots.GetValueOrDefault(), DisablePubSub = DisablePubSub.GetValueOrDefault(), PubSubPageSize = PubSubPageSize, + MaxPubSubTasks = MaxPubSubTasks, DisableObjects = DisableObjects.GetValueOrDefault(), EnableCluster = EnableCluster.GetValueOrDefault(), CleanClusterConfig = CleanClusterConfig.GetValueOrDefault(), diff --git a/libs/host/defaults.conf b/libs/host/defaults.conf index 01c31e5c3f..575bb54f33 100644 --- a/libs/host/defaults.conf +++ b/libs/host/defaults.conf @@ -96,6 +96,9 @@ /* Page size of log used for pub/sub (rounds down to power of 2) */ "PubSubPageSize" : "4k", + /* Number of outstanding forwarding pubsub tasks at any given time */ + "MaxPubSubTasks" : "1024", + /* Disable support for data structure objects. */ "DisableObjects" : false, diff --git a/libs/server/Servers/ServerOptions.cs b/libs/server/Servers/ServerOptions.cs index 92795ba60d..6a0cd60b14 100644 --- a/libs/server/Servers/ServerOptions.cs +++ b/libs/server/Servers/ServerOptions.cs @@ -93,6 +93,11 @@ public class ServerOptions /// public string PubSubPageSize = "4k"; + /// + /// Number of outstanding forwarding pubsub tasks at any given time + /// + public int MaxPubSubTasks = 1 << 10; + /// /// Server bootup should fail if errors happen during bootup of AOF and checkpointing. /// From 0830b59c68be68c90ef6fefd8196331e6ed7cc67 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 5 Dec 2024 16:10:08 -0800 Subject: [PATCH 09/31] add API to forward published messages using GarnetClient --- libs/client/GarnetClient.cs | 151 +++++++++++++++--- .../GarnetClientClusterCommands.cs | 10 ++ .../GarnetClientAPI/GarnetClientExecuteAPI.cs | 38 ++--- libs/cluster/Server/GarnetClientExtensions.cs | 7 + 4 files changed, 165 insertions(+), 41 deletions(-) diff --git a/libs/client/GarnetClient.cs b/libs/client/GarnetClient.cs index dcd976a7e6..d0e7c24444 100644 --- a/libs/client/GarnetClient.cs +++ b/libs/client/GarnetClient.cs @@ -50,7 +50,6 @@ public sealed partial class GarnetClient : IServerHook, IMessageConsumer, IDispo readonly int bufferSize; readonly int maxOutstandingTasks; NetworkWriter networkWriter; - INetworkSender networkSender; readonly TcsWrapper[] tcsArray; readonly SslClientAuthenticationOptions sslOptions; @@ -191,7 +190,6 @@ public void Connect(CancellationToken token = default) socket = CreateSendSocket(timeoutMilliseconds); networkWriter = new NetworkWriter(this, socket, bufferSize, sslOptions, out networkHandler, sendPageSize, networkSendThrottleMax, logger); networkHandler.StartAsync(sslOptions, $"{address}:{port}", token).ConfigureAwait(false).GetAwaiter().GetResult(); - networkSender = networkHandler.GetNetworkSender(); if (timeoutMilliseconds > 0) { @@ -224,7 +222,6 @@ public async Task ConnectAsync(CancellationToken token = default) socket = CreateSendSocket(timeoutMilliseconds); networkWriter = new NetworkWriter(this, socket, bufferSize, sslOptions, out networkHandler, sendPageSize, networkSendThrottleMax, logger); await networkHandler.StartAsync(sslOptions, $"{address}:{port}", token).ConfigureAwait(false); - networkSender = networkHandler.GetNetworkSender(); if (timeoutMilliseconds > 0) { @@ -631,32 +628,136 @@ async ValueTask InternalExecuteAsync(TcsWrapper tcs, Memory op, string par } } - async ValueTask InternalExecuteAsync(Memory op, Memory clusterOp, string nodeId, long currentAddress, long nextAddress, long payloadPtr, int payloadLength, CancellationToken token = default) + async ValueTask InternalExecuteAsync(TcsWrapper tcs, Memory op, Memory param1, Memory param2, CancellationToken token = default) { - Debug.Assert(nodeId != null); - + tcs.timestamp = GetTimestamp(); int totalLen = 0; int arraySize = 1; totalLen += op.Length; - int len = clusterOp.Length; - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; + if (!param1.IsEmpty) + { + int len = param1.Length; + totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; + arraySize++; + } + if (!param2.IsEmpty) + { + int len = param2.Length; + totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; + arraySize++; + } - len = Encoding.UTF8.GetByteCount(nodeId); - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; + totalLen += 1 + NumUtils.NumDigits(arraySize) + 2; + CheckLength(totalLen, tcs); + await InputGateAsync(token); + + try + { + networkWriter.epoch.Resume(); + + #region reserveSpaceAndWriteIntoNetworkBuffer + int taskId; + long address; + while (true) + { + token.ThrowIfCancellationRequested(); + if (!IsConnected) + { + Dispose(); + ThrowException(disposeException); + } + (taskId, address) = networkWriter.TryAllocate(totalLen, out var flushEvent); + if (address >= 0) break; + try + { + networkWriter.epoch.Suspend(); + await flushEvent.WaitAsync(token).ConfigureAwait(false); + } + finally + { + networkWriter.epoch.Resume(); + } + } + + // Console.WriteLine($"Allocated {taskId} @ {address}"); + tcs.nextTaskId = taskId; + + unsafe + { + byte* curr = (byte*)networkWriter.GetPhysicalAddress(address); + byte* end = curr + totalLen; + RespWriteUtils.WriteArrayLength(arraySize, ref curr, end); + + RespWriteUtils.WriteDirect(op.Span, ref curr, end); + if (!param1.IsEmpty) + RespWriteUtils.WriteBulkString(param1.Span, ref curr, end); + if (!param2.IsEmpty) + RespWriteUtils.WriteBulkString(param2.Span, ref curr, end); + + Debug.Assert(curr == end); + } + #endregion + + #region waitForEmptySlot + int shortTaskId = taskId & (maxOutstandingTasks - 1); + var oldTcs = tcsArray[shortTaskId]; + //1. if taskType != None, we are waiting for previous task to finish + //2. if taskType == None and my taskId is not the next in line wait for previous task to acquire slot + if (oldTcs.taskType != TaskType.None || !oldTcs.IsNext(taskId)) + { + // Console.WriteLine($"Before filling slot {taskId & (maxOutstandingTasks - 1)} for task {taskId} @ {address} : {tcs.taskType}"); + networkWriter.epoch.ProtectAndDrain(); + networkWriter.DoAggressiveShiftReadOnly(); + try + { + networkWriter.epoch.Suspend(); + await AwaitPreviousTaskAsync(taskId); // does not take token, as task is not cancelable at this point + } + finally + { + networkWriter.epoch.Resume(); + } + } + #endregion + + #region scheduleAwaitForResponse + // Console.WriteLine($"Filled slot {taskId & (maxOutstandingTasks - 1)} for task {taskId} @ {address} : {tcs.taskType}"); + tcsArray[shortTaskId].LoadFrom(tcs); + if (Disposed) + { + DisposeOffset(shortTaskId); + ThrowException(disposeException); + } + // Console.WriteLine($"Filled {address}-{address + totalLen}"); + networkWriter.epoch.ProtectAndDrain(); + networkWriter.DoAggressiveShiftReadOnly(); + #endregion + } + finally + { + networkWriter.epoch.Suspend(); + } + return; + } + + async ValueTask InternalExecuteForNoResponse(Memory op, Memory subop, Memory param1, Memory param2, CancellationToken token = default) + { + int totalLen = 0; + int arraySize = 1; + + totalLen += op.Length; - len = NumUtils.NumDigitsInLong(currentAddress); + int len = subop.Length; totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; arraySize++; - len = NumUtils.NumDigitsInLong(nextAddress); + len = param1.Length; totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; arraySize++; - len = payloadLength; + len = param2.Length; totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; arraySize++; @@ -705,12 +806,9 @@ async ValueTask InternalExecuteAsync(Memory op, Memory clusterOp, st RespWriteUtils.WriteArrayLength(arraySize, ref curr, end); RespWriteUtils.WriteDirect(op.Span, ref curr, end); - RespWriteUtils.WriteBulkString(clusterOp.Span, ref curr, end); - RespWriteUtils.WriteUtf8BulkString(nodeId, ref curr, end); - RespWriteUtils.WriteArrayItem(currentAddress, ref curr, end); - RespWriteUtils.WriteArrayItem(nextAddress, ref curr, end); - RespWriteUtils.WriteBulkString(new Span((void*)payloadPtr, payloadLength), ref curr, end); - + RespWriteUtils.WriteBulkString(subop.Span, ref curr, end); + RespWriteUtils.WriteBulkString(param1.Span, ref curr, end); + RespWriteUtils.WriteBulkString(param2.Span, ref curr, end); Debug.Assert(curr == end); } #endregion @@ -731,14 +829,19 @@ async ValueTask InternalExecuteAsync(Memory op, Memory clusterOp, st return; } - async ValueTask InternalExecuteAsync(TcsWrapper tcs, Memory op, Memory param1, Memory param2, CancellationToken token = default) + async ValueTask InternalExecuteFireAndForgetWithNoResponse(TcsWrapper tcs, Memory op, Memory subop, Memory param1, Memory param2, CancellationToken token = default) { - tcs.timestamp = GetTimestamp(); int totalLen = 0; int arraySize = 1; totalLen += op.Length; + if (!subop.IsEmpty) + { + int len = subop.Length; + totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; + arraySize++; + } if (!param1.IsEmpty) { int len = param1.Length; @@ -794,6 +897,8 @@ async ValueTask InternalExecuteAsync(TcsWrapper tcs, Memory op, Memory CLUSTER = "$7\r\nCLUSTER\r\n"u8.ToArray(); static readonly Memory FAILOVER = "FAILOVER"u8.ToArray(); + /// + /// PUBLISH resp formatted + /// + public static readonly Memory PUBLISH = "PUBLISH"u8.ToArray(); + + /// + /// PUBLISH resp formatted + /// + public static readonly Memory SPUBLISH = "SPUBLISH"u8.ToArray(); + /// /// Issue cluster failover command to replica node /// diff --git a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs index a20ae5823d..dad5f169f5 100644 --- a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs +++ b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs @@ -159,23 +159,6 @@ public async Task ExecuteForStringResultWithCancellationAsync(Memory - /// Execute command (async) - /// - /// Operation in resp format - /// - /// - /// - /// - /// - /// - /// - /// - public async Task ExecuteForVoidResultWithCancellationAsync(Memory respOp, Memory clusterOp, string nodeId, long currentAddress, long nextAddress, long payloadPtr, int payloadLength, CancellationToken token = default) - { - await InternalExecuteAsync(respOp, clusterOp, nodeId, currentAddress, nextAddress, payloadPtr, payloadLength, token); - } - /// /// Execute command (async) /// @@ -1061,7 +1044,6 @@ public async Task ExecuteForLongResultWithCancellationAsync(string op, ICo } } - /// /// Execute command (async) with cancellation token /// @@ -1087,6 +1069,26 @@ public async Task ExecuteForLongResultWithCancellationAsync(Memory r } } + /// + /// Execute command (async) with cancellation token + /// + /// + /// + /// + /// + /// + /// + public async Task ExecuteFireAndForgetWithNoResponse(Memory op, Memory param1, Memory param2, Memory param3, CancellationToken token = default) + { + var tcs = new TcsWrapper { taskType = TaskType.LongAsync, longTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) }; + var _ = InternalExecuteFireAndForgetWithNoResponse(tcs, op, param1, param2, param3, token); + return await tcs.longTcs.Task.ConfigureAwait(false); + } + + public void ExecuteForNoResponse(Memory op, Memory param1, Memory param2, Memory param3, CancellationToken token = default) + { + var _ = InternalExecuteForNoResponse(op, param1, param2, param3, token); + } #endregion diff --git a/libs/cluster/Server/GarnetClientExtensions.cs b/libs/cluster/Server/GarnetClientExtensions.cs index c81b226358..fbb18dd4cc 100644 --- a/libs/cluster/Server/GarnetClientExtensions.cs +++ b/libs/cluster/Server/GarnetClientExtensions.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using Garnet.client; using Garnet.common; +using Garnet.server; namespace Garnet.cluster { @@ -60,5 +61,11 @@ public static async Task failreplicationoffset(this GarnetClient client, l }; return await client.ExecuteForLongResultWithCancellationAsync(GarnetClient.CLUSTER, args, cancellationToken).ConfigureAwait(false); } + + public static async Task ClusterPublish(this GarnetClient client, RespCommand cmd, Memory channel, Memory message, CancellationToken cancellationToken = default) + => await client.ExecuteFireAndForgetWithNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, channel, message, cancellationToken); + + //public static void ClusterPublish(this GarnetClient client, RespCommand cmd, Memory channel, Memory message, CancellationToken cancellationToken = default) + // => client.ExecuteForNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, channel, message, cancellationToken); } } \ No newline at end of file From 824f614f603bd46aae6f05e1130e9b251c48e1ed Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 5 Dec 2024 16:14:39 -0800 Subject: [PATCH 10/31] add methods to extract nodeIds of all nodes in cluster and all nodes in a shard --- libs/cluster/Server/ClusterConfig.cs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/libs/cluster/Server/ClusterConfig.cs b/libs/cluster/Server/ClusterConfig.cs index c45320b0ce..f4c1717d55 100644 --- a/libs/cluster/Server/ClusterConfig.cs +++ b/libs/cluster/Server/ClusterConfig.cs @@ -785,6 +785,33 @@ public List GetReplicas(string nodeid, ClusterProvider clusterProvider) return replicas; } + /// + /// Return the nodeIds of all nodes in the shard + /// + /// + public IEnumerable GetShardNodeIds() + { + var primaryId = LocalNodeRole == NodeRole.PRIMARY ? LocalNodeId : workers[1].ReplicaOfNodeId; + if (primaryId == null) yield break; + + for (ushort i = 2; i < workers.Length; i++) + { + var replicaOf = workers[i].ReplicaOfNodeId; + if ((replicaOf != null && replicaOf.Equals(primaryId, StringComparison.OrdinalIgnoreCase)) || primaryId.Equals(workers[i].Nodeid)) + yield return workers[i].Nodeid; + } + } + + /// + /// Return the nodeIds of all nodes in the cluster + /// + /// + public IEnumerable GetAllNodeIds() + { + for (ushort i = 2; i < workers.Length; i++) + yield return workers[i].Nodeid; + } + /// /// /// From bd744bfbce0dfff6ce520e51f4ac92c562228891 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 5 Dec 2024 16:16:10 -0800 Subject: [PATCH 11/31] hook up published message forwarding --- libs/cluster/Server/ClusterProvider.cs | 3 ++ libs/cluster/Server/GarnetServerNode.cs | 24 +++++++++++-- libs/cluster/Server/Gossip.cs | 46 +++++++++++++++++++++++++ libs/server/Cluster/IClusterProvider.cs | 8 +++++ libs/server/Resp/PubSubCommands.cs | 8 +++++ 5 files changed, 87 insertions(+), 2 deletions(-) diff --git a/libs/cluster/Server/ClusterProvider.cs b/libs/cluster/Server/ClusterProvider.cs index 398f3bc93e..cf04055d39 100644 --- a/libs/cluster/Server/ClusterProvider.cs +++ b/libs/cluster/Server/ClusterProvider.cs @@ -355,6 +355,9 @@ public void ExtractKeySpecs(RespCommandsInfo commandInfo, RespCommand cmd, ref S } } + public void ClusterPublish(RespCommand cmd, ref Span channel, ref Span message) + => clusterManager.TryClusterPublish(cmd, ref channel, ref message); + internal ReplicationLogCheckpointManager GetReplicationLogCheckpointManager(StoreType storeType) { Debug.Assert(serverOptions.EnableCluster); diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index dae6afb3e4..e3747a9c6c 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using Garnet.client; using Garnet.common; +using Garnet.server; using Microsoft.Extensions.Logging; namespace Garnet.cluster @@ -59,6 +60,16 @@ internal sealed class GarnetServerNode /// public int Port; + /// + /// Default send page size for GarnetClient + /// + const int defaultSendPageSize = 1 << 17; + + /// + /// Default max outstanding tasks for GarnetClient + /// + const int defaultMaxOutstandingTask = 8; + /// /// GarnetServerNode constructor /// @@ -69,13 +80,14 @@ internal sealed class GarnetServerNode /// public GarnetServerNode(ClusterProvider clusterProvider, string address, int port, SslClientAuthenticationOptions tlsOptions, ILogger logger = null) { + var opts = clusterProvider.storeWrapper.serverOptions; this.clusterProvider = clusterProvider; this.Address = address; this.Port = port; this.gc = new GarnetClient( address, port, tlsOptions, - sendPageSize: 1 << 17, - maxOutstandingTasks: 8, + sendPageSize: opts.DisablePubSub ? defaultSendPageSize : Math.Max(defaultSendPageSize, (int)opts.PubSubPageSizeBytes()), + maxOutstandingTasks: opts.DisablePubSub ? defaultMaxOutstandingTask : Math.Max(defaultMaxOutstandingTask, opts.MaxPubSubTasks), authUsername: clusterProvider.clusterManager.clusterProvider.ClusterUsername, authPassword: clusterProvider.clusterManager.clusterProvider.ClusterPassword, logger: logger); @@ -259,5 +271,13 @@ public ConnectionInfo GetConnectionInfo() lastIO = last_io_seconds, }; } + + /// + /// Send a CLUSTER PUBLISH message to another remote node + /// + /// + /// + public void TryClusterPublish(RespCommand cmd, Memory channel, Memory message) + => gc.ClusterPublish(cmd, channel, message).GetAwaiter().GetResult(); } } \ No newline at end of file diff --git a/libs/cluster/Server/Gossip.cs b/libs/cluster/Server/Gossip.cs index e1b1987b60..8ee85699c5 100644 --- a/libs/cluster/Server/Gossip.cs +++ b/libs/cluster/Server/Gossip.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Garnet.common; +using Garnet.server; using Microsoft.Extensions.Logging; namespace Garnet.cluster @@ -200,6 +201,51 @@ public async Task TryMeetAsync(string address, int port, bool acquireLock = true } } + public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span message) + { + GarnetServerNode gsn = null; + var conf = CurrentConfig; + bool created = false; + + var _channel = channel.ToArray(); + var _message = message.ToArray(); + + foreach (var nodeId in cmd == RespCommand.PUBLISH ? conf.GetAllNodeIds() : conf.GetShardNodeIds()) + { + try + { + if (nodeId != null) + clusterConnectionStore.GetConnection(nodeId, out gsn); + + if (gsn == null) + { + var (address, port) = conf.GetEndpointFromNodeId(nodeId); + gsn = new GarnetServerNode(clusterProvider, address, port, tlsOptions?.TlsClientOptions, logger: logger); + created = true; + } + + // Initialize GarnetServerNode + // Thread-Safe initialization executes only once + gsn.Initialize(); + + // Publish to remote nodes + gsn.TryClusterPublish(cmd, _channel, _message); + + if (created && !clusterConnectionStore.AddConnection(gsn)) + gsn.Dispose(); + } + catch (Exception ex) + { + logger?.LogError(ex, $"{nameof(ClusterManager)}.{nameof(TryClusterPublish)}"); + if (created) gsn?.Dispose(); + } + finally + { + + } + } + } + /// /// Main gossip async task /// diff --git a/libs/server/Cluster/IClusterProvider.cs b/libs/server/Cluster/IClusterProvider.cs index bf664141e1..1abd1de0e2 100644 --- a/libs/server/Cluster/IClusterProvider.cs +++ b/libs/server/Cluster/IClusterProvider.cs @@ -63,6 +63,14 @@ public interface IClusterProvider : IDisposable /// void ExtractKeySpecs(RespCommandsInfo commandInfo, RespCommand cmd, ref SessionParseState parseState, ref ClusterSlotVerificationInput csvi); + /// + /// Issue a cluster publish message to remote nodes + /// + /// + /// + /// + void ClusterPublish(RespCommand cmd, ref Span channel, ref Span message); + /// /// Is replica /// diff --git a/libs/server/Resp/PubSubCommands.cs b/libs/server/Resp/PubSubCommands.cs index 26d6bceb6d..9f10a62db5 100644 --- a/libs/server/Resp/PubSubCommands.cs +++ b/libs/server/Resp/PubSubCommands.cs @@ -137,6 +137,14 @@ private bool NetworkPUBLISH(RespCommand cmd) *(int*)valPtr = vSize; var numClients = subscribeBroker.PublishNow(keyPtr, valPtr, vSize + sizeof(int), true); + + if (storeWrapper.serverOptions.EnableCluster) + { + var _key = parseState.GetArgSliceByRef(0).Span; + var _val = parseState.GetArgSliceByRef(1).Span; + storeWrapper.clusterProvider.ClusterPublish(cmd, ref _key, ref _val); + } + while (!RespWriteUtils.WriteInteger(numClients, ref dcurr, dend)) SendAndReset(); From c7208bcf9c01843500dcbd0bbc84260a4315598c Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 9 Dec 2024 15:06:33 -0800 Subject: [PATCH 12/31] PUBLISH and SPUBLISH benchmark addition --- benchmark/Resp.benchmark/OpType.cs | 1 + benchmark/Resp.benchmark/Program.cs | 2 +- benchmark/Resp.benchmark/ReqGen.cs | 2 ++ benchmark/Resp.benchmark/ReqGenLoadBuffers.cs | 4 ++++ benchmark/Resp.benchmark/ReqGenUtils.cs | 4 ++++ benchmark/Resp.benchmark/RespOnlineBench.cs | 21 ++++++++++++++++++- 6 files changed, 32 insertions(+), 2 deletions(-) diff --git a/benchmark/Resp.benchmark/OpType.cs b/benchmark/Resp.benchmark/OpType.cs index 4b4c3e4db5..b4d7f2fbd8 100644 --- a/benchmark/Resp.benchmark/OpType.cs +++ b/benchmark/Resp.benchmark/OpType.cs @@ -12,6 +12,7 @@ public enum OpType DBSIZE, READ_TXN, WRITE_TXN, READWRITETX, WATCH_TXN, SAMPLEUPDATETX, SAMPLEDELETETX, SCRIPTSET, SCRIPTGET, SCRIPTRETKEY, + PUBLISH, SPUBLISH, READONLY = 8888, AUTH = 9999, } diff --git a/benchmark/Resp.benchmark/Program.cs b/benchmark/Resp.benchmark/Program.cs index cb3a151e8d..42a41f8de2 100644 --- a/benchmark/Resp.benchmark/Program.cs +++ b/benchmark/Resp.benchmark/Program.cs @@ -219,7 +219,7 @@ static void RunBasicCommandsBenchmark(Options opts) int keyLen = opts.KeyLength; int valueLen = opts.ValueLength; - if (opts.Op == OpType.ZADD || opts.Op == OpType.ZREM || opts.Op == OpType.ZADDREM || opts.Op == OpType.PING || opts.Op == OpType.GEOADD || opts.Op == OpType.GEOADDREM || opts.Op == OpType.SETEX || opts.Op == OpType.ZCARD || opts.Op == OpType.ZADDCARD) + if (opts.Op == OpType.PUBLISH || opts.Op == OpType.SPUBLISH || opts.Op == OpType.ZADD || opts.Op == OpType.ZREM || opts.Op == OpType.ZADDREM || opts.Op == OpType.PING || opts.Op == OpType.GEOADD || opts.Op == OpType.GEOADDREM || opts.Op == OpType.SETEX || opts.Op == OpType.ZCARD || opts.Op == OpType.ZADDCARD) opts.SkipLoad = true; //if we have scripts ops we need to load them in memory diff --git a/benchmark/Resp.benchmark/ReqGen.cs b/benchmark/Resp.benchmark/ReqGen.cs index feb83b6fca..c6d30a00a6 100644 --- a/benchmark/Resp.benchmark/ReqGen.cs +++ b/benchmark/Resp.benchmark/ReqGen.cs @@ -204,6 +204,8 @@ public static (int, int) OnResponse(byte* buf, int bytesRead, int opType) case OpType.DEL: case OpType.INCR: case OpType.DBSIZE: + case OpType.PUBLISH: + case OpType.SPUBLISH: for (int i = 0; i < bytesRead; i++) if (buf[i] == ':') count++; break; diff --git a/benchmark/Resp.benchmark/ReqGenLoadBuffers.cs b/benchmark/Resp.benchmark/ReqGenLoadBuffers.cs index fa34703eba..fd452599df 100644 --- a/benchmark/Resp.benchmark/ReqGenLoadBuffers.cs +++ b/benchmark/Resp.benchmark/ReqGenLoadBuffers.cs @@ -133,6 +133,8 @@ private bool GenerateBatch(int i, int start, int end, OpType opType) OpType.SCRIPTSET => System.Text.Encoding.ASCII.GetBytes($"*5\r\n$7\r\nEVALSHA\r\n{BenchUtils.sha1SetScript}\r\n$1\r\n1\r\n"), OpType.SCRIPTGET => System.Text.Encoding.ASCII.GetBytes($"*4\r\n$7\r\nEVALSHA\r\n{BenchUtils.sha1GetScript}\r\n$1\r\n1\r\n"), OpType.SCRIPTRETKEY => System.Text.Encoding.ASCII.GetBytes($"*4\r\n$7\r\nEVALSHA\r\n{BenchUtils.sha1RetKeyScript}\r\n$1\r\n1\r\n"), + OpType.PUBLISH => System.Text.Encoding.ASCII.GetBytes($"*3\r\n$7\r\nPUBLISH\r\n"), + OpType.SPUBLISH => System.Text.Encoding.ASCII.GetBytes($"*3\r\n$8\r\nSPUBLISH\r\n"), _ => null }; @@ -174,6 +176,8 @@ private bool GenerateBatch(int i, int start, int end, OpType opType) case OpType.SCRIPTSET: case OpType.SCRIPTGET: case OpType.SCRIPTRETKEY: + case OpType.PUBLISH: + case OpType.SPUBLISH: writeSuccess = GenerateSingleKeyValueOp(i, opHeader, start, end, opType); return writeSuccess; default: diff --git a/benchmark/Resp.benchmark/ReqGenUtils.cs b/benchmark/Resp.benchmark/ReqGenUtils.cs index 0095b15c96..be7f601c40 100644 --- a/benchmark/Resp.benchmark/ReqGenUtils.cs +++ b/benchmark/Resp.benchmark/ReqGenUtils.cs @@ -94,6 +94,8 @@ private bool WriteOp(ref byte* curr, byte* vend, OpType opType) case OpType.SCRIPTSET: case OpType.SCRIPTGET: case OpType.SCRIPTRETKEY: + case OpType.PUBLISH: + case OpType.SPUBLISH: if (!WriteKey(ref curr, vend, out keyData)) return false; break; @@ -189,6 +191,8 @@ private bool WriteOp(ref byte* curr, byte* vend, OpType opType) case OpType.MPFADD: case OpType.SET: case OpType.SCRIPTSET: + case OpType.PUBLISH: + case OpType.SPUBLISH: RandomString(); if (!WriteStringBytes(ref curr, vend, valueBuffer)) return false; diff --git a/benchmark/Resp.benchmark/RespOnlineBench.cs b/benchmark/Resp.benchmark/RespOnlineBench.cs index c7813ca3cc..8d2c1fbc8e 100644 --- a/benchmark/Resp.benchmark/RespOnlineBench.cs +++ b/benchmark/Resp.benchmark/RespOnlineBench.cs @@ -595,7 +595,6 @@ public async void OpRunnerGarnetClientSession(int thread_id) var op = SelectOpType(rand); var startTimestamp = Stopwatch.GetTimestamp(); var c = opts.Pool ? await gcsPool.GetAsync() : client; - _ = op switch { OpType.PING => await c.ExecuteAsync(["PING"]), @@ -605,6 +604,8 @@ public async void OpRunnerGarnetClientSession(int thread_id) OpType.DEL => await c.ExecuteAsync(["DEL", req.GenerateKey()]), OpType.SETBIT => await c.ExecuteAsync(["SETBIT", req.GenerateKey(), req.GenerateBitOffset()]), OpType.GETBIT => await c.ExecuteAsync(["GETBIT", req.GenerateKey(), req.GenerateBitOffset()]), + OpType.PUBLISH => await c.ExecuteAsync(["PUBLISH", req.GenerateKey(), req.GenerateValue()]), + OpType.SPUBLISH => await c.ExecuteAsync(["SPUBLISH", req.GenerateKey(), req.GenerateValue()]), OpType.ZADD => await ZADD(), OpType.ZREM => await ZREM(), OpType.ZCARD => await ZCARD(), @@ -717,6 +718,12 @@ public async void OpRunnerGarnetClientSessionParallel(int thread_id, int paralle case OpType.GETBIT: c.ExecuteBatch(["GETBIT", req.GenerateKey(), req.GenerateBitOffset()]); break; + case OpType.PUBLISH: + c.ExecuteBatch(["PUBLISH", req.GenerateKey(), req.GenerateValue()]); + break; + case OpType.SPUBLISH: + c.ExecuteBatch(["SPUBLISH", req.GenerateKey(), req.GenerateValue()]); + break; default: throw new Exception($"opType: {op} benchmark not supported with {opts.Client} ClientType!"); @@ -1046,6 +1053,12 @@ public async void OpRunnerSERedis(int thread_id) case OpType.DEL: await db.KeyDeleteAsync(req.GenerateKey()); break; + case OpType.PUBLISH: + await db.PublishAsync(RedisChannel.Literal(req.GenerateKey()), req.GenerateValue()); + break; + case OpType.SPUBLISH: + await db.ExecuteAsync("SPUBLISH", req.GenerateKey(), req.GenerateValue()); + break; case OpType.ZADD: { var key = req.GenerateKey(); @@ -1121,6 +1134,12 @@ public async void OpRunnerSERedisParallel(int thread_id, int parallel) case OpType.SET: tasks[offset++] = db.StringSetAsync(req.GenerateKey(), req.GenerateValue()); break; + case OpType.PUBLISH: + tasks[offset++] = db.PublishAsync(RedisChannel.Literal(req.GenerateKey()), req.GenerateValue()); + break; + case OpType.SPUBLISH: + tasks[offset++] = db.ExecuteAsync("SPUBLISH", req.GenerateKey(), req.GenerateValue()); + break; case OpType.SETEX: tasks[offset++] = db.StringSetAsync(req.GenerateKey(), req.GenerateValue(), TimeSpan.FromSeconds(opts.Ttl)); break; From 9db32e77361e5c6114c5415d292bae43686867fe Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 10 Dec 2024 15:43:25 -0800 Subject: [PATCH 13/31] fix check for getting most recent config instance --- libs/cluster/Server/GarnetServerNode.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index e3747a9c6c..b2071e89a4 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -158,9 +158,9 @@ byte[] GetMostRecentConfig() byte[] byteArray; if (conf != lastConfig) { - if (clusterProvider.replicationManager != null) conf.LazyUpdateLocalReplicationOffset(clusterProvider.replicationManager.ReplicationOffset); - byteArray = conf.ToByteArray(); lastConfig = conf; + if (clusterProvider.replicationManager != null) lastConfig.LazyUpdateLocalReplicationOffset(clusterProvider.replicationManager.ReplicationOffset); + byteArray = lastConfig.ToByteArray(); } else { From c9345aaf56d38cb7a648ddfbc825e9abbf68f940 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 10 Dec 2024 15:51:21 -0800 Subject: [PATCH 14/31] optimization 1: calculate candidates for published message forwarding only when config gets updated --- libs/cluster/Server/ClusterConfig.cs | 26 +++++++++---------------- libs/cluster/Server/GarnetServerNode.cs | 2 +- libs/cluster/Server/Gossip.cs | 20 +++++++++++++++++-- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/libs/cluster/Server/ClusterConfig.cs b/libs/cluster/Server/ClusterConfig.cs index f4c1717d55..fef43eb118 100644 --- a/libs/cluster/Server/ClusterConfig.cs +++ b/libs/cluster/Server/ClusterConfig.cs @@ -786,32 +786,24 @@ public List GetReplicas(string nodeid, ClusterProvider clusterProvider) } /// - /// Return the nodeIds of all nodes in the shard + /// NodeIds /// - /// - public IEnumerable GetShardNodeIds() + /// + /// + public void GetNodeIdsForPublishedMessageForwarding(out List allNodeIds, out List shardNodeIds) { var primaryId = LocalNodeRole == NodeRole.PRIMARY ? LocalNodeId : workers[1].ReplicaOfNodeId; - if (primaryId == null) yield break; - + allNodeIds = []; + shardNodeIds = []; for (ushort i = 2; i < workers.Length; i++) { + allNodeIds.Add(workers[i].Nodeid); var replicaOf = workers[i].ReplicaOfNodeId; - if ((replicaOf != null && replicaOf.Equals(primaryId, StringComparison.OrdinalIgnoreCase)) || primaryId.Equals(workers[i].Nodeid)) - yield return workers[i].Nodeid; + if (primaryId != null && ((replicaOf != null && replicaOf.Equals(primaryId, StringComparison.OrdinalIgnoreCase)) || primaryId.Equals(workers[i].Nodeid))) + shardNodeIds.Add(workers[i].Nodeid); } } - /// - /// Return the nodeIds of all nodes in the cluster - /// - /// - public IEnumerable GetAllNodeIds() - { - for (ushort i = 2; i < workers.Length; i++) - yield return workers[i].Nodeid; - } - /// /// /// diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index b2071e89a4..089406d170 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -105,7 +105,7 @@ public GarnetServerNode(ClusterProvider clusterProvider, string address, int por public Task InitializeAsync() { // Ensure initialize executes only once - if (Interlocked.CompareExchange(ref initialized, 1, 0) != 0) return Task.CompletedTask; + if (initialized != 0 || Interlocked.CompareExchange(ref initialized, 1, 0) != 0) return Task.CompletedTask; cts = CancellationTokenSource.CreateLinkedTokenSource(clusterProvider.clusterManager.ctsGossip.Token, internalCts.Token); return gc.ReconnectAsync().WaitAsync(clusterProvider.clusterManager.gossipDelay, cts.Token); diff --git a/libs/cluster/Server/Gossip.cs b/libs/cluster/Server/Gossip.cs index 8ee85699c5..8c2fba1d4c 100644 --- a/libs/cluster/Server/Gossip.cs +++ b/libs/cluster/Server/Gossip.cs @@ -28,6 +28,14 @@ internal sealed partial class ClusterManager : IDisposable readonly ConcurrentDictionary workerBanList = new(); public readonly CancellationTokenSource ctsGossip = new(); + /// + /// Last transmitted configuration + /// + ClusterConfig lastConfig = null; + + List shardIds; + List allNodeIds; + public List GetBanList() { var banlist = new List(); @@ -205,12 +213,20 @@ public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span< { GarnetServerNode gsn = null; var conf = CurrentConfig; - bool created = false; + var created = false; var _channel = channel.ToArray(); var _message = message.ToArray(); - foreach (var nodeId in cmd == RespCommand.PUBLISH ? conf.GetAllNodeIds() : conf.GetShardNodeIds()) + // Ensure we calculate the list of nodeIds only once for every update in the configuration + if (lastConfig == null || conf != lastConfig) + { + lastConfig = conf; + lastConfig.GetNodeIdsForPublishedMessageForwarding(out allNodeIds, out shardIds); + } + + var nodes = cmd == RespCommand.PUBLISH ? allNodeIds : shardIds; + foreach (var nodeId in nodes) { try { From 412ae03d23a7e2c6902ee95b8ee8afef603aa333 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Wed, 11 Dec 2024 17:20:35 -0800 Subject: [PATCH 15/31] optimization2: ignore response from cluster publish --- libs/client/GarnetClient.cs | 87 ------------------- .../GarnetClientAPI/GarnetClientExecuteAPI.cs | 6 -- libs/cluster/Server/GarnetClientExtensions.cs | 3 - libs/cluster/Server/GarnetServerNode.cs | 2 +- libs/cluster/Server/Gossip.cs | 6 ++ 5 files changed, 7 insertions(+), 97 deletions(-) diff --git a/libs/client/GarnetClient.cs b/libs/client/GarnetClient.cs index d0e7c24444..fd99ab5fb5 100644 --- a/libs/client/GarnetClient.cs +++ b/libs/client/GarnetClient.cs @@ -742,93 +742,6 @@ async ValueTask InternalExecuteAsync(TcsWrapper tcs, Memory op, Memory op, Memory subop, Memory param1, Memory param2, CancellationToken token = default) - { - int totalLen = 0; - int arraySize = 1; - - totalLen += op.Length; - - int len = subop.Length; - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; - - len = param1.Length; - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; - - len = param2.Length; - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; - - totalLen += 1 + NumUtils.NumDigits(arraySize) + 2; - - if (totalLen > networkWriter.PageSize) - { - ThrowException(new Exception($"Entry of size {totalLen} does not fit on page of size {networkWriter.PageSize}. Try increasing sendPageSize parameter to GarnetClient constructor.")); - } - - // No need for gate as this is a void return - // await InputGateAsync(token); - - try - { - networkWriter.epoch.Resume(); - - #region reserveSpaceAndWriteIntoNetworkBuffer - int taskId; - long address; - while (true) - { - token.ThrowIfCancellationRequested(); - if (!IsConnected) - { - Dispose(); - ThrowException(disposeException); - } - (taskId, address) = networkWriter.TryAllocate(totalLen, out var flushEvent); - if (address >= 0) break; - try - { - networkWriter.epoch.Suspend(); - await flushEvent.WaitAsync(token).ConfigureAwait(false); - } - finally - { - networkWriter.epoch.Resume(); - } - } - - unsafe - { - byte* curr = (byte*)networkWriter.GetPhysicalAddress(address); - byte* end = curr + totalLen; - RespWriteUtils.WriteArrayLength(arraySize, ref curr, end); - - RespWriteUtils.WriteDirect(op.Span, ref curr, end); - RespWriteUtils.WriteBulkString(subop.Span, ref curr, end); - RespWriteUtils.WriteBulkString(param1.Span, ref curr, end); - RespWriteUtils.WriteBulkString(param2.Span, ref curr, end); - Debug.Assert(curr == end); - } - #endregion - - if (!IsConnected) - { - Dispose(); - ThrowException(disposeException); - } - // Console.WriteLine($"Filled {address}-{address + totalLen}"); - networkWriter.epoch.ProtectAndDrain(); - networkWriter.DoAggressiveShiftReadOnly(); - } - finally - { - networkWriter.epoch.Suspend(); - } - return; - } - async ValueTask InternalExecuteFireAndForgetWithNoResponse(TcsWrapper tcs, Memory op, Memory subop, Memory param1, Memory param2, CancellationToken token = default) { int totalLen = 0; diff --git a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs index dad5f169f5..c0f50a6c1e 100644 --- a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs +++ b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs @@ -1084,12 +1084,6 @@ public async Task ExecuteFireAndForgetWithNoResponse(Memory op, Memo var _ = InternalExecuteFireAndForgetWithNoResponse(tcs, op, param1, param2, param3, token); return await tcs.longTcs.Task.ConfigureAwait(false); } - - public void ExecuteForNoResponse(Memory op, Memory param1, Memory param2, Memory param3, CancellationToken token = default) - { - var _ = InternalExecuteForNoResponse(op, param1, param2, param3, token); - } - #endregion void TokenRegistrationLongCallback(object s) => ((TaskCompletionSource)s).TrySetCanceled(); diff --git a/libs/cluster/Server/GarnetClientExtensions.cs b/libs/cluster/Server/GarnetClientExtensions.cs index fbb18dd4cc..a95ef4c6e1 100644 --- a/libs/cluster/Server/GarnetClientExtensions.cs +++ b/libs/cluster/Server/GarnetClientExtensions.cs @@ -64,8 +64,5 @@ public static async Task failreplicationoffset(this GarnetClient client, l public static async Task ClusterPublish(this GarnetClient client, RespCommand cmd, Memory channel, Memory message, CancellationToken cancellationToken = default) => await client.ExecuteFireAndForgetWithNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, channel, message, cancellationToken); - - //public static void ClusterPublish(this GarnetClient client, RespCommand cmd, Memory channel, Memory message, CancellationToken cancellationToken = default) - // => client.ExecuteForNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, channel, message, cancellationToken); } } \ No newline at end of file diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index 089406d170..0bddd65616 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -278,6 +278,6 @@ public ConnectionInfo GetConnectionInfo() /// /// public void TryClusterPublish(RespCommand cmd, Memory channel, Memory message) - => gc.ClusterPublish(cmd, channel, message).GetAwaiter().GetResult(); + => _ = gc.ClusterPublish(cmd, channel, message); } } \ No newline at end of file diff --git a/libs/cluster/Server/Gossip.cs b/libs/cluster/Server/Gossip.cs index 8c2fba1d4c..8dc7f5ccdf 100644 --- a/libs/cluster/Server/Gossip.cs +++ b/libs/cluster/Server/Gossip.cs @@ -209,6 +209,12 @@ public async Task TryMeetAsync(string address, int port, bool acquireLock = true } } + /// + /// Forward message by issuing CLUSTER PUBLISH|SPUBLISH + /// + /// + /// + /// public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span message) { GarnetServerNode gsn = null; From 95db9b0d2b4be5e49ee770f351ccba9cc4488d4a Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 12 Dec 2024 11:37:01 -0800 Subject: [PATCH 16/31] add publish BDN --- .../BDN.benchmark/Cluster/ClusterContext.cs | 29 ++++++++++++++++++- .../Cluster/ClusterOperations.cs | 9 ++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/benchmark/BDN.benchmark/Cluster/ClusterContext.cs b/benchmark/BDN.benchmark/Cluster/ClusterContext.cs index c6a375ff5c..c4ded2fa1b 100644 --- a/benchmark/BDN.benchmark/Cluster/ClusterContext.cs +++ b/benchmark/BDN.benchmark/Cluster/ClusterContext.cs @@ -20,6 +20,7 @@ unsafe class ClusterContext public static ReadOnlySpan keyTag => "{0}"u8; public Request[] singleGetSet; public Request[] singleMGetMSet; + public Request singlePublish; public Request singleCTXNSET; public void Dispose() @@ -68,7 +69,7 @@ public void CreateGetSet(int keySize = 8, int valueSize = 32, int batchSize = 10 benchUtils.RandomBytes(ref pairs[i].Item2); } - var setByteCount = batchSize * ("*2\r\n$3\r\nSET\r\n"u8.Length + 1 + NumUtils.NumDigits(keySize) + 2 + keySize + 2 + 1 + NumUtils.NumDigits(valueSize) + 2 + valueSize + 2); + var setByteCount = batchSize * ("*3\r\n$3\r\nSET\r\n"u8.Length + 1 + NumUtils.NumDigits(keySize) + 2 + keySize + 2 + 1 + NumUtils.NumDigits(valueSize) + 2 + valueSize + 2); var setReq = new Request(setByteCount); var curr = setReq.ptr; var end = curr + setReq.buffer.Length; @@ -163,6 +164,32 @@ public void CreateCTXNSET(int keySize = 8, int batchSize = 100) singleCTXNSET = ctxnsetReq; } + public void CreatePublish(int keySize = 8, int valueSize = 32, int batchSize = 100) + { + var pairs = new (byte[], byte[])[batchSize]; + for (var i = 0; i < batchSize; i++) + { + pairs[i] = (new byte[keySize], new byte[valueSize]); + + keyTag.CopyTo(pairs[i].Item1.AsSpan()); + benchUtils.RandomBytes(ref pairs[i].Item1, startOffset: keyTag.Length); + benchUtils.RandomBytes(ref pairs[i].Item2); + } + + var publishByteCount = batchSize * ("*3\r\n$7\r\nPUBLISH\r\n"u8.Length + 1 + NumUtils.NumDigits(keySize) + 2 + keySize + 2 + 1 + NumUtils.NumDigits(valueSize) + 2 + valueSize + 2); + var publishReq = new Request(publishByteCount); + var curr = publishReq.ptr; + var end = curr + publishReq.buffer.Length; + for (var i = 0; i < batchSize; i++) + { + _ = RespWriteUtils.WriteArrayLength(3, ref curr, end); + _ = RespWriteUtils.WriteBulkString("PUBLISH"u8, ref curr, end); + _ = RespWriteUtils.WriteBulkString(pairs[i].Item1, ref curr, end); + _ = RespWriteUtils.WriteBulkString(pairs[i].Item2, ref curr, end); + } + singlePublish = publishReq; + } + public void Consume(byte* ptr, int length) => session.TryConsumeMessages(ptr, length); } diff --git a/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs b/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs index 2799885ae9..e29c1e3cc3 100644 --- a/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs +++ b/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs @@ -37,6 +37,7 @@ public virtual void GlobalSetup() cc.CreateGetSet(); cc.CreateMGetMSet(); cc.CreateCTXNSET(); + cc.CreatePublish(); // Warmup/Prepopulate stage cc.Consume(cc.singleGetSet[1].ptr, cc.singleGetSet[1].buffer.Length); @@ -44,6 +45,8 @@ public virtual void GlobalSetup() cc.Consume(cc.singleMGetMSet[1].ptr, cc.singleMGetMSet[1].buffer.Length); // Warmup/Prepopulate stage cc.Consume(cc.singleCTXNSET.ptr, cc.singleCTXNSET.buffer.Length); + // Warmup/Prepopulate stage + cc.Consume(cc.singlePublish.ptr, cc.singlePublish.buffer.Length); } [GlobalCleanup] @@ -81,5 +84,11 @@ public void CTXNSET() { cc.Consume(cc.singleCTXNSET.ptr, cc.singleCTXNSET.buffer.Length); } + + [Benchmark] + public void Publish() + { + cc.Consume(cc.singlePublish.ptr, cc.singlePublish.buffer.Length); + } } } \ No newline at end of file From 7816c5ebb63deeeff33601f3d36c15e502ecd355 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 12 Dec 2024 14:30:21 -0800 Subject: [PATCH 17/31] minimize task creation --- libs/client/GarnetClient.cs | 71 +++++++++---------- .../GarnetClientAPI/GarnetClientExecuteAPI.cs | 6 +- libs/cluster/Server/GarnetClientExtensions.cs | 4 +- libs/cluster/Server/GarnetServerNode.cs | 2 +- 4 files changed, 39 insertions(+), 44 deletions(-) diff --git a/libs/client/GarnetClient.cs b/libs/client/GarnetClient.cs index fd99ab5fb5..8bf479347f 100644 --- a/libs/client/GarnetClient.cs +++ b/libs/client/GarnetClient.cs @@ -504,13 +504,15 @@ async ValueTask AwaitPreviousTaskAsync(int taskId) case TaskType.MemoryByteArrayAsync: if (oldTcs.memoryByteArrayTcs != null) await oldTcs.memoryByteArrayTcs.Task.ConfigureAwait(false); break; + case TaskType.LongAsync: + if (oldTcs.longTcs != null) await oldTcs.longTcs.Task.ConfigureAwait(false); + break; } } catch { if (Disposed) ThrowException(disposeException); } - await Task.Yield(); oldTcs = tcsArray[shortTaskId]; } } @@ -742,36 +744,34 @@ async ValueTask InternalExecuteAsync(TcsWrapper tcs, Memory op, Memory op, Memory subop, Memory param1, Memory param2, CancellationToken token = default) + void InternalExecuteFireAndForgetWithNoResponse(Memory op, Memory subop, Memory param1, Memory param2, CancellationToken token = default) { - int totalLen = 0; - int arraySize = 1; + var tcs = new TcsWrapper { taskType = TaskType.LongAsync, longTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) }; + var totalLen = 0; + var arraySize = 4; + totalLen += 1 + NumUtils.NumDigits(arraySize) + 2; + // op (NOTE: true length because op already resp formatted) totalLen += op.Length; - if (!subop.IsEmpty) - { - int len = subop.Length; - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; - } - if (!param1.IsEmpty) - { - int len = param1.Length; - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; - } - if (!param2.IsEmpty) + // subop + var len = subop.Length; + totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; + + // param1 + len = param1.Length; + totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; + + // param2 + len = param2.Length; + totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; + + if (totalLen > networkWriter.PageSize) { - int len = param2.Length; - totalLen += 1 + NumUtils.NumDigits(len) + 2 + len + 2; - arraySize++; + var e = new Exception($"Entry of size {totalLen} does not fit on page of size {networkWriter.PageSize}. Try increasing sendPageSize parameter to GarnetClient constructor."); + ThrowException(e); } - totalLen += 1 + NumUtils.NumDigits(arraySize) + 2; - CheckLength(totalLen, tcs); - await InputGateAsync(token); - try { networkWriter.epoch.Resume(); @@ -787,12 +787,12 @@ async ValueTask InternalExecuteFireAndForgetWithNoResponse(TcsWrapper tcs, Memor Dispose(); ThrowException(disposeException); } - (taskId, address) = networkWriter.TryAllocate(totalLen, out var flushEvent); + (taskId, address) = networkWriter.TryAllocate(totalLen, out _); if (address >= 0) break; try { networkWriter.epoch.Suspend(); - await flushEvent.WaitAsync(token).ConfigureAwait(false); + Thread.Yield(); } finally { @@ -805,24 +805,21 @@ async ValueTask InternalExecuteFireAndForgetWithNoResponse(TcsWrapper tcs, Memor unsafe { - byte* curr = (byte*)networkWriter.GetPhysicalAddress(address); - byte* end = curr + totalLen; - RespWriteUtils.WriteArrayLength(arraySize, ref curr, end); + var curr = (byte*)networkWriter.GetPhysicalAddress(address); + var end = curr + totalLen; + RespWriteUtils.WriteArrayLength(arraySize, ref curr, end); RespWriteUtils.WriteDirect(op.Span, ref curr, end); - if (!subop.IsEmpty) - RespWriteUtils.WriteBulkString(subop.Span, ref curr, end); - if (!param1.IsEmpty) - RespWriteUtils.WriteBulkString(param1.Span, ref curr, end); - if (!param2.IsEmpty) - RespWriteUtils.WriteBulkString(param2.Span, ref curr, end); + RespWriteUtils.WriteBulkString(subop.Span, ref curr, end); + RespWriteUtils.WriteBulkString(param1.Span, ref curr, end); + RespWriteUtils.WriteBulkString(param2.Span, ref curr, end); Debug.Assert(curr == end); } #endregion #region waitForEmptySlot - int shortTaskId = taskId & (maxOutstandingTasks - 1); + var shortTaskId = taskId & (maxOutstandingTasks - 1); var oldTcs = tcsArray[shortTaskId]; //1. if taskType != None, we are waiting for previous task to finish //2. if taskType == None and my taskId is not the next in line wait for previous task to acquire slot @@ -834,7 +831,7 @@ async ValueTask InternalExecuteFireAndForgetWithNoResponse(TcsWrapper tcs, Memor try { networkWriter.epoch.Suspend(); - await AwaitPreviousTaskAsync(taskId); // does not take token, as task is not cancelable at this point + AwaitPreviousTaskAsync(taskId).ConfigureAwait(false).GetAwaiter().GetResult(); // does not take token, as task is not cancelable at this point } finally { diff --git a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs index c0f50a6c1e..9817995804 100644 --- a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs +++ b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs @@ -1078,11 +1078,9 @@ public async Task ExecuteForLongResultWithCancellationAsync(Memory r /// /// /// - public async Task ExecuteFireAndForgetWithNoResponse(Memory op, Memory param1, Memory param2, Memory param3, CancellationToken token = default) + public void ExecuteFireAndForgetWithNoResponse(Memory op, Memory param1, Memory param2, Memory param3, CancellationToken token = default) { - var tcs = new TcsWrapper { taskType = TaskType.LongAsync, longTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) }; - var _ = InternalExecuteFireAndForgetWithNoResponse(tcs, op, param1, param2, param3, token); - return await tcs.longTcs.Task.ConfigureAwait(false); + InternalExecuteFireAndForgetWithNoResponse(op, param1, param2, param3, token); } #endregion diff --git a/libs/cluster/Server/GarnetClientExtensions.cs b/libs/cluster/Server/GarnetClientExtensions.cs index a95ef4c6e1..b3d0e726e2 100644 --- a/libs/cluster/Server/GarnetClientExtensions.cs +++ b/libs/cluster/Server/GarnetClientExtensions.cs @@ -62,7 +62,7 @@ public static async Task failreplicationoffset(this GarnetClient client, l return await client.ExecuteForLongResultWithCancellationAsync(GarnetClient.CLUSTER, args, cancellationToken).ConfigureAwait(false); } - public static async Task ClusterPublish(this GarnetClient client, RespCommand cmd, Memory channel, Memory message, CancellationToken cancellationToken = default) - => await client.ExecuteFireAndForgetWithNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, channel, message, cancellationToken); + public static void ClusterPublish(this GarnetClient client, RespCommand cmd, Memory channel, Memory message, CancellationToken cancellationToken = default) + => client.ExecuteFireAndForgetWithNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, channel, message, cancellationToken); } } \ No newline at end of file diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index 0bddd65616..1ccab26c34 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -278,6 +278,6 @@ public ConnectionInfo GetConnectionInfo() /// /// public void TryClusterPublish(RespCommand cmd, Memory channel, Memory message) - => _ = gc.ClusterPublish(cmd, channel, message); + => gc.ClusterPublish(cmd, channel, message); } } \ No newline at end of file From 5ef16a310bac7896a7f806720b11964fa55aaea6 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 6 Jan 2025 12:05:08 -0800 Subject: [PATCH 18/31] load no response task --- libs/client/GarnetClient.cs | 38 +++---------------- .../GarnetClientClusterCommands.cs | 4 +- .../GarnetClientAPI/GarnetClientExecuteAPI.cs | 6 +-- libs/client/GarnetClientProcessReplies.cs | 5 ++- libs/client/TcsWrapper.cs | 8 ++++ libs/cluster/Server/GarnetClientExtensions.cs | 4 +- libs/cluster/Server/GarnetServerNode.cs | 5 ++- libs/cluster/Server/Gossip.cs | 5 +-- .../Session/RespClusterBasicCommands.cs | 3 -- 9 files changed, 28 insertions(+), 50 deletions(-) diff --git a/libs/client/GarnetClient.cs b/libs/client/GarnetClient.cs index 8bf479347f..343f154bd5 100644 --- a/libs/client/GarnetClient.cs +++ b/libs/client/GarnetClient.cs @@ -744,9 +744,8 @@ async ValueTask InternalExecuteAsync(TcsWrapper tcs, Memory op, Memory op, Memory subop, Memory param1, Memory param2, CancellationToken token = default) + void InternalExecuteNoResponse(ref Memory op, ref ReadOnlySpan subop, ref Span param1, ref Span param2, CancellationToken token = default) { - var tcs = new TcsWrapper { taskType = TaskType.LongAsync, longTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) }; var totalLen = 0; var arraySize = 4; @@ -800,9 +799,6 @@ void InternalExecuteFireAndForgetWithNoResponse(Memory op, Memory su } } - // Console.WriteLine($"Allocated {taskId} @ {address}"); - tcs.nextTaskId = taskId; - unsafe { var curr = (byte*)networkWriter.GetPhysicalAddress(address); @@ -810,39 +806,17 @@ void InternalExecuteFireAndForgetWithNoResponse(Memory op, Memory su RespWriteUtils.WriteArrayLength(arraySize, ref curr, end); RespWriteUtils.WriteDirect(op.Span, ref curr, end); - RespWriteUtils.WriteBulkString(subop.Span, ref curr, end); - RespWriteUtils.WriteBulkString(param1.Span, ref curr, end); - RespWriteUtils.WriteBulkString(param2.Span, ref curr, end); + RespWriteUtils.WriteBulkString(subop, ref curr, end); + RespWriteUtils.WriteBulkString(param1, ref curr, end); + RespWriteUtils.WriteBulkString(param2, ref curr, end); Debug.Assert(curr == end); } #endregion - #region waitForEmptySlot + #region scheduleSend var shortTaskId = taskId & (maxOutstandingTasks - 1); - var oldTcs = tcsArray[shortTaskId]; - //1. if taskType != None, we are waiting for previous task to finish - //2. if taskType == None and my taskId is not the next in line wait for previous task to acquire slot - if (oldTcs.taskType != TaskType.None || !oldTcs.IsNext(taskId)) - { - // Console.WriteLine($"Before filling slot {taskId & (maxOutstandingTasks - 1)} for task {taskId} @ {address} : {tcs.taskType}"); - networkWriter.epoch.ProtectAndDrain(); - networkWriter.DoAggressiveShiftReadOnly(); - try - { - networkWriter.epoch.Suspend(); - AwaitPreviousTaskAsync(taskId).ConfigureAwait(false).GetAwaiter().GetResult(); // does not take token, as task is not cancelable at this point - } - finally - { - networkWriter.epoch.Resume(); - } - } - #endregion - - #region scheduleAwaitForResponse - // Console.WriteLine($"Filled slot {taskId & (maxOutstandingTasks - 1)} for task {taskId} @ {address} : {tcs.taskType}"); - tcsArray[shortTaskId].LoadFrom(tcs); + tcsArray[shortTaskId].LoadFrom(TaskType.NoResponse, taskId); if (Disposed) { DisposeOffset(shortTaskId); diff --git a/libs/client/GarnetClientAPI/GarnetClientClusterCommands.cs b/libs/client/GarnetClientAPI/GarnetClientClusterCommands.cs index 9db4aa1d51..4b05245be0 100644 --- a/libs/client/GarnetClientAPI/GarnetClientClusterCommands.cs +++ b/libs/client/GarnetClientAPI/GarnetClientClusterCommands.cs @@ -19,12 +19,12 @@ public sealed partial class GarnetClient /// /// PUBLISH resp formatted /// - public static readonly Memory PUBLISH = "PUBLISH"u8.ToArray(); + public static ReadOnlySpan PUBLISH => "PUBLISH"u8; /// /// PUBLISH resp formatted /// - public static readonly Memory SPUBLISH = "SPUBLISH"u8.ToArray(); + public static ReadOnlySpan SPUBLISH => "SPUBLISH"u8; /// /// Issue cluster failover command to replica node diff --git a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs index 9817995804..89bba64fb9 100644 --- a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs +++ b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs @@ -1078,10 +1078,8 @@ public async Task ExecuteForLongResultWithCancellationAsync(Memory r /// /// /// - public void ExecuteFireAndForgetWithNoResponse(Memory op, Memory param1, Memory param2, Memory param3, CancellationToken token = default) - { - InternalExecuteFireAndForgetWithNoResponse(op, param1, param2, param3, token); - } + public void ExecuteNoResponse(Memory op, ReadOnlySpan param1, ref Span param2, ref Span param3, CancellationToken token = default) + => InternalExecuteNoResponse(ref op, ref param1, ref param2, ref param3, token); #endregion void TokenRegistrationLongCallback(object s) => ((TaskCompletionSource)s).TrySetCanceled(); diff --git a/libs/client/GarnetClientProcessReplies.cs b/libs/client/GarnetClientProcessReplies.cs index 9885c98b27..c1094dbbff 100644 --- a/libs/client/GarnetClientProcessReplies.cs +++ b/libs/client/GarnetClientProcessReplies.cs @@ -231,11 +231,14 @@ unsafe int ProcessReplies(byte* recvBufferPtr, int bytesRead) Thread.Yield(); continue; } + switch (tcs.taskType) { case TaskType.None: return readHead; - + case TaskType.NoResponse: + ConsumeTcsOffset(shortTaskId); + continue; case TaskType.StringCallback: if (!ProcessReplyAsString(ref ptr, end, out var resultString, out var error)) return readHead; diff --git a/libs/client/TcsWrapper.cs b/libs/client/TcsWrapper.cs index f6804abdfb..26d2d4acda 100644 --- a/libs/client/TcsWrapper.cs +++ b/libs/client/TcsWrapper.cs @@ -22,6 +22,7 @@ enum TaskType : int MemoryByteArrayAsync, LongAsync, LongCallback, + NoResponse, } /// @@ -108,6 +109,13 @@ public void LoadFrom(TcsWrapper source) nextTaskId = source.nextTaskId; } + public void LoadFrom(TaskType taskType, int nextTaskId) + { + this.taskType = taskType; + //NOTE: prevTaskId should be set last + this.nextTaskId = nextTaskId; + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool IsNext(int taskId) => taskId == nextTaskId; } diff --git a/libs/cluster/Server/GarnetClientExtensions.cs b/libs/cluster/Server/GarnetClientExtensions.cs index b3d0e726e2..b3c9bc62cc 100644 --- a/libs/cluster/Server/GarnetClientExtensions.cs +++ b/libs/cluster/Server/GarnetClientExtensions.cs @@ -62,7 +62,7 @@ public static async Task failreplicationoffset(this GarnetClient client, l return await client.ExecuteForLongResultWithCancellationAsync(GarnetClient.CLUSTER, args, cancellationToken).ConfigureAwait(false); } - public static void ClusterPublish(this GarnetClient client, RespCommand cmd, Memory channel, Memory message, CancellationToken cancellationToken = default) - => client.ExecuteFireAndForgetWithNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, channel, message, cancellationToken); + public static void ClusterPublish(this GarnetClient client, RespCommand cmd, ref Span channel, ref Span message, CancellationToken cancellationToken = default) + => client.ExecuteNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, ref channel, ref message, cancellationToken); } } \ No newline at end of file diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index 1ccab26c34..4499e01473 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -275,9 +275,10 @@ public ConnectionInfo GetConnectionInfo() /// /// Send a CLUSTER PUBLISH message to another remote node /// + /// /// /// - public void TryClusterPublish(RespCommand cmd, Memory channel, Memory message) - => gc.ClusterPublish(cmd, channel, message); + public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span message) + => gc.ClusterPublish(cmd, ref channel, ref message); } } \ No newline at end of file diff --git a/libs/cluster/Server/Gossip.cs b/libs/cluster/Server/Gossip.cs index 8dc7f5ccdf..62d0d38100 100644 --- a/libs/cluster/Server/Gossip.cs +++ b/libs/cluster/Server/Gossip.cs @@ -221,9 +221,6 @@ public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span< var conf = CurrentConfig; var created = false; - var _channel = channel.ToArray(); - var _message = message.ToArray(); - // Ensure we calculate the list of nodeIds only once for every update in the configuration if (lastConfig == null || conf != lastConfig) { @@ -251,7 +248,7 @@ public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span< gsn.Initialize(); // Publish to remote nodes - gsn.TryClusterPublish(cmd, _channel, _message); + gsn.TryClusterPublish(cmd, ref channel, ref message); if (created && !clusterConnectionStore.AddConnection(gsn)) gsn.Dispose(); diff --git a/libs/cluster/Session/RespClusterBasicCommands.cs b/libs/cluster/Session/RespClusterBasicCommands.cs index 84704b9037..69c964a4f5 100644 --- a/libs/cluster/Session/RespClusterBasicCommands.cs +++ b/libs/cluster/Session/RespClusterBasicCommands.cs @@ -487,9 +487,6 @@ private bool NetworkClusterPublish(out bool invalidParameters) *(int*)valPtr = vSize; var numClients = clusterProvider.storeWrapper.subscribeBroker.PublishNow(keyPtr, valPtr, vSize + sizeof(int), true); - - while (!RespWriteUtils.WriteInteger(numClients, ref dcurr, dend)) - SendAndReset(); return true; } } From 8fc56d126ea301aa77802689482a7387187f705b Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 6 Jan 2025 15:59:28 -0800 Subject: [PATCH 19/31] change to pinned array --- libs/client/GarnetClient.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/client/GarnetClient.cs b/libs/client/GarnetClient.cs index 343f154bd5..78bd008d2e 100644 --- a/libs/client/GarnetClient.cs +++ b/libs/client/GarnetClient.cs @@ -162,7 +162,7 @@ public GarnetClient( this.maxOutstandingTasks = maxOutstandingTasks; this.sslOptions = tlsOptions; this.disposed = 0; - this.tcsArray = new TcsWrapper[maxOutstandingTasks]; + this.tcsArray = GC.AllocateArray(maxOutstandingTasks, pinned: true); this.memoryPool = memoryPool ?? MemoryPool.Shared; this.logger = logger; this.latency = recordLatency ? new LongHistogram(1, TimeStamp.Seconds(100), 2) : null; @@ -817,6 +817,7 @@ void InternalExecuteNoResponse(ref Memory op, ref ReadOnlySpan subop #region scheduleSend var shortTaskId = taskId & (maxOutstandingTasks - 1); tcsArray[shortTaskId].LoadFrom(TaskType.NoResponse, taskId); + if (Disposed) { DisposeOffset(shortTaskId); From bd1e3c8d490c32a2eee4720ccbd0029bdb9c8aa1 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 7 Jan 2025 11:20:33 -0800 Subject: [PATCH 20/31] simplify config update for publish forward --- libs/cluster/Server/ClusterConfig.cs | 21 ++++++++++++++------- libs/cluster/Server/Gossip.cs | 28 ++++++++-------------------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/libs/cluster/Server/ClusterConfig.cs b/libs/cluster/Server/ClusterConfig.cs index fef43eb118..0e80508c02 100644 --- a/libs/cluster/Server/ClusterConfig.cs +++ b/libs/cluster/Server/ClusterConfig.cs @@ -786,18 +786,25 @@ public List GetReplicas(string nodeid, ClusterProvider clusterProvider) } /// - /// NodeIds + /// Get all know node ids /// - /// - /// - public void GetNodeIdsForPublishedMessageForwarding(out List allNodeIds, out List shardNodeIds) + public void GetAllNodeIds(out List allNodeIds) + { + allNodeIds = new List(); + for (ushort i = 2; i < workers.Length; i++) + allNodeIds.Add(workers[i].Nodeid); + } + + /// + /// Get node-ids for nodes in the local shard + /// + /// + public void GetNodeIdsForShard(out List shardNodeIds) { var primaryId = LocalNodeRole == NodeRole.PRIMARY ? LocalNodeId : workers[1].ReplicaOfNodeId; - allNodeIds = []; - shardNodeIds = []; + shardNodeIds = new List(); for (ushort i = 2; i < workers.Length; i++) { - allNodeIds.Add(workers[i].Nodeid); var replicaOf = workers[i].ReplicaOfNodeId; if (primaryId != null && ((replicaOf != null && replicaOf.Equals(primaryId, StringComparison.OrdinalIgnoreCase)) || primaryId.Equals(workers[i].Nodeid))) shardNodeIds.Add(workers[i].Nodeid); diff --git a/libs/cluster/Server/Gossip.cs b/libs/cluster/Server/Gossip.cs index 62d0d38100..42f25426f5 100644 --- a/libs/cluster/Server/Gossip.cs +++ b/libs/cluster/Server/Gossip.cs @@ -28,14 +28,6 @@ internal sealed partial class ClusterManager : IDisposable readonly ConcurrentDictionary workerBanList = new(); public readonly CancellationTokenSource ctsGossip = new(); - /// - /// Last transmitted configuration - /// - ClusterConfig lastConfig = null; - - List shardIds; - List allNodeIds; - public List GetBanList() { var banlist = new List(); @@ -217,20 +209,16 @@ public async Task TryMeetAsync(string address, int port, bool acquireLock = true /// public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span message) { - GarnetServerNode gsn = null; var conf = CurrentConfig; - var created = false; - - // Ensure we calculate the list of nodeIds only once for every update in the configuration - if (lastConfig == null || conf != lastConfig) - { - lastConfig = conf; - lastConfig.GetNodeIdsForPublishedMessageForwarding(out allNodeIds, out shardIds); - } - - var nodes = cmd == RespCommand.PUBLISH ? allNodeIds : shardIds; - foreach (var nodeId in nodes) + List nodeIds = null; + if (cmd == RespCommand.PUBLISH) + conf.GetAllNodeIds(out nodeIds); + else + conf.GetNodeIdsForShard(out nodeIds); + foreach (var nodeId in nodeIds) { + GarnetServerNode gsn = null; + var created = false; try { if (nodeId != null) From 120edee83f8eb0151be9c842652f67b9db070891 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 7 Jan 2025 18:06:41 -0800 Subject: [PATCH 21/31] wip; wait for response vs fire and forget --- libs/client/GarnetClient.cs | 8 ++--- .../GarnetClientAPI/GarnetClientExecuteAPI.cs | 2 +- libs/client/NetworkWriter.cs | 10 +++--- libs/cluster/Server/GarnetClientExtensions.cs | 5 ++- libs/cluster/Server/GarnetServerNode.cs | 32 +++++++++++++++++-- libs/cluster/Server/Gossip.cs | 21 ++---------- .../Session/RespClusterBasicCommands.cs | 5 +-- libs/common/SingleWriterMultiReaderLock.cs | 19 +++++++++++ libs/server/PubSub/SubscribeBroker.cs | 29 +++++++++++++++++ 9 files changed, 96 insertions(+), 35 deletions(-) diff --git a/libs/client/GarnetClient.cs b/libs/client/GarnetClient.cs index 78bd008d2e..bfc021cb62 100644 --- a/libs/client/GarnetClient.cs +++ b/libs/client/GarnetClient.cs @@ -786,12 +786,12 @@ void InternalExecuteNoResponse(ref Memory op, ref ReadOnlySpan subop Dispose(); ThrowException(disposeException); } - (taskId, address) = networkWriter.TryAllocate(totalLen, out _); + (taskId, address) = networkWriter.TryAllocate(totalLen, out var flushEvent, skipTaskIdIncrement: true); if (address >= 0) break; try { networkWriter.epoch.Suspend(); - Thread.Yield(); + flushEvent.Wait(token); } finally { @@ -815,12 +815,8 @@ void InternalExecuteNoResponse(ref Memory op, ref ReadOnlySpan subop #endregion #region scheduleSend - var shortTaskId = taskId & (maxOutstandingTasks - 1); - tcsArray[shortTaskId].LoadFrom(TaskType.NoResponse, taskId); - if (Disposed) { - DisposeOffset(shortTaskId); ThrowException(disposeException); } // Console.WriteLine($"Filled {address}-{address + totalLen}"); diff --git a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs index 89bba64fb9..dbed29ec64 100644 --- a/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs +++ b/libs/client/GarnetClientAPI/GarnetClientExecuteAPI.cs @@ -1070,7 +1070,7 @@ public async Task ExecuteForLongResultWithCancellationAsync(Memory r } /// - /// Execute command (async) with cancellation token + /// Execute command expecting no response /// /// /// diff --git a/libs/client/NetworkWriter.cs b/libs/client/NetworkWriter.cs index e9654e9d98..8c9f597ec5 100644 --- a/libs/client/NetworkWriter.cs +++ b/libs/client/NetworkWriter.cs @@ -140,9 +140,10 @@ public long GetTailAddress() /// /// Number of bytes to allocate /// + /// /// The allocated logical address, or negative in case of inability to allocate [MethodImpl(MethodImplOptions.AggressiveInlining)] - private long TryAllocate(int size, out int taskId) + private long TryAllocate(int size, out int taskId, bool skipTaskIdIncrement = false) { PageOffset localTailPageOffset = default; localTailPageOffset.PageAndOffset = TailPageOffset.PageAndOffset; @@ -158,7 +159,7 @@ private long TryAllocate(int size, out int taskId) } // Determine insertion index. - localTailPageOffset.PageAndOffset = Interlocked.Add(ref TailPageOffset.PageAndOffset, size + (1L << PageOffset.kTaskOffset)); + localTailPageOffset.PageAndOffset = skipTaskIdIncrement ? Interlocked.Add(ref TailPageOffset.PageAndOffset, size) : Interlocked.Add(ref TailPageOffset.PageAndOffset, size + (1L << PageOffset.kTaskOffset)); taskId = localTailPageOffset.PrevTaskId; int page = localTailPageOffset.Page; @@ -214,9 +215,10 @@ private long TryAllocate(int size, out int taskId) /// /// Number of slots to allocate /// + /// /// The allocated logical address [MethodImpl(MethodImplOptions.AggressiveInlining)] - public (int, long) TryAllocate(int size, out CompletionEvent flushEvent) + public (int, long) TryAllocate(int size, out CompletionEvent flushEvent, bool skipTaskIdIncrement = false) { const int kFlushSpinCount = 10; var spins = 0; @@ -224,7 +226,7 @@ private long TryAllocate(int size, out int taskId) { Debug.Assert(epoch.ThisInstanceProtected()); flushEvent = this.FlushEvent; - var logicalAddress = this.TryAllocate(size, out int taskId); + var logicalAddress = this.TryAllocate(size, out int taskId, skipTaskIdIncrement: skipTaskIdIncrement); // Console.WriteLine($"Allocated {logicalAddress}-{logicalAddress + size}"); if (logicalAddress >= 0) diff --git a/libs/cluster/Server/GarnetClientExtensions.cs b/libs/cluster/Server/GarnetClientExtensions.cs index b3c9bc62cc..0cf3053a0f 100644 --- a/libs/cluster/Server/GarnetClientExtensions.cs +++ b/libs/cluster/Server/GarnetClientExtensions.cs @@ -16,6 +16,9 @@ internal static partial class GarnetClientExtensions static readonly Memory GOSSIP = "GOSSIP"u8.ToArray(); static readonly Memory WITHMEET = "WITHMEET"u8.ToArray(); + static Memory PUBLISH => "PUBLISH"u8.ToArray(); + static Memory SPUBLISH => "SPUBLISH"u8.ToArray(); + /// /// Send config /// @@ -62,7 +65,7 @@ public static async Task failreplicationoffset(this GarnetClient client, l return await client.ExecuteForLongResultWithCancellationAsync(GarnetClient.CLUSTER, args, cancellationToken).ConfigureAwait(false); } - public static void ClusterPublish(this GarnetClient client, RespCommand cmd, ref Span channel, ref Span message, CancellationToken cancellationToken = default) + public static void ClusterPublishNoResponse(this GarnetClient client, RespCommand cmd, ref Span channel, ref Span message, CancellationToken cancellationToken = default) => client.ExecuteNoResponse(GarnetClient.CLUSTER, RespCommand.PUBLISH == cmd ? GarnetClient.PUBLISH : GarnetClient.SPUBLISH, ref channel, ref message, cancellationToken); } } \ No newline at end of file diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index 4499e01473..a3593282ec 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -23,7 +23,7 @@ internal sealed class GarnetServerNode CancellationTokenSource internalCts = new(); volatile int initialized = 0; readonly ILogger logger = null; - int disposeCount = 0; + SingleWriterMultiReaderLock dispose; /// /// Last transmitted configuration @@ -88,6 +88,7 @@ public GarnetServerNode(ClusterProvider clusterProvider, string address, int por address, port, tlsOptions, sendPageSize: opts.DisablePubSub ? defaultSendPageSize : Math.Max(defaultSendPageSize, (int)opts.PubSubPageSizeBytes()), maxOutstandingTasks: opts.DisablePubSub ? defaultMaxOutstandingTask : Math.Max(defaultMaxOutstandingTask, opts.MaxPubSubTasks), + timeoutMilliseconds: opts.ClusterTimeout <= 0 ? 0 : TimeSpan.FromSeconds(opts.ClusterTimeout).Milliseconds, authUsername: clusterProvider.clusterManager.clusterProvider.ClusterUsername, authPassword: clusterProvider.clusterManager.clusterProvider.ClusterPassword, logger: logger); @@ -113,8 +114,13 @@ public Task InitializeAsync() public void Dispose() { - if (Interlocked.Increment(ref disposeCount) != 1) + // Single write lock acquisition only + if (!dispose.FinalWriteLock()) + { logger?.LogTrace("GarnetServerNode.Dispose called multiple times"); + return; + } + try { cts?.Cancel(); @@ -258,6 +264,10 @@ public bool TryGossip() return false; } + /// + /// Get connection info + /// + /// public ConnectionInfo GetConnectionInfo() { var nowTicks = DateTimeOffset.UtcNow.Ticks; @@ -279,6 +289,22 @@ public ConnectionInfo GetConnectionInfo() /// /// public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span message) - => gc.ClusterPublish(cmd, ref channel, ref message); + { + var locked = false; + try + { + if (!dispose.TryReadLock()) + { + logger?.LogWarning("Could not acquire readLock for publish forwarding"); + return; + } + locked = true; + gc.ClusterPublishNoResponse(cmd, ref channel, ref message); + } + finally + { + if (locked) dispose.ReadUnlock(); + } + } } } \ No newline at end of file diff --git a/libs/cluster/Server/Gossip.cs b/libs/cluster/Server/Gossip.cs index 42f25426f5..1a87d44f45 100644 --- a/libs/cluster/Server/Gossip.cs +++ b/libs/cluster/Server/Gossip.cs @@ -217,19 +217,12 @@ public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span< conf.GetNodeIdsForShard(out nodeIds); foreach (var nodeId in nodeIds) { - GarnetServerNode gsn = null; - var created = false; try { - if (nodeId != null) - clusterConnectionStore.GetConnection(nodeId, out gsn); + _ = clusterConnectionStore.GetConnection(nodeId, out var gsn); if (gsn == null) - { - var (address, port) = conf.GetEndpointFromNodeId(nodeId); - gsn = new GarnetServerNode(clusterProvider, address, port, tlsOptions?.TlsClientOptions, logger: logger); - created = true; - } + continue; // Initialize GarnetServerNode // Thread-Safe initialization executes only once @@ -237,18 +230,10 @@ public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span< // Publish to remote nodes gsn.TryClusterPublish(cmd, ref channel, ref message); - - if (created && !clusterConnectionStore.AddConnection(gsn)) - gsn.Dispose(); } catch (Exception ex) { - logger?.LogError(ex, $"{nameof(ClusterManager)}.{nameof(TryClusterPublish)}"); - if (created) gsn?.Dispose(); - } - finally - { - + logger?.LogWarning(ex, $"{nameof(ClusterManager)}.{nameof(TryClusterPublish)}"); } } } diff --git a/libs/cluster/Session/RespClusterBasicCommands.cs b/libs/cluster/Session/RespClusterBasicCommands.cs index 69c964a4f5..ed00d12277 100644 --- a/libs/cluster/Session/RespClusterBasicCommands.cs +++ b/libs/cluster/Session/RespClusterBasicCommands.cs @@ -293,7 +293,7 @@ private bool NetworkClusterSetConfigEpoch(out bool invalidParameters) return true; } - if (clusterProvider.clusterManager.CurrentConfig.NumWorkers > 2) + if (clusterProvider.clusterManager.CurrentConfig.NumWorkers > 1) { while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_CONFIG_EPOCH_ASSIGNMENT, ref dcurr, dend)) SendAndReset(); @@ -486,7 +486,8 @@ private bool NetworkClusterPublish(out bool invalidParameters) *(int*)keyPtr = kSize; *(int*)valPtr = vSize; - var numClients = clusterProvider.storeWrapper.subscribeBroker.PublishNow(keyPtr, valPtr, vSize + sizeof(int), true); + clusterProvider.storeWrapper.subscribeBroker.Publish(keyPtr, valPtr, vSize + sizeof(int), true); + return true; } } diff --git a/libs/common/SingleWriterMultiReaderLock.cs b/libs/common/SingleWriterMultiReaderLock.cs index 6e6b12ec2d..c201cae1f4 100644 --- a/libs/common/SingleWriterMultiReaderLock.cs +++ b/libs/common/SingleWriterMultiReaderLock.cs @@ -87,6 +87,25 @@ public void ReadUnlock() Interlocked.Decrement(ref _lock); } + /// + /// Try acquire write lock and spin wait until isWriteLocked + /// + /// Return true if current thread is the one that acquired write lock + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool FinalWriteLock() + { + while (true) + { + var isWriteLocked = IsWriteLocked; + var acquiredWriteLock = TryWriteLock(); + if (isWriteLocked || acquiredWriteLock) + { + return acquiredWriteLock; + } + Thread.Yield(); + } + } + /// public override string ToString() => _lock.ToString(); diff --git a/libs/server/PubSub/SubscribeBroker.cs b/libs/server/PubSub/SubscribeBroker.cs index 3d57b750bf..75afb319a5 100644 --- a/libs/server/PubSub/SubscribeBroker.cs +++ b/libs/server/PubSub/SubscribeBroker.cs @@ -380,6 +380,35 @@ public unsafe int PublishNow(byte* key, byte* value, int valueLength, bool ascii return numSubscribedSessions; } + /// + /// Publish the update made to key to all the subscribers, asynchronously + /// + /// key that has been updated + /// value that has been updated + /// value length that has been updated + /// is payload ascii + public unsafe void Publish(byte* key, byte* value, int valueLength, bool ascii = false) + { + if (subscriptions == null && prefixSubscriptions == null) return; + + var start = key; + ref TKey k = ref keySerializer.ReadKeyByRef(ref key); + // TODO: this needs to be a single atomic enqueue + byte[] logEntryBytes = new byte[(key - start) + valueLength + sizeof(bool)]; + fixed (byte* logEntryBytePtr = &logEntryBytes[0]) + { + byte* dst = logEntryBytePtr; + Buffer.MemoryCopy(start, dst, (key - start), (key - start)); + dst += (key - start); + Buffer.MemoryCopy(value, dst, valueLength, valueLength); + dst += valueLength; + byte* asciiPtr = (byte*)&ascii; + Buffer.MemoryCopy(asciiPtr, dst, sizeof(bool), sizeof(bool)); + } + + log.Enqueue(logEntryBytes); + } + /// /// Retrieves the collection of channels that have active subscriptions. /// From 5788de09e66d0d5fedd32e91f38edaa28d5060f7 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 13 Jan 2025 10:12:38 -0800 Subject: [PATCH 22/31] add protected connection initialization for gossip-publish --- libs/cluster/Server/ClusterConfig.cs | 12 ++-- .../Server/Failover/ReplicaFailoverSession.cs | 27 ++------- .../Server/GarnetClusterConnectionStore.cs | 55 +++++++++++++++++++ libs/cluster/Server/GarnetServerNode.cs | 2 + libs/cluster/Server/Gossip.cs | 38 ++++++++----- 5 files changed, 93 insertions(+), 41 deletions(-) diff --git a/libs/cluster/Server/ClusterConfig.cs b/libs/cluster/Server/ClusterConfig.cs index 0e80508c02..c97a66aa1d 100644 --- a/libs/cluster/Server/ClusterConfig.cs +++ b/libs/cluster/Server/ClusterConfig.cs @@ -788,26 +788,26 @@ public List GetReplicas(string nodeid, ClusterProvider clusterProvider) /// /// Get all know node ids /// - public void GetAllNodeIds(out List allNodeIds) + public void GetAllNodeIds(out List<(string, string, int)> allNodeIds) { - allNodeIds = new List(); + allNodeIds = []; for (ushort i = 2; i < workers.Length; i++) - allNodeIds.Add(workers[i].Nodeid); + allNodeIds.Add((workers[i].Nodeid, workers[i].Address, workers[i].Port)); } /// /// Get node-ids for nodes in the local shard /// /// - public void GetNodeIdsForShard(out List shardNodeIds) + public void GetNodeIdsForShard(out List<(string, string, int)> shardNodeIds) { var primaryId = LocalNodeRole == NodeRole.PRIMARY ? LocalNodeId : workers[1].ReplicaOfNodeId; - shardNodeIds = new List(); + shardNodeIds = []; for (ushort i = 2; i < workers.Length; i++) { var replicaOf = workers[i].ReplicaOfNodeId; if (primaryId != null && ((replicaOf != null && replicaOf.Equals(primaryId, StringComparison.OrdinalIgnoreCase)) || primaryId.Equals(workers[i].Nodeid))) - shardNodeIds.Add(workers[i].Nodeid); + shardNodeIds.Add((workers[i].Nodeid, workers[i].Address, workers[i].Port)); } } diff --git a/libs/cluster/Server/Failover/ReplicaFailoverSession.cs b/libs/cluster/Server/Failover/ReplicaFailoverSession.cs index dcbcc7dda9..f5994c9985 100644 --- a/libs/cluster/Server/Failover/ReplicaFailoverSession.cs +++ b/libs/cluster/Server/Failover/ReplicaFailoverSession.cs @@ -40,29 +40,14 @@ private async Task GetOrAddConnectionAsync(string nodeId) { _ = clusterProvider.clusterManager.clusterConnectionStore.GetConnection(nodeId, out var gsn); - // If connection not available try to initialize it + var (address, port) = oldConfig.GetEndpointFromNodeId(nodeId); + while (!clusterProvider.clusterManager.clusterConnectionStore.GetOrAdd(clusterProvider, address, port, clusterProvider.serverOptions.TlsOptions, nodeId, out gsn, logger: logger)) + _ = System.Threading.Thread.Yield(); + if (gsn == null) { - var (address, port) = oldConfig.GetEndpointFromNodeId(nodeId); - gsn = new GarnetServerNode( - clusterProvider, - address, - port, - clusterProvider.storeWrapper.serverOptions.TlsOptions?.TlsClientOptions, - logger: logger); - - // Try add connection to the connection store - if (!clusterProvider.clusterManager.clusterConnectionStore.AddConnection(gsn)) - { - // If failed to add dispose connection resources - gsn.Dispose(); - // Retry to get established connection if it was added after our first attempt - _ = clusterProvider.clusterManager.clusterConnectionStore.GetConnection(nodeId, out gsn); - } - - // Final check fail, if connection is not established. - if (gsn == null) - throw new GarnetException($"Connection not established to node {nodeId}"); + logger?.LogWarning("TryMeet: Could not establish connection to remote node [{nodeId} {address}:{port}] failed", nodeId, address, port); + return null; } await gsn.InitializeAsync(); diff --git a/libs/cluster/Server/GarnetClusterConnectionStore.cs b/libs/cluster/Server/GarnetClusterConnectionStore.cs index a78583a3be..6c50c2fc5e 100644 --- a/libs/cluster/Server/GarnetClusterConnectionStore.cs +++ b/libs/cluster/Server/GarnetClusterConnectionStore.cs @@ -4,6 +4,7 @@ using System; using System.Security.Cryptography; using Garnet.common; +using Garnet.server.TLS; using Microsoft.Extensions.Logging; namespace Garnet.cluster @@ -92,6 +93,60 @@ public bool AddConnection(GarnetServerNode conn) return true; } + /// + /// Get or add a connection to the store. + /// + /// + /// + /// + /// + /// + /// + /// + /// + public bool GetOrAdd(ClusterProvider clusterProvider, string address, int port, IGarnetTlsOptions tlsOptions, string nodeId, out GarnetServerNode conn, ILogger logger = null) + { + conn = null; + try + { + _lock.WriteLock(); + if (_disposed) return false; + + if (UnsafeGetConnection(nodeId, out conn)) return true; + + conn = new GarnetServerNode(clusterProvider, address, port, tlsOptions?.TlsClientOptions, logger: logger) + { + NodeId = nodeId + }; + + // Iterate array of existing connections + for (int i = 0; i < numConnection; i++) + { + var _conn = connections[i]; + if (_conn.NodeId.Equals(conn.NodeId, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + } + + if (numConnection == connections.Length) + { + var oldArray = connections; + var newArray = new GarnetServerNode[connections.Length * 2]; + Array.Copy(oldArray, newArray, oldArray.Length); + Array.Clear(oldArray); + connections = newArray; + } + connections[numConnection++] = conn; + } + finally + { + _lock.WriteUnlock(); + } + + return true; + } + public void CloseAll() { try diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index a3593282ec..40526124c1 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -293,11 +293,13 @@ public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span< var locked = false; try { + // Try to acquire dispose lock to avoid a dipose happening during publish if (!dispose.TryReadLock()) { logger?.LogWarning("Could not acquire readLock for publish forwarding"); return; } + locked = true; gc.ClusterPublishNoResponse(cmd, ref channel, ref message); } diff --git a/libs/cluster/Server/Gossip.cs b/libs/cluster/Server/Gossip.cs index 1a87d44f45..a6c9bcb573 100644 --- a/libs/cluster/Server/Gossip.cs +++ b/libs/cluster/Server/Gossip.cs @@ -210,23 +210,28 @@ public async Task TryMeetAsync(string address, int port, bool acquireLock = true public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span message) { var conf = CurrentConfig; - List nodeIds = null; + List<(string, string, int)> nodeEntries = null; if (cmd == RespCommand.PUBLISH) - conf.GetAllNodeIds(out nodeIds); + conf.GetAllNodeIds(out nodeEntries); else - conf.GetNodeIdsForShard(out nodeIds); - foreach (var nodeId in nodeIds) + conf.GetNodeIdsForShard(out nodeEntries); + foreach (var entry in nodeEntries) { try { - _ = clusterConnectionStore.GetConnection(nodeId, out var gsn); + var nodeId = entry.Item1; + var address = entry.Item2; + var port = entry.Item3; + GarnetServerNode gsn = null; + while (!clusterConnectionStore.GetOrAdd(clusterProvider, address, port, tlsOptions, nodeId, out gsn, logger: logger)) + Thread.Yield(); if (gsn == null) continue; // Initialize GarnetServerNode // Thread-Safe initialization executes only once - gsn.Initialize(); + gsn.InitializeAsync().GetAwaiter().GetResult(); // Publish to remote nodes gsn.TryClusterPublish(cmd, ref channel, ref message); @@ -295,20 +300,25 @@ async Task InitConnections() // Establish new connection only if it is not in banlist and not in dictionary if (!workerBanList.ContainsKey(nodeId) && !clusterConnectionStore.GetConnection(nodeId, out var _)) { - var gsn = new GarnetServerNode(clusterProvider, address, port, tlsOptions?.TlsClientOptions, logger: logger) - { - NodeId = nodeId - }; try { + GarnetServerNode gsn = null; + while (!clusterConnectionStore.GetOrAdd(clusterProvider, address, port, tlsOptions, nodeId, out gsn, logger: logger)) + await Task.Yield(); + + if (gsn == null) + { + logger?.LogWarning("InitConnections: Could not establish connection to remote node [{nodeId} {address}:{port}] failed", nodeId, address, port); + _ = clusterConnectionStore.TryRemove(nodeId); + continue; + } + await gsn.InitializeAsync(); - if (!clusterConnectionStore.AddConnection(gsn)) - gsn.Dispose(); } catch (Exception ex) { - logger?.LogWarning("Connection to remote node [{nodeId} {address}:{port}] failed with message:{msg}", nodeId, address, port, ex.Message); - gsn?.Dispose(); + logger?.LogWarning(ex, "InitConnections: Could not establish connection to remote node [{nodeId} {address}:{port}] failed", nodeId, address, port); + _ = clusterConnectionStore.TryRemove(nodeId); } } } From f42a1f452dfcf6ccfb6c2fc7453a46d23841606a Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 13 Jan 2025 10:17:19 -0800 Subject: [PATCH 23/31] remove BDN because not useful --- .../BDN.benchmark/Cluster/ClusterContext.cs | 27 ------------------- .../Cluster/ClusterOperations.cs | 9 ------- 2 files changed, 36 deletions(-) diff --git a/benchmark/BDN.benchmark/Cluster/ClusterContext.cs b/benchmark/BDN.benchmark/Cluster/ClusterContext.cs index c4ded2fa1b..9ade4fd41f 100644 --- a/benchmark/BDN.benchmark/Cluster/ClusterContext.cs +++ b/benchmark/BDN.benchmark/Cluster/ClusterContext.cs @@ -20,7 +20,6 @@ unsafe class ClusterContext public static ReadOnlySpan keyTag => "{0}"u8; public Request[] singleGetSet; public Request[] singleMGetMSet; - public Request singlePublish; public Request singleCTXNSET; public void Dispose() @@ -164,32 +163,6 @@ public void CreateCTXNSET(int keySize = 8, int batchSize = 100) singleCTXNSET = ctxnsetReq; } - public void CreatePublish(int keySize = 8, int valueSize = 32, int batchSize = 100) - { - var pairs = new (byte[], byte[])[batchSize]; - for (var i = 0; i < batchSize; i++) - { - pairs[i] = (new byte[keySize], new byte[valueSize]); - - keyTag.CopyTo(pairs[i].Item1.AsSpan()); - benchUtils.RandomBytes(ref pairs[i].Item1, startOffset: keyTag.Length); - benchUtils.RandomBytes(ref pairs[i].Item2); - } - - var publishByteCount = batchSize * ("*3\r\n$7\r\nPUBLISH\r\n"u8.Length + 1 + NumUtils.NumDigits(keySize) + 2 + keySize + 2 + 1 + NumUtils.NumDigits(valueSize) + 2 + valueSize + 2); - var publishReq = new Request(publishByteCount); - var curr = publishReq.ptr; - var end = curr + publishReq.buffer.Length; - for (var i = 0; i < batchSize; i++) - { - _ = RespWriteUtils.WriteArrayLength(3, ref curr, end); - _ = RespWriteUtils.WriteBulkString("PUBLISH"u8, ref curr, end); - _ = RespWriteUtils.WriteBulkString(pairs[i].Item1, ref curr, end); - _ = RespWriteUtils.WriteBulkString(pairs[i].Item2, ref curr, end); - } - singlePublish = publishReq; - } - public void Consume(byte* ptr, int length) => session.TryConsumeMessages(ptr, length); } diff --git a/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs b/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs index e29c1e3cc3..2799885ae9 100644 --- a/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs +++ b/benchmark/BDN.benchmark/Cluster/ClusterOperations.cs @@ -37,7 +37,6 @@ public virtual void GlobalSetup() cc.CreateGetSet(); cc.CreateMGetMSet(); cc.CreateCTXNSET(); - cc.CreatePublish(); // Warmup/Prepopulate stage cc.Consume(cc.singleGetSet[1].ptr, cc.singleGetSet[1].buffer.Length); @@ -45,8 +44,6 @@ public virtual void GlobalSetup() cc.Consume(cc.singleMGetMSet[1].ptr, cc.singleMGetMSet[1].buffer.Length); // Warmup/Prepopulate stage cc.Consume(cc.singleCTXNSET.ptr, cc.singleCTXNSET.buffer.Length); - // Warmup/Prepopulate stage - cc.Consume(cc.singlePublish.ptr, cc.singlePublish.buffer.Length); } [GlobalCleanup] @@ -84,11 +81,5 @@ public void CTXNSET() { cc.Consume(cc.singleCTXNSET.ptr, cc.singleCTXNSET.buffer.Length); } - - [Benchmark] - public void Publish() - { - cc.Consume(cc.singlePublish.ptr, cc.singlePublish.buffer.Length); - } } } \ No newline at end of file From e5b48b6bda09f39211fe651c1f2db435c998a0f6 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 13 Jan 2025 10:20:46 -0800 Subject: [PATCH 24/31] remove unused NoResponse flag --- libs/client/GarnetClientProcessReplies.cs | 3 --- libs/client/TcsWrapper.cs | 8 -------- 2 files changed, 11 deletions(-) diff --git a/libs/client/GarnetClientProcessReplies.cs b/libs/client/GarnetClientProcessReplies.cs index c1094dbbff..978afe2332 100644 --- a/libs/client/GarnetClientProcessReplies.cs +++ b/libs/client/GarnetClientProcessReplies.cs @@ -236,9 +236,6 @@ unsafe int ProcessReplies(byte* recvBufferPtr, int bytesRead) { case TaskType.None: return readHead; - case TaskType.NoResponse: - ConsumeTcsOffset(shortTaskId); - continue; case TaskType.StringCallback: if (!ProcessReplyAsString(ref ptr, end, out var resultString, out var error)) return readHead; diff --git a/libs/client/TcsWrapper.cs b/libs/client/TcsWrapper.cs index 26d2d4acda..f6804abdfb 100644 --- a/libs/client/TcsWrapper.cs +++ b/libs/client/TcsWrapper.cs @@ -22,7 +22,6 @@ enum TaskType : int MemoryByteArrayAsync, LongAsync, LongCallback, - NoResponse, } /// @@ -109,13 +108,6 @@ public void LoadFrom(TcsWrapper source) nextTaskId = source.nextTaskId; } - public void LoadFrom(TaskType taskType, int nextTaskId) - { - this.taskType = taskType; - //NOTE: prevTaskId should be set last - this.nextTaskId = nextTaskId; - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool IsNext(int taskId) => taskId == nextTaskId; } From eecb8678cdb02fa778c0f33cbc26b9a70651ba04 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 13 Jan 2025 10:40:13 -0800 Subject: [PATCH 25/31] remove option for tasks --- libs/cluster/Server/GarnetServerNode.cs | 2 +- libs/host/Configuration/Options.cs | 5 ----- libs/host/defaults.conf | 3 --- libs/server/Servers/ServerOptions.cs | 5 ----- 4 files changed, 1 insertion(+), 14 deletions(-) diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index 40526124c1..6d1ac6c98f 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -87,7 +87,7 @@ public GarnetServerNode(ClusterProvider clusterProvider, string address, int por this.gc = new GarnetClient( address, port, tlsOptions, sendPageSize: opts.DisablePubSub ? defaultSendPageSize : Math.Max(defaultSendPageSize, (int)opts.PubSubPageSizeBytes()), - maxOutstandingTasks: opts.DisablePubSub ? defaultMaxOutstandingTask : Math.Max(defaultMaxOutstandingTask, opts.MaxPubSubTasks), + maxOutstandingTasks: defaultMaxOutstandingTask, timeoutMilliseconds: opts.ClusterTimeout <= 0 ? 0 : TimeSpan.FromSeconds(opts.ClusterTimeout).Milliseconds, authUsername: clusterProvider.clusterManager.clusterProvider.ClusterUsername, authPassword: clusterProvider.clusterManager.clusterProvider.ClusterPassword, diff --git a/libs/host/Configuration/Options.cs b/libs/host/Configuration/Options.cs index b217271c96..331eeea5e3 100644 --- a/libs/host/Configuration/Options.cs +++ b/libs/host/Configuration/Options.cs @@ -156,10 +156,6 @@ internal sealed class Options [Option("pubsub-pagesize", Required = false, HelpText = "Page size of log used for pub/sub (rounds down to power of 2)")] public string PubSubPageSize { get; set; } - [IntRangeValidation(0, int.MaxValue)] - [Option("max-pubsub-tasks", Required = false, HelpText = "Number of outstanding forwarding pubsub tasks at any given time")] - public int MaxPubSubTasks { get; set; } - [OptionValidation] [Option("no-obj", Required = false, HelpText = "Disable support for data structure objects.")] public bool? DisableObjects { get; set; } @@ -666,7 +662,6 @@ public GarnetServerOptions GetServerOptions(ILogger logger = null) EnableIncrementalSnapshots = EnableIncrementalSnapshots.GetValueOrDefault(), DisablePubSub = DisablePubSub.GetValueOrDefault(), PubSubPageSize = PubSubPageSize, - MaxPubSubTasks = MaxPubSubTasks, DisableObjects = DisableObjects.GetValueOrDefault(), EnableCluster = EnableCluster.GetValueOrDefault(), CleanClusterConfig = CleanClusterConfig.GetValueOrDefault(), diff --git a/libs/host/defaults.conf b/libs/host/defaults.conf index 575bb54f33..01c31e5c3f 100644 --- a/libs/host/defaults.conf +++ b/libs/host/defaults.conf @@ -96,9 +96,6 @@ /* Page size of log used for pub/sub (rounds down to power of 2) */ "PubSubPageSize" : "4k", - /* Number of outstanding forwarding pubsub tasks at any given time */ - "MaxPubSubTasks" : "1024", - /* Disable support for data structure objects. */ "DisableObjects" : false, diff --git a/libs/server/Servers/ServerOptions.cs b/libs/server/Servers/ServerOptions.cs index 6a0cd60b14..92795ba60d 100644 --- a/libs/server/Servers/ServerOptions.cs +++ b/libs/server/Servers/ServerOptions.cs @@ -93,11 +93,6 @@ public class ServerOptions /// public string PubSubPageSize = "4k"; - /// - /// Number of outstanding forwarding pubsub tasks at any given time - /// - public int MaxPubSubTasks = 1 << 10; - /// /// Server bootup should fail if errors happen during bootup of AOF and checkpointing. /// From 8026a30e61789d8892ce7b4c3aebfb1653dd7072 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Mon, 13 Jan 2025 10:54:46 -0800 Subject: [PATCH 26/31] ensure noResponse for slot verification of cluster commands --- libs/cluster/Session/ClusterSession.cs | 2 +- .../SlotVerification/RespClusterSlotVerify.cs | 27 +++++++++++++++++++ libs/server/Cluster/IClusterSession.cs | 10 +++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/libs/cluster/Session/ClusterSession.cs b/libs/cluster/Session/ClusterSession.cs index 11253d644a..a75a16c634 100644 --- a/libs/cluster/Session/ClusterSession.cs +++ b/libs/cluster/Session/ClusterSession.cs @@ -79,7 +79,7 @@ public void ProcessClusterCommands(RespCommand command, ref SessionParseState pa { csvi.keyNumOffset = -1; clusterProvider.ExtractKeySpecs(commandInfo, command, ref parseState, ref csvi); - if (NetworkMultiKeySlotVerify(ref parseState, ref csvi, ref this.dcurr, ref this.dend)) + if (NetworkMultiKeySlotVerifyNoResponse(ref parseState, ref csvi, ref this.dcurr, ref this.dend)) return; } diff --git a/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs b/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs index 056c7232c9..591a38ee1d 100644 --- a/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs +++ b/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs @@ -110,6 +110,14 @@ public bool NetworkKeyArraySlotVerify(Span keys, bool readOnly, byte s return true; } + /// + /// Verify multi-key slot ownership + /// + /// + /// + /// + /// + /// public unsafe bool NetworkMultiKeySlotVerify(ref SessionParseState parseState, ref ClusterSlotVerificationInput csvi, ref byte* dcurr, ref byte* dend) { // If cluster is not enabled or a transaction is running skip slot check @@ -124,5 +132,24 @@ public unsafe bool NetworkMultiKeySlotVerify(ref SessionParseState parseState, r WriteClusterSlotVerificationMessage(config, vres, ref dcurr, ref dend); return true; } + + /// + /// Verify multi-key slot ownership without generating a response + /// + /// + /// + /// + /// + /// + public unsafe bool NetworkMultiKeySlotVerifyNoResponse(ref SessionParseState parseState, ref ClusterSlotVerificationInput csvi, ref byte* dcurr, ref byte* dend) + { + // If cluster is not enabled or a transaction is running skip slot check + if (!clusterProvider.serverOptions.EnableCluster || txnManager.state == TxnState.Running) return false; + + var config = clusterProvider.clusterManager.CurrentConfig; + var vres = MultiKeySlotVerify(config, ref parseState, ref csvi); + + return vres.state == SlotVerifiedState.OK ? false : true; + } } } \ No newline at end of file diff --git a/libs/server/Cluster/IClusterSession.cs b/libs/server/Cluster/IClusterSession.cs index a7f76a8807..9e23d4b375 100644 --- a/libs/server/Cluster/IClusterSession.cs +++ b/libs/server/Cluster/IClusterSession.cs @@ -88,6 +88,16 @@ public interface IClusterSession /// unsafe bool NetworkMultiKeySlotVerify(ref SessionParseState parseState, ref ClusterSlotVerificationInput csvi, ref byte* dcurr, ref byte* dend); + /// + /// Array slot verify with no response + /// + /// + /// + /// + /// + /// + unsafe bool NetworkMultiKeySlotVerifyNoResponse(ref SessionParseState parseState, ref ClusterSlotVerificationInput csvi, ref byte* dcurr, ref byte* dend); + /// /// Sets the user currently authenticated in this session (used for permission checks) /// From 8c17dd9d6459dbb98f5cdbffeff7cc386984ccb1 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Fri, 17 Jan 2025 14:55:02 -0800 Subject: [PATCH 27/31] add SPUBLISH and PUBLISH docs --- libs/resources/RespCommandsDocs.json | 37 ++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/libs/resources/RespCommandsDocs.json b/libs/resources/RespCommandsDocs.json index 3a3c46bdcf..b80c685149 100644 --- a/libs/resources/RespCommandsDocs.json +++ b/libs/resources/RespCommandsDocs.json @@ -6552,6 +6552,27 @@ } ] }, + { + "Command": "SPUBLISH", + "Name": "SPUBLISH", + "Summary": "Posts a message to a shard channel.", + "Group": "PubSub", + "Complexity": "O(N) where N is the number of clients subscribed to the receiving shard channel.", + "Arguments": [ + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "SHARDCHANNEL", + "DisplayText": "shardchannel", + "Type": "String" + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "MESSAGE", + "DisplayText": "message", + "Type": "String" + } + ] + }, { "Command": "STRLEN", "Name": "STRLEN", @@ -6584,6 +6605,22 @@ } ] }, + { + "Command": "SSUBSCRIBE", + "Name": "SSUBSCRIBE", + "Summary": "Listens for messages published to shard channels.", + "Group": "PubSub", + "Complexity": "O(N) where N is the number of shard channels to subscribe to.", + "Arguments": [ + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "shardchannel", + "DisplayText": "channel", + "Type": "String", + "ArgumentFlags": "Multiple" + } + ] + }, { "Command": "SUBSTR", "Name": "SUBSTR", From 4fa2ddd123dafa166852e7e4befa5b1084b74e09 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 23 Jan 2025 09:53:51 -0800 Subject: [PATCH 28/31] correct typo --- libs/cluster/Server/GarnetServerNode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index 6d1ac6c98f..6f082472e2 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -293,7 +293,7 @@ public void TryClusterPublish(RespCommand cmd, ref Span channel, ref Span< var locked = false; try { - // Try to acquire dispose lock to avoid a dipose happening during publish + // Try to acquire dispose lock to avoid a dispose during publish forwarding if (!dispose.TryReadLock()) { logger?.LogWarning("Could not acquire readLock for publish forwarding"); From 305eaf79bcb6c62775720070fe903fe6d5dd8cd6 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 28 Jan 2025 17:41:35 -0800 Subject: [PATCH 29/31] addressing comments v1 --- .../SlotVerification/RespClusterSlotVerify.cs | 2 +- libs/server/Resp/CmdStrings.cs | 2 + libs/server/Resp/PubSubCommands.cs | 4 +- .../GarnetCommandsDocs.json | 23 ++++++++++++ .../GarnetCommandsInfo.json | 37 +++++++++++++++++++ 5 files changed, 65 insertions(+), 3 deletions(-) diff --git a/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs b/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs index ae911d1646..af69ed8d2b 100644 --- a/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs +++ b/libs/cluster/Session/SlotVerification/RespClusterSlotVerify.cs @@ -149,7 +149,7 @@ public unsafe bool NetworkMultiKeySlotVerifyNoResponse(ref SessionParseState par var config = clusterProvider.clusterManager.CurrentConfig; var vres = MultiKeySlotVerify(config, ref parseState, ref csvi); - return vres.state == SlotVerifiedState.OK ? false : true; + return vres.state != SlotVerifiedState.OK; } } } \ No newline at end of file diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index 9b85e317dc..1801c193db 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -15,7 +15,9 @@ static partial class CmdStrings /// public static ReadOnlySpan CLIENT => "CLIENT"u8; public static ReadOnlySpan SUBSCRIBE => "SUBSCRIBE"u8; + public static ReadOnlySpan subscribe => "subcribe"u8; public static ReadOnlySpan SSUBSCRIBE => "SSUBSCRIBE"u8; + public static ReadOnlySpan ssubscribe => "ssubcribe"u8; public static ReadOnlySpan RUNTXP => "RUNTXP"u8; public static ReadOnlySpan GET => "GET"u8; public static ReadOnlySpan get => "get"u8; diff --git a/libs/server/Resp/PubSubCommands.cs b/libs/server/Resp/PubSubCommands.cs index 8d59b0b494..3c248dbcf1 100644 --- a/libs/server/Resp/PubSubCommands.cs +++ b/libs/server/Resp/PubSubCommands.cs @@ -174,8 +174,8 @@ private bool NetworkSUBSCRIBE(RespCommand cmd) var disabledBroker = subscribeBroker == null; var header = cmd switch { - RespCommand.SUBSCRIBE => "subscribe"u8, - RespCommand.SSUBSCRIBE => "ssubscribe"u8, + RespCommand.SUBSCRIBE => CmdStrings.subscribe, + RespCommand.SSUBSCRIBE => CmdStrings.ssubscribe, _ => throw new NotImplementedException() }; diff --git a/playground/CommandInfoUpdater/GarnetCommandsDocs.json b/playground/CommandInfoUpdater/GarnetCommandsDocs.json index 1599c22037..84c80cea0e 100644 --- a/playground/CommandInfoUpdater/GarnetCommandsDocs.json +++ b/playground/CommandInfoUpdater/GarnetCommandsDocs.json @@ -763,5 +763,28 @@ ] } ] + }, + { + "Command": "CLUSTER", + "Name": "CLUSTER", + "Summary": "A container for Redis Cluster internal commands.", + "Group": "Cluster", + "Complexity": "Depends on subcommand.", + "SubCommands": [ + { + "Command": "CLUSTER_PUBLISH", + "Name": "CLUSTER|PUBLISH", + "Summary": "Processes a forwarded published message from any node in the cluster", + "Group": "Cluster", + "Complexity": "O(1)" + }, + { + "Command": "CLUSTER_SPUBLISH", + "Name": "CLUSTER|SPUBLISH", + "Summary": "Processes a forwarded published message from a node in the same shard", + "Group": "Cluster", + "Complexity": "O(1)" + } + ] } ] \ No newline at end of file diff --git a/playground/CommandInfoUpdater/GarnetCommandsInfo.json b/playground/CommandInfoUpdater/GarnetCommandsInfo.json index b7adc23f4c..04570affc7 100644 --- a/playground/CommandInfoUpdater/GarnetCommandsInfo.json +++ b/playground/CommandInfoUpdater/GarnetCommandsInfo.json @@ -207,6 +207,43 @@ "KeySpecifications": null, "SubCommands": null }, + { + "Command": "CLUSTER_PUBLISH", + "Name": "CLUSTER|PUBLISH", + "IsInternal": true, + "Arity": 4, + "Flags": "Loading, NoScript, PubSub, Stale", + "FirstKey": 1, + "LastKey": 1, + "Step": 1, + "AclCategories": "Admin, PubSub, Slow, Garnet" + }, + { + "Command": "CLUSTER_SPUBLISH", + "Name": "CLUSTER|SPUBLISH", + "IsInternal": true, + "Arity": 4, + "Flags": "Loading, NoScript, PubSub, Stale", + "FirstKey": 1, + "LastKey": 1, + "Step": 1, + "AclCategories": "Admin, PubSub, Slow, Garnet", + "KeySpecifications": [ + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 1 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": 0, + "KeyStep": 1, + "Limit": 0 + }, + "Flags": "RO" + } + ] + }, { "Command": "CLUSTER_SEND_CKPT_FILE_SEGMENT", "Name": "CLUSTER|SEND_CKPT_FILE_SEGMENT", From bbddb199ffc1c9dbfb27586cff8b70eb14e93735 Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Tue, 28 Jan 2025 17:45:22 -0800 Subject: [PATCH 30/31] addressing comments v2 --- libs/cluster/Session/RespClusterBasicCommands.cs | 14 +------------- libs/server/PubSub/SubscribeBroker.cs | 15 +++++++++++---- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/libs/cluster/Session/RespClusterBasicCommands.cs b/libs/cluster/Session/RespClusterBasicCommands.cs index 297a25fb57..7bddbc7b0a 100644 --- a/libs/cluster/Session/RespClusterBasicCommands.cs +++ b/libs/cluster/Session/RespClusterBasicCommands.cs @@ -475,19 +475,7 @@ private bool NetworkClusterPublish(out bool invalidParameters) return true; } - var key = parseState.GetArgSliceByRef(0).SpanByte; - var val = parseState.GetArgSliceByRef(1).SpanByte; - - var keyPtr = key.ToPointer() - sizeof(int); - var valPtr = val.ToPointer() - sizeof(int); - var kSize = key.Length; - var vSize = val.Length; - - *(int*)keyPtr = kSize; - *(int*)valPtr = vSize; - - clusterProvider.storeWrapper.subscribeBroker.Publish(keyPtr, valPtr, vSize + sizeof(int), true); - + clusterProvider.storeWrapper.subscribeBroker.Publish(ref parseState, true); return true; } } diff --git a/libs/server/PubSub/SubscribeBroker.cs b/libs/server/PubSub/SubscribeBroker.cs index f63cabd220..a6c5a719d9 100644 --- a/libs/server/PubSub/SubscribeBroker.cs +++ b/libs/server/PubSub/SubscribeBroker.cs @@ -383,14 +383,21 @@ public unsafe int PublishNow(byte* key, byte* value, int valueLength, bool ascii /// /// Publish the update made to key to all the subscribers, asynchronously /// - /// key that has been updated - /// value that has been updated - /// value length that has been updated + /// ParseState for publish message /// is payload ascii - public unsafe void Publish(byte* key, byte* value, int valueLength, bool ascii = false) + public unsafe void Publish(ref SessionParseState parseState, bool ascii = false) { if (subscriptions == null && prefixSubscriptions == null) return; + var key = parseState.GetArgSliceByRef(0).ptr - sizeof(int); + var value = parseState.GetArgSliceByRef(1).ptr - sizeof(int); + + var kSize = parseState.GetArgSliceByRef(0).Length; + var vSize = parseState.GetArgSliceByRef(1).Length; + *(int*)key = kSize; + *(int*)value = vSize; + var valueLength = vSize + sizeof(int); + var start = key; ref TKey k = ref keySerializer.ReadKeyByRef(ref key); // TODO: this needs to be a single atomic enqueue From 385cea988d7ae72aa222cfe144bb0f882d547c9b Mon Sep 17 00:00:00 2001 From: Vasileios Zois Date: Thu, 30 Jan 2025 14:24:15 -0800 Subject: [PATCH 31/31] rename new lock interface to indicate clear semantics --- libs/cluster/Server/GarnetServerNode.cs | 2 +- libs/common/SingleWriterMultiReaderLock.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/cluster/Server/GarnetServerNode.cs b/libs/cluster/Server/GarnetServerNode.cs index 6f082472e2..0a735588a7 100644 --- a/libs/cluster/Server/GarnetServerNode.cs +++ b/libs/cluster/Server/GarnetServerNode.cs @@ -115,7 +115,7 @@ public Task InitializeAsync() public void Dispose() { // Single write lock acquisition only - if (!dispose.FinalWriteLock()) + if (!dispose.CloseLock()) { logger?.LogTrace("GarnetServerNode.Dispose called multiple times"); return; diff --git a/libs/common/SingleWriterMultiReaderLock.cs b/libs/common/SingleWriterMultiReaderLock.cs index c201cae1f4..c32db2876c 100644 --- a/libs/common/SingleWriterMultiReaderLock.cs +++ b/libs/common/SingleWriterMultiReaderLock.cs @@ -89,10 +89,11 @@ public void ReadUnlock() /// /// Try acquire write lock and spin wait until isWriteLocked + /// NOTE: once closed this lock should never be unlocked because is considered disposed /// /// Return true if current thread is the one that acquired write lock [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool FinalWriteLock() + public bool CloseLock() { while (true) {