Fix LuaMemoryManagementMode option in .conf files (#952)

* add JSON version to memory limits configs, apparently this is broken :(

* fix memory management mode when using .conf files; unfortunate interaction with default values here

* fix option text

libs/host/Configuration/Options.cs

@@ -516,7 +516,8 @@ internal sealed class Options
         [Option("skip-rdb-restore-checksum-validation", Default = false, Required = false, HelpText = "Skip RDB restore checksum validation")]
         public bool? SkipRDBRestoreChecksumValidation { get; set; }
 
-        [Option("lua-memory-management-mode", Default = LuaMemoryManagementMode.Native, Required = false, HelpText = "Memory management mode for Lua scripts, must be set to LimittedNative or Managed to impose script limits")]
+        [OptionValidation]
+        [Option("lua-memory-management-mode", Required = false, HelpText = "Memory management mode for Lua scripts, must be set to Tracked or Managed to impose script limits")]
         public LuaMemoryManagementMode LuaMemoryManagementMode { get; set; }
 
         [MemorySizeValidation(false)]

test/Garnet.test/GarnetServerConfigTests.cs

@@ -251,75 +251,175 @@ public void ImportExportConfigAzure()
         [Test]
         public void LuaMemoryOptions()
         {
-            // Defaults to Native with no limit
+            // Command line
             {
-                var args = new[] { "--lua" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsTrue(parseSuccessful);
-                ClassicAssert.IsTrue(options.EnableLua);
-                ClassicAssert.AreEqual(LuaMemoryManagementMode.Native, options.LuaMemoryManagementMode);
-                ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
-            }
+                // Defaults to Native with no limit
+                {
+                    var args = new[] { "--lua" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Native, options.LuaMemoryManagementMode);
+                    ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
+                }
 
-            // Native with limit rejected
-            {
-                var args = new[] { "--lua", "--lua-script-memory-limit", "10m" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsFalse(parseSuccessful);
-            }
+                // Native with limit rejected
+                {
+                    var args = new[] { "--lua", "--lua-script-memory-limit", "10m" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsFalse(parseSuccessful);
+                }
 
-            // Tracked with no limit works
-            {
-                var args = new[] { "--lua", "--lua-memory-management-mode", "Tracked" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsTrue(parseSuccessful);
-                ClassicAssert.IsTrue(options.EnableLua);
-                ClassicAssert.AreEqual(LuaMemoryManagementMode.Tracked, options.LuaMemoryManagementMode);
-                ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
-            }
+                // Tracked with no limit works
+                {
+                    var args = new[] { "--lua", "--lua-memory-management-mode", "Tracked" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Tracked, options.LuaMemoryManagementMode);
+                    ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
+                }
 
-            // Tracked with limit works
-            {
-                var args = new[] { "--lua", "--lua-memory-management-mode", "Tracked", "--lua-script-memory-limit", "10m" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsTrue(parseSuccessful);
-                ClassicAssert.IsTrue(options.EnableLua);
-                ClassicAssert.AreEqual(LuaMemoryManagementMode.Tracked, options.LuaMemoryManagementMode);
-                ClassicAssert.AreEqual("10m", options.LuaScriptMemoryLimit);
-            }
+                // Tracked with limit works
+                {
+                    var args = new[] { "--lua", "--lua-memory-management-mode", "Tracked", "--lua-script-memory-limit", "10m" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Tracked, options.LuaMemoryManagementMode);
+                    ClassicAssert.AreEqual("10m", options.LuaScriptMemoryLimit);
+                }
 
-            // Tracked with bad limit rejected
-            {
-                var args = new[] { "--lua", "--lua-memory-management-mode", "Tracked", "--lua-script-memory-limit", "10Q" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsFalse(parseSuccessful);
-            }
+                // Tracked with bad limit rejected
+                {
+                    var args = new[] { "--lua", "--lua-memory-management-mode", "Tracked", "--lua-script-memory-limit", "10Q" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsFalse(parseSuccessful);
+                }
 
-            // Managed with no limit works
-            {
-                var args = new[] { "--lua", "--lua-memory-management-mode", "Managed" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsTrue(parseSuccessful);
-                ClassicAssert.IsTrue(options.EnableLua);
-                ClassicAssert.AreEqual(LuaMemoryManagementMode.Managed, options.LuaMemoryManagementMode);
-                ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
+                // Managed with no limit works
+                {
+                    var args = new[] { "--lua", "--lua-memory-management-mode", "Managed" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Managed, options.LuaMemoryManagementMode);
+                    ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
+                }
+
+                // Managed with limit works
+                {
+                    var args = new[] { "--lua", "--lua-memory-management-mode", "Managed", "--lua-script-memory-limit", "10m" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Managed, options.LuaMemoryManagementMode);
+                    ClassicAssert.AreEqual("10m", options.LuaScriptMemoryLimit);
+                }
+
+                // Managed with bad limit rejected
+                {
+                    var args = new[] { "--lua", "--lua-memory-management-mode", "Managed", "--lua-script-memory-limit", "10Q" };
+                    var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsFalse(parseSuccessful);
+                }
             }
 
-            // Managed with limit works
+            // Garnet.conf
             {
-                var args = new[] { "--lua", "--lua-memory-management-mode", "Managed", "--lua-script-memory-limit", "10m" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsTrue(parseSuccessful);
-                ClassicAssert.IsTrue(options.EnableLua);
-                ClassicAssert.AreEqual(LuaMemoryManagementMode.Managed, options.LuaMemoryManagementMode);
-                ClassicAssert.AreEqual("10m", options.LuaScriptMemoryLimit);
+                // Defaults to Native with no limit
+                {
+                    const string JSON = @"{ ""EnableLua"": true }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Native, options.LuaMemoryManagementMode);
+                    ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
+                }
+
+                // Native with limit rejected
+                {
+                    const string JSON = @"{ ""EnableLua"": true, ""LuaScriptMemoryLimit"": ""10m"" }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsFalse(parseSuccessful);
+                }
+
+                // Tracked with no limit works
+                {
+                    const string JSON = @"{ ""EnableLua"": true, ""LuaMemoryManagementMode"": ""Tracked"" }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Tracked, options.LuaMemoryManagementMode);
+                    ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
+                }
+
+                // Tracked with limit works
+                {
+                    const string JSON = @"{ ""EnableLua"": true, ""LuaMemoryManagementMode"": ""Tracked"", ""LuaScriptMemoryLimit"": ""10m"" }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Tracked, options.LuaMemoryManagementMode);
+                    ClassicAssert.AreEqual("10m", options.LuaScriptMemoryLimit);
+                }
+
+                // Tracked with bad limit rejected
+                {
+                    const string JSON = @"{ ""EnableLua"": true, ""LuaMemoryManagementMode"": ""Tracked"", ""LuaScriptMemoryLimit"": ""10Q"" }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsFalse(parseSuccessful);
+                }
+
+                // Managed with no limit works
+                {
+                    const string JSON = @"{ ""EnableLua"": true, ""LuaMemoryManagementMode"": ""Managed"" }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Managed, options.LuaMemoryManagementMode);
+                    ClassicAssert.IsNull(options.LuaScriptMemoryLimit);
+                }
+
+                // Managed with limit works
+                {
+                    const string JSON = @"{ ""EnableLua"": true, ""LuaMemoryManagementMode"": ""Managed"", ""LuaScriptMemoryLimit"": ""10m"" }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsTrue(parseSuccessful);
+                    ClassicAssert.IsTrue(options.EnableLua);
+                    ClassicAssert.AreEqual(LuaMemoryManagementMode.Managed, options.LuaMemoryManagementMode);
+                    ClassicAssert.AreEqual("10m", options.LuaScriptMemoryLimit);
+                }
+
+                // Managed with bad limit rejected
+                {
+                    const string JSON = @"{ ""EnableLua"": true, ""LuaMemoryManagementMode"": ""Managed"", ""LuaScriptMemoryLimit"": ""10Q"" }";
+                    var parseSuccessful = TryParseGarnetConfOptions(JSON, out var options, out var invalidOptions, out var exitGracefully);
+                    ClassicAssert.IsFalse(parseSuccessful);
+                }
             }
 
-            // Managed with bad limit rejected
+            // Import a garnet.conf file with the given contents
+            static bool TryParseGarnetConfOptions(string json, out Options options, out List<string> invalidOptions, out bool exitGracefully)
             {
-                var args = new[] { "--lua", "--lua-memory-management-mode", "Managed", "--lua-script-memory-limit", "10Q" };
-                var parseSuccessful = ServerSettingsManager.TryParseCommandLineArguments(args, out var options, out var invalidOptions, out var exitGracefully);
-                ClassicAssert.IsFalse(parseSuccessful);
+                var tempPath = Path.GetTempFileName();
+                try
+                {
+                    File.WriteAllText(tempPath, json);
+
+                    return ServerSettingsManager.TryParseCommandLineArguments(["--config-import-path", tempPath], out options, out invalidOptions, out exitGracefully);
+                }
+                finally
+                {
+                    try
+                    {
+                        File.Delete(tempPath);
+                    }
+                    catch
+                    {
+                        // Best effort
+                    }
+                }
             }
         }
     }