fix: Handle django-cache script so it could run in dragonfly (#6123)

automatically.

Also remove legacy flag legacy_saddex_keepttl.

Fixes #6119

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

Author: romange

src/core/string_set.cc

@@ -16,9 +16,6 @@ extern "C" {
 
 #include "base/logging.h"
 
-ABSL_FLAG(bool, legacy_saddex_keepttl, false,
-          "If true SADDEX does not update TTL for existing fields");
-
 using namespace std;
 
 namespace dfly {
@@ -93,19 +90,13 @@ unsigned StringSet::AddBatch(absl::Span<std::string_view> span, uint32_t ttl_sec
     Prefetch(hash[i]);
   }
 
-  // update ttl if legacy_saddex_keepttl is off (which is default). This variable is intended for
-  // SADDEX, but this method is called from SADD as well, where ttl is set to UINT32_MAX value,
-  // which results in has_ttl being false. This means that ObjUpdateExpireTime is never called from
-  // SADD code path even when update_ttl is true.
-  const thread_local bool update_ttl = !absl::GetFlag(FLAGS_legacy_saddex_keepttl);
-
   for (unsigned i = 0; i < count; ++i) {
     void* prev = FindInternal(&span[i], hash[i], 1);
     if (prev == nullptr) {
       ++res;
       sds field = MakeSetSds(span[i], ttl_sec);
       AddUnique(field, has_ttl, hash[i]);
-    } else if (update_ttl && has_ttl && !keepttl) {
+    } else if (has_ttl && !keepttl) {
       ObjUpdateExpireTime(prev, ttl_sec);
     }
   }

src/server/multi_test.cc

@@ -840,8 +840,6 @@ TEST_F(MultiTest, ScriptFlagsEmbedded) {
   EXPECT_THAT(Run({"eval", s2, "0"}), ErrArg("Invalid flag: this-is-an-error"));
 }
 
-// Flaky because of https://github.com/google/sanitizers/issues/1760
-#ifndef SANITIZERS
 TEST_F(MultiTest, UndeclaredKeyFlag) {
   absl::FlagSaver fs;  // lua_undeclared_keys_shas changed via CONFIG cmd below
 
@@ -890,7 +888,6 @@ TEST_F(MultiTest, ScriptBadCommand) {
   resp = Run({"eval", s4, "0"});
   EXPECT_EQ(resp, "OK");
 }
-#endif
 
 TEST_F(MultiTest, MultiEvalModeConflict) {
   const char* s1 = R"(
@@ -1254,29 +1251,6 @@ TEST_F(MultiTest, EvalShaRo) {
   EXPECT_THAT(resp, ErrArg("Write commands are not allowed from read-only scripts"));
 }
 
-TEST_F(MultiTest, ForceAtomicityFlag) {
-  absl::FlagSaver fs;
-
-  const string kHash = "bb855c2ecfa3114d222cb11e0682af6360e9712f";
-  const string_view kScript = R"(
-    --!df flags=disable-atomicity
-    redis.call('get', 'x');
-    return "OK";
-  )";
-
-  // EVAL the script works due to disable-atomicity flag
-  EXPECT_EQ(Run({"eval", kScript, "0"}), "OK");
-
-  EXPECT_THAT(Run({"script", "list"}), RespElementsAre(kHash, kScript));
-
-  // Flush scripts to force re-evaluating of flags
-  EXPECT_EQ(Run({"script", "flush"}), "OK");
-
-  // Now it doesn't work, because we force atomicity
-  absl::SetFlag(&FLAGS_lua_force_atomicity_shas, {kHash});
-  EXPECT_THAT(Run({"eval", kScript, "0"}), ErrArg("undeclared"));
-}
-
 TEST_F(MultiTest, StoredCmdBytesMetric) {
   ASSERT_EQ(GetMetrics().coordinator_stats.stored_cmd_bytes, 0);
 

src/server/script_mgr.cc

@@ -39,21 +39,10 @@ ABSL_FLAG(bool, lua_allow_undeclared_auto_correct, false,
           "undeclared key.");
 
 ABSL_FLAG(
-    std::vector<std::string>, lua_undeclared_keys_shas,
-    std::vector<std::string>({"351130589c64523cb98978dc32c64173a31244f3",    // Sidekiq, see #2442
-                              "6ae15ef4678593dc61f991c9953722d67d822776",    // Sidekiq, see #2442
-                              "34b1048274c8e50a0cc587a3ed9c383a82bb78c5",    // Sidekiq
-                              "b725ca33e5b36f318ab1150b8ac955a3d997c872"}),  // Sentry, see #5495
+    std::vector<std::string>, lua_undeclared_keys_shas, {},
     "Comma-separated list of Lua script SHAs which are allowed to access undeclared keys. SHAs are "
     "only looked at when loading the script, and new values do not affect already-loaded script.");
 
-ABSL_FLAG(std::vector<std::string>, lua_force_atomicity_shas,
-          std::vector<std::string>({
-              "f8133be7f04abd9dfefa83c3b29a9d837cfbda86",  // Sidekiq, see #4522
-          }),
-          "Comma-separated list of Lua script SHAs which are forced to run in atomic mode, even if "
-          "the script specifies disable-atomicity.");
-
 namespace dfly {
 using namespace std;
 using namespace facade;
@@ -272,17 +261,30 @@ io::Result<string, GenericError> ScriptMgr::Insert(string_view body, Interpreter
   auto params = params_opt->value_or(default_params_);
 
   if (!params.atomic) {
-    auto force_atomic_shas = absl::GetFlag(FLAGS_lua_force_atomicity_shas);
-    if (find(force_atomic_shas.begin(), force_atomic_shas.end(), sha) != force_atomic_shas.end()) {
+    // override atomicity for a specific buggy script.
+    constexpr string_view sha_4522 =
+        "f8133be7f04abd9dfefa83c3b29a9d837cfbda86"sv;  // Sidekiq, see #4522
+    if (sha == sha_4522) {
       params.atomic = true;
     }
   }
 
-  auto undeclared_shas = absl::GetFlag(FLAGS_lua_undeclared_keys_shas);
-  if (find(undeclared_shas.begin(), undeclared_shas.end(), sha) != undeclared_shas.end()) {
+  const char* kUndeclaredShas[] = {
+      "351130589c64523cb98978dc32c64173a31244f3",  // Sidekiq, see #2442
+      "6ae15ef4678593dc61f991c9953722d67d822776",  // Sidekiq, see #2442
+      "34b1048274c8e50a0cc587a3ed9c383a82bb78c5",  // Sidekiq
+      "b725ca33e5b36f318ab1150b8ac955a3d997c872",  // Sentry, see #5495
+      "8c4dafdf9b6b7bcf511a0d1ec0518bed9260e16d",  // django-ops see #6119
+  };
+
+  if (find(begin(kUndeclaredShas), end(kUndeclaredShas), sha) != end(kUndeclaredShas)) {
     params.undeclared_keys = true;
+  } else {
+    auto undeclared_shas = absl::GetFlag(FLAGS_lua_undeclared_keys_shas);
+    if (find(undeclared_shas.begin(), undeclared_shas.end(), sha) != undeclared_shas.end()) {
+      params.undeclared_keys = true;
+    }
   }
-
   // If the script is atomic, check for possible squashing optimizations.
   // For non atomic modes, squashing increases the time locks are held, which
   // can decrease throughput with frequently accessed keys.