chore: Change Lua embedded flags syntax (#3517)

Background

We tried to be compatible with Valkey in their support of Lua flags, but we generally failed:

We are not really compatible with Valkey because our flags are different (we reject unknown flags, and Valkey flags are unknown to us)
The #!lua syntax doesn't work with Lua (# is not a comment), so scripts written for older versions of Redis can't be used with Dragonfly (i.e. they can't add Dragonfly flags and remain compatible with older Redis versions)
Changes

Instead of the previous syntax:

#!lua flags=allow-undeclared-keys,disable-atomicity

We now use this syntax:

--!df flags=allow-undeclared-keys,disable-atomicity
It is not backwards compatible (with older versions of Dragonfly), but it should be very easy to adapt to, and doesn't suffer from the disadvantages above.

Related to #3512

src/server/multi_test.cc

@@ -747,7 +747,7 @@ TEST_F(MultiTest, ScriptFlagsCommand) {
 
 TEST_F(MultiTest, ScriptFlagsEmbedded) {
   const char* s1 = R"(
-  #!lua flags=allow-undeclared-keys
+  --!df flags=allow-undeclared-keys
   return redis.call('GET', 'random-key');
 )";
 
@@ -756,7 +756,7 @@ TEST_F(MultiTest, ScriptFlagsEmbedded) {
   EXPECT_EQ(Run({"eval", s1, "0"}), "works");
 
   const char* s2 = R"(
-  #!lua flags=this-is-an-error
+  --!df flags=this-is-an-error
   redis.call('SET', 'random-key', 'failed')
   )";
 
@@ -801,7 +801,7 @@ TEST_F(MultiTest, ScriptBadCommand) {
   const char* s2 = "redis.call('FLUSHALL'); redis.set(KEYS[1], ARGS[1]);";
   const char* s3 = "redis.acall('FLUSHALL'); redis.set(KEYS[1], ARGS[1]);";
   const char* s4 = R"(
-    #!lua flags=disable-atomicity
+    --!df flags=disable-atomicity
     redis.call('FLUSHALL');
     return "OK";
   )";
@@ -827,7 +827,7 @@ TEST_F(MultiTest, MultiEvalModeConflict) {
   }
 
   const char* s1 = R"(
-  #!lua flags=allow-undeclared-keys
+  --!df flags=allow-undeclared-keys
   return redis.call('GET', 'random-key');
 )";
 

src/server/script_mgr.cc

@@ -190,11 +190,9 @@ void ScriptMgr::ListCmd(ConnectionContext* cntx) const {
   auto rb = static_cast<RedisReplyBuilder*>(cntx->reply_builder());
   rb->StartArray(scripts.size());
   for (const auto& [sha, data] : scripts) {
-    rb->StartArray(data.orig_body.empty() ? 2 : 3);
+    rb->StartArray(2);
     rb->SendBulkString(sha);
     rb->SendBulkString(data.body);
-    if (!data.orig_body.empty())
-      rb->SendBulkString(data.orig_body);
   }
 }
 
@@ -230,20 +228,18 @@ void ScriptMgr::GCCmd(ConnectionContext* cntx) const {
   return cntx->SendOk();
 }
 
-// Check if script starts with shebang (#!lua). If present, look for flags parameter and truncate
-// it.
-io::Result<optional<ScriptMgr::ScriptParams>, GenericError> DeduceParams(string_view* body) {
-  static const regex kRegex{"^\\s*?#!lua.*?flags=([^\\s\\n\\r]*).*[\\s\\r\\n]"};
+// Check if script starts with lua flags instructions (--df flags=...).
+io::Result<optional<ScriptMgr::ScriptParams>, GenericError> DeduceParams(string_view body) {
+  static const regex kRegex{R"(^\s*?--!df flags=([^\s\n\r]*)[\s\n\r])"};
   cmatch matches;
 
-  if (!regex_search(body->data(), matches, kRegex))
+  if (!regex_search(body.data(), matches, kRegex))
     return nullopt;
 
   ScriptMgr::ScriptParams params;
   if (auto err = ScriptMgr::ScriptParams::ApplyFlags(matches.str(1), &params); err)
     return nonstd::make_unexpected(err);
 
-  *body = body->substr(matches[0].length());
   return params;
 }
 
@@ -255,7 +251,6 @@ unique_ptr<char[]> CharBufFromSV(string_view sv) {
 }
 
 io::Result<string, GenericError> ScriptMgr::Insert(string_view body, Interpreter* interpreter) {
-  // Calculate hash before removing shebang (#!lua).
   char sha_buf[64];
   Interpreter::FuncSha1(body, sha_buf);
   string_view sha{sha_buf, std::strlen(sha_buf)};
@@ -264,9 +259,7 @@ io::Result<string, GenericError> ScriptMgr::Insert(string_view body, Interpreter
     return string{sha};
   }
 
-  string_view orig_body = body;
-
-  auto params_opt = DeduceParams(&body);
+  auto params_opt = DeduceParams(body);
   if (!params_opt)
     return params_opt.get_unexpected();
   auto params = params_opt->value_or(default_params_);
@@ -295,8 +288,6 @@ io::Result<string, GenericError> ScriptMgr::Insert(string_view body, Interpreter
 
   if (!it->second.body) {
     it->second.body = CharBufFromSV(body);
-    if (body != orig_body)
-      it->second.orig_body = CharBufFromSV(orig_body);
   }
 
   UpdateScriptCaches(sha, it->second);
@@ -310,7 +301,7 @@ optional<ScriptMgr::ScriptData> ScriptMgr::Find(std::string_view sha) const {
 
   lock_guard lk{mu_};
   if (auto it = db_.find(sha); it != db_.end() && it->second.body)
-    return ScriptData{it->second, it->second.body.get(), {}};
+    return ScriptData{it->second, it->second.body.get()};
 
   return std::nullopt;
 }
@@ -356,9 +347,7 @@ vector<pair<string, ScriptMgr::ScriptData>> ScriptMgr::GetAll() const {
   res.reserve(db_.size());
   for (const auto& [sha, data] : db_) {
     string body = data.body ? string{data.body.get()} : string{};
-    string orig_body = data.orig_body ? string{data.orig_body.get()} : string{};
-    res.emplace_back(string{sha.data(), sha.size()},
-                     ScriptData{data, std::move(body), std::move(orig_body)});
+    res.emplace_back(string{sha.data(), sha.size()}, ScriptData{data, std::move(body)});
   }
 
   return res;

src/server/script_mgr.h

@@ -31,8 +31,7 @@ class ScriptMgr {
   };
 
   struct ScriptData : public ScriptParams {
-    std::string body;       // script source code present in lua interpreter
-    std::string orig_body;  // original code, before removing header and adding async
+    std::string body;  // script source code present in lua interpreter
   };
 
   struct ScriptKey : public std::array<char, 40> {

tests/dragonfly/replication_test.py

@@ -874,7 +874,7 @@ async def test_scripts(df_factory, t_master, t_replicas, num_ops, num_keys, num_
         await c_replica.execute_command(f"REPLICAOF localhost {master.port}")
         await wait_available_async(c_replica)
 
-    script = script_test_s1.format(flags=f"#!lua flags={flags}" if flags else "")
+    script = script_test_s1.format(flags=f"--!df flags={flags}" if flags else "")
     sha = await c_master.script_load(script)
 
     key_sets = [[f"{i}-{j}" for j in range(num_keys)] for i in range(num_par)]

tests/dragonfly/seeder/script-generate.lua

@@ -1,4 +1,4 @@
-#!lua flags=disable-atomicity
+--!df flags=disable-atomicity
 
 --[[
 Script for quickly generating various data

tests/dragonfly/seeder/script-hash.lua

@@ -1,4 +1,4 @@
-#!lua flags=disable-atomicity
+--!df flags=disable-atomicity
 --[[
 Script for quickly computing single 64bit hash for keys of types specified in ARGV[].
 Keys of every type are sorted lexicographically to ensure consistent order.