Skip to content

Commit f75773c

Browse files
vazoisvazois
authored
Fix Latency Tracking Issue (#659)
* consider INFO and other commands as admin commands for latency tracking * cleanup process other commands * move non data commands under other commands * classify command early for latency calculation * revert bool return from process admin commands * nit * classify command for latency using commandInfo * inline Process method * revert back to implicit classification of commands for latency measurements * bump garnet version * add safety messages in code
1 parent 840997f commit f75773c

File tree

5 files changed

+108
-85
lines changed

5 files changed

+108
-85
lines changed

‎.azure/pipelines/azure-pipelines-external-release.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# 1) update the name: string below (line 6) -- this is the version for the nuget package (e.g. 1.0.0)
44
# 2) update \libs\host\GarnetServer.cs readonly string version (~line 53) -- NOTE - these two values need to be the same
55
######################################
6-
name: 1.0.22
6+
name: 1.0.23
77
trigger:
88
branches:
99
include:

‎benchmark/BDN.benchmark/Resp/RespParseStress.cs

+2
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public void GlobalSetup()
6262
{
6363
QuietMode = true,
6464
AuthSettings = authSettings,
65+
MetricsSamplingFrequency = 5,
66+
LatencyMonitor = true,
6567
};
6668
server = new EmbeddedRespServer(opt);
6769

‎libs/host/GarnetServer.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public class GarnetServer : IDisposable
4949
protected StoreWrapper storeWrapper;
5050

5151
// IMPORTANT: Keep the version in sync with .azure\pipelines\azure-pipelines-external-release.yml line ~6.
52-
readonly string version = "1.0.22";
52+
readonly string version = "1.0.23";
5353

5454
/// <summary>
5555
/// Resp protocol version

‎libs/server/Resp/AdminCommands.cs

+5-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ internal sealed unsafe partial class RespServerSession : ServerSessionBase
2222
{
2323
private void ProcessAdminCommands(RespCommand command)
2424
{
25-
hasAdminCommand = true;
26-
25+
/*
26+
* WARNING: Here is safe to add @slow commands (check how containsSlowCommand is used).
27+
*/
28+
containsSlowCommand = true;
2729
if (_authenticator.CanAuthenticate && !_authenticator.IsAuthenticated)
2830
{
2931
// If the current session is unauthenticated, we stop parsing, because no other commands are allowed
@@ -39,6 +41,7 @@ private void ProcessAdminCommands(RespCommand command)
3941
RespCommand.CONFIG_SET => NetworkCONFIG_SET(),
4042
RespCommand.FAILOVER or
4143
RespCommand.REPLICAOF or
44+
RespCommand.MIGRATE or
4245
RespCommand.SECONDARYOF => NetworkProcessClusterCommand(command),
4346
RespCommand.LATENCY_HELP => NetworkLatencyHelp(),
4447
RespCommand.LATENCY_HISTOGRAM => NetworkLatencyHistogram(),

‎libs/server/Resp/RespServerSession.cs

+99-81
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System;
55
using System.Buffers;
6-
using System.Buffers.Binary;
76
using System.Diagnostics;
87
using System.Runtime.CompilerServices;
98
using System.Runtime.InteropServices;
@@ -115,8 +114,8 @@ internal sealed unsafe partial class RespServerSession : ServerSessionBase
115114
/// </summary>
116115
public IGarnetServer Server { get; set; }
117116

118-
// Track whether the incoming network batch had some admin command
119-
bool hasAdminCommand;
117+
// Track whether the incoming network batch contains slow commands that should not be counter in NET_RS histogram
118+
bool containsSlowCommand;
120119

121120
readonly CustomCommandManagerSession customCommandManagerSession;
122121

@@ -369,10 +368,10 @@ public override int TryConsumeMessages(byte* reqBuffer, int bytesReceived)
369368
{
370369
if (latencyMetrics != null)
371370
{
372-
if (hasAdminCommand)
371+
if (containsSlowCommand)
373372
{
374373
latencyMetrics.StopAndSwitch(LatencyMetricsType.NET_RS_LAT, LatencyMetricsType.NET_RS_LAT_ADMIN);
375-
hasAdminCommand = false;
374+
containsSlowCommand = false;
376375
}
377376
else
378377
latencyMetrics.Stop(LatencyMetricsType.NET_RS_LAT);
@@ -443,6 +442,10 @@ private void ProcessMessages()
443442
SendAndReset();
444443
}
445444
}
445+
else
446+
{
447+
containsSlowCommand = true;
448+
}
446449

447450
// Advance read head variables to process the next command
448451
_origReadHead = readHead = endReadHead;
@@ -496,6 +499,10 @@ private bool MakeUpperCase(byte* ptr)
496499
private bool ProcessBasicCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi storageApi)
497500
where TGarnetApi : IGarnetApi
498501
{
502+
/*
503+
* WARNING: Do not add any command here classified as @slow!
504+
* Only @fast commands otherwise latency tracking will break for NET_RS (check how containsSlowCommand is used).
505+
*/
499506
_ = cmd switch
500507
{
501508
RespCommand.GET => NetworkGET(ref storageApi),
@@ -515,6 +522,7 @@ private bool ProcessBasicCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi st
515522
RespCommand.SETRANGE => NetworkSetRange(ref storageApi),
516523
RespCommand.GETDEL => NetworkGETDEL(ref storageApi),
517524
RespCommand.APPEND => NetworkAppend(ref storageApi),
525+
RespCommand.STRLEN => NetworkSTRLEN(ref storageApi),
518526
RespCommand.INCR => NetworkIncrement(RespCommand.INCR, ref storageApi),
519527
RespCommand.INCRBY => NetworkIncrement(RespCommand.INCRBY, ref storageApi),
520528
RespCommand.DECR => NetworkIncrement(RespCommand.DECR, ref storageApi),
@@ -524,7 +532,7 @@ private bool ProcessBasicCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi st
524532
RespCommand.BITCOUNT => NetworkStringBitCount(ref storageApi),
525533
RespCommand.BITPOS => NetworkStringBitPosition(ref storageApi),
526534
RespCommand.PUBLISH => NetworkPUBLISH(),
527-
RespCommand.PING => parseState.Count == 0 ? NetworkPING() : ProcessArrayCommands(cmd, ref storageApi),
535+
RespCommand.PING => parseState.Count == 0 ? NetworkPING() : NetworkArrayPING(),
528536
RespCommand.ASKING => NetworkASKING(),
529537
RespCommand.MULTI => NetworkMULTI(),
530538
RespCommand.EXEC => NetworkEXEC(),
@@ -534,22 +542,6 @@ private bool ProcessBasicCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi st
534542
RespCommand.RUNTXP => NetworkRUNTXP(),
535543
RespCommand.READONLY => NetworkREADONLY(),
536544
RespCommand.READWRITE => NetworkREADWRITE(),
537-
RespCommand.COMMAND => NetworkCOMMAND(),
538-
RespCommand.COMMAND_COUNT => NetworkCOMMAND_COUNT(),
539-
RespCommand.COMMAND_INFO => NetworkCOMMAND_INFO(),
540-
RespCommand.ECHO => NetworkECHO(),
541-
RespCommand.INFO => NetworkINFO(),
542-
RespCommand.HELLO => NetworkHELLO(),
543-
RespCommand.TIME => NetworkTIME(),
544-
RespCommand.FLUSHALL => NetworkFLUSHALL(),
545-
RespCommand.FLUSHDB => NetworkFLUSHDB(),
546-
RespCommand.AUTH => NetworkAUTH(),
547-
RespCommand.MEMORY_USAGE => NetworkMemoryUsage(ref storageApi),
548-
RespCommand.ACL_CAT => NetworkAclCat(),
549-
RespCommand.ACL_WHOAMI => NetworkAclWhoAmI(),
550-
RespCommand.ASYNC => NetworkASYNC(),
551-
RespCommand.MIGRATE => NetworkProcessClusterCommand(cmd),
552-
553545
_ => ProcessArrayCommands(cmd, ref storageApi)
554546
};
555547

@@ -559,6 +551,10 @@ private bool ProcessBasicCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi st
559551
private bool ProcessArrayCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi storageApi)
560552
where TGarnetApi : IGarnetApi
561553
{
554+
/*
555+
* WARNING: Do not add any command here classified as @slow!
556+
* Only @fast commands otherwise latency tracking will break for NET_RS (check how containsSlowCommand is used).
557+
*/
562558
var success = cmd switch
563559
{
564560
RespCommand.MGET => NetworkMGET(ref storageApi),
@@ -569,13 +565,6 @@ private bool ProcessArrayCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi st
569565
RespCommand.WATCH => NetworkWATCH(),
570566
RespCommand.WATCH_MS => NetworkWATCH_MS(),
571567
RespCommand.WATCH_OS => NetworkWATCH_OS(),
572-
RespCommand.STRLEN => NetworkSTRLEN(ref storageApi),
573-
RespCommand.PING => NetworkArrayPING(),
574-
//General key commands
575-
RespCommand.DBSIZE => NetworkDBSIZE(ref storageApi),
576-
RespCommand.KEYS => NetworkKEYS(ref storageApi),
577-
RespCommand.SCAN => NetworkSCAN(ref storageApi),
578-
RespCommand.TYPE => NetworkTYPE(ref storageApi),
579568
// Pub/sub commands
580569
RespCommand.SUBSCRIBE => NetworkSUBSCRIBE(),
581570
RespCommand.PSUBSCRIBE => NetworkPSUBSCRIBE(),
@@ -676,10 +665,6 @@ private bool ProcessArrayCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi st
676665
RespCommand.SUNIONSTORE => SetUnionStore(ref storageApi),
677666
RespCommand.SDIFF => SetDiff(ref storageApi),
678667
RespCommand.SDIFFSTORE => SetDiffStore(ref storageApi),
679-
// Script Commands
680-
RespCommand.SCRIPT => TrySCRIPT(),
681-
RespCommand.EVAL => TryEVAL(),
682-
RespCommand.EVALSHA => TryEVALSHA(),
683668
_ => ProcessOtherCommands(cmd, ref storageApi)
684669
};
685670
return success;
@@ -688,7 +673,48 @@ private bool ProcessArrayCommands<TGarnetApi>(RespCommand cmd, ref TGarnetApi st
688673
private bool ProcessOtherCommands<TGarnetApi>(RespCommand command, ref TGarnetApi storageApi)
689674
where TGarnetApi : IGarnetApi
690675
{
691-
if (command == RespCommand.CLIENT_ID)
676+
/*
677+
* WARNING: Here is safe to add @slow commands (check how containsSlowCommand is used).
678+
*/
679+
containsSlowCommand = true;
680+
var success = command switch
681+
{
682+
RespCommand.AUTH => NetworkAUTH(),
683+
RespCommand.MEMORY_USAGE => NetworkMemoryUsage(ref storageApi),
684+
RespCommand.CLIENT_ID => NetworkCLIENTID(),
685+
RespCommand.CLIENT_INFO => NetworkCLIENTINFO(),
686+
RespCommand.CLIENT_LIST => NetworkCLIENTLIST(),
687+
RespCommand.CLIENT_KILL => NetworkCLIENTKILL(),
688+
RespCommand.COMMAND => NetworkCOMMAND(),
689+
RespCommand.COMMAND_COUNT => NetworkCOMMAND_COUNT(),
690+
RespCommand.COMMAND_INFO => NetworkCOMMAND_INFO(),
691+
RespCommand.ECHO => NetworkECHO(),
692+
RespCommand.HELLO => NetworkHELLO(),
693+
RespCommand.TIME => NetworkTIME(),
694+
RespCommand.FLUSHALL => NetworkFLUSHALL(),
695+
RespCommand.FLUSHDB => NetworkFLUSHDB(),
696+
RespCommand.ACL_CAT => NetworkAclCat(),
697+
RespCommand.ACL_WHOAMI => NetworkAclWhoAmI(),
698+
RespCommand.ASYNC => NetworkASYNC(),
699+
RespCommand.RUNTXP => NetworkRUNTXP(),
700+
RespCommand.INFO => NetworkINFO(),
701+
RespCommand.CustomTxn => NetworkCustomTxn(),
702+
RespCommand.CustomRawStringCmd => NetworkCustomRawStringCmd(ref storageApi),
703+
RespCommand.CustomObjCmd => NetworkCustomObjCmd(ref storageApi),
704+
RespCommand.CustomProcedure => NetworkCustomProcedure(),
705+
//General key commands
706+
RespCommand.DBSIZE => NetworkDBSIZE(ref storageApi),
707+
RespCommand.KEYS => NetworkKEYS(ref storageApi),
708+
RespCommand.SCAN => NetworkSCAN(ref storageApi),
709+
RespCommand.TYPE => NetworkTYPE(ref storageApi),
710+
// Script Commands
711+
RespCommand.SCRIPT => TrySCRIPT(),
712+
RespCommand.EVAL => TryEVAL(),
713+
RespCommand.EVALSHA => TryEVALSHA(),
714+
_ => Process(command)
715+
};
716+
717+
bool NetworkCLIENTID()
692718
{
693719
if (parseState.Count != 0)
694720
{
@@ -700,28 +726,8 @@ private bool ProcessOtherCommands<TGarnetApi>(RespCommand command, ref TGarnetAp
700726

701727
return true;
702728
}
703-
else if (command == RespCommand.CLIENT_INFO)
704-
{
705-
return NetworkCLIENTINFO();
706-
}
707-
else if (command == RespCommand.CLIENT_LIST)
708-
{
709-
return NetworkCLIENTLIST();
710-
}
711-
else if (command == RespCommand.CLIENT_KILL)
712-
{
713-
return NetworkCLIENTKILL();
714-
}
715-
else if (command == RespCommand.SUBSCRIBE)
716-
{
717-
while (!RespWriteUtils.WriteInteger(1, ref dcurr, dend))
718-
SendAndReset();
719-
}
720-
else if (command == RespCommand.RUNTXP)
721-
{
722-
return NetworkRUNTXP();
723-
}
724-
else if (command == RespCommand.CustomTxn)
729+
730+
bool NetworkCustomTxn()
725731
{
726732
if (!IsCommandArityValid(currentCustomTransaction.NameStr, parseState.Count))
727733
{
@@ -732,32 +738,10 @@ private bool ProcessOtherCommands<TGarnetApi>(RespCommand command, ref TGarnetAp
732738
// Perform the operation
733739
TryTransactionProc(currentCustomTransaction.id, recvBufferPtr + readHead, recvBufferPtr + endReadHead, customCommandManagerSession.GetCustomTransactionProcedure(currentCustomTransaction.id, txnManager, scratchBufferManager).Item1);
734740
currentCustomTransaction = null;
741+
return true;
735742
}
736-
else if (command == RespCommand.CustomRawStringCmd)
737-
{
738-
if (!IsCommandArityValid(currentCustomRawStringCommand.NameStr, parseState.Count))
739-
{
740-
currentCustomRawStringCommand = null;
741-
return true;
742-
}
743-
744-
// Perform the operation
745-
TryCustomRawStringCommand(recvBufferPtr + readHead, recvBufferPtr + endReadHead, currentCustomRawStringCommand.GetRespCommand(), currentCustomRawStringCommand.expirationTicks, currentCustomRawStringCommand.type, ref storageApi);
746-
currentCustomRawStringCommand = null;
747-
}
748-
else if (command == RespCommand.CustomObjCmd)
749-
{
750-
if (!IsCommandArityValid(currentCustomObjectCommand.NameStr, parseState.Count))
751-
{
752-
currentCustomObjectCommand = null;
753-
return true;
754-
}
755743

756-
// Perform the operation
757-
TryCustomObjectCommand(recvBufferPtr + readHead, recvBufferPtr + endReadHead, currentCustomObjectCommand.GetRespCommand(), currentCustomObjectCommand.subid, currentCustomObjectCommand.type, ref storageApi);
758-
currentCustomObjectCommand = null;
759-
}
760-
else if (command == RespCommand.CustomProcedure)
744+
bool NetworkCustomProcedure()
761745
{
762746
if (!IsCommandArityValid(currentCustomProcedure.NameStr, parseState.Count))
763747
{
@@ -769,12 +753,46 @@ private bool ProcessOtherCommands<TGarnetApi>(RespCommand command, ref TGarnetAp
769753
currentCustomProcedure.CustomProcedureImpl);
770754

771755
currentCustomProcedure = null;
756+
return true;
772757
}
773-
else
758+
759+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
760+
bool Process(RespCommand command)
774761
{
775762
ProcessAdminCommands(command);
776763
return true;
777764
}
765+
766+
return success;
767+
}
768+
769+
private bool NetworkCustomRawStringCmd<TGarnetApi>(ref TGarnetApi storageApi)
770+
where TGarnetApi : IGarnetApi
771+
{
772+
if (!IsCommandArityValid(currentCustomRawStringCommand.NameStr, parseState.Count))
773+
{
774+
currentCustomRawStringCommand = null;
775+
return true;
776+
}
777+
778+
// Perform the operation
779+
TryCustomRawStringCommand(recvBufferPtr + readHead, recvBufferPtr + endReadHead, currentCustomRawStringCommand.GetRespCommand(), currentCustomRawStringCommand.expirationTicks, currentCustomRawStringCommand.type, ref storageApi);
780+
currentCustomRawStringCommand = null;
781+
return true;
782+
}
783+
784+
bool NetworkCustomObjCmd<TGarnetApi>(ref TGarnetApi storageApi)
785+
where TGarnetApi : IGarnetApi
786+
{
787+
if (!IsCommandArityValid(currentCustomObjectCommand.NameStr, parseState.Count))
788+
{
789+
currentCustomObjectCommand = null;
790+
return true;
791+
}
792+
793+
// Perform the operation
794+
TryCustomObjectCommand(recvBufferPtr + readHead, recvBufferPtr + endReadHead, currentCustomObjectCommand.GetRespCommand(), currentCustomObjectCommand.subid, currentCustomObjectCommand.type, ref storageApi);
795+
currentCustomObjectCommand = null;
778796
return true;
779797
}
780798

0 commit comments

Comments
 (0)