diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4ea1a3552..5fb1a3f4f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -159,8 +159,6 @@ jobs: FLAGS_cluster_mode=emulated FLAGS_lock_on_hashtags=true timeout 20m ctest -V -L DFLY timeout 5m ./dragonfly_test - timeout 5m ./multi_test --multi_exec_mode=1 - timeout 5m ./multi_test --multi_exec_mode=3 timeout 5m ./json_family_test --jsonpathv2=false timeout 5m ./tiered_storage_test --vmodule=db_slice=2 --logtostderr - name: Upload unit logs on failure diff --git a/src/server/main_service.cc b/src/server/main_service.cc index df22f6b8c..a33a86739 100644 --- a/src/server/main_service.cc +++ b/src/server/main_service.cc @@ -76,13 +76,13 @@ ABSL_FLAG(uint32_t, memcached_port, 0, "Memcached port"); ABSL_FLAG(uint32_t, num_shards, 0, "Number of database shards, 0 - to choose automatically"); -ABSL_FLAG(uint32_t, multi_exec_mode, 2, - "Set multi exec atomicity mode: 1 for global, 2 for locking ahead, 3 for non atomic"); +ABSL_RETIRED_FLAG(uint32_t, multi_exec_mode, 2, "DEPRECATED. Sets multi exec atomicity mode"); ABSL_FLAG(bool, multi_exec_squash, true, "Whether multi exec will squash single shard commands to optimize performance"); -ABSL_FLAG(bool, track_exec_frequencies, true, "Whether to track exec frequencies for multi exec"); +ABSL_RETIRED_FLAG(bool, track_exec_frequencies, true, + "DEPRECATED. Whether to track exec frequencies for multi exec"); ABSL_FLAG(bool, lua_resp2_legacy_float, false, "Return rounded down integers instead of floats for lua scripts with RESP2"); ABSL_FLAG(uint32_t, multi_eval_squash_buffer, 4096, "Max buffer for squashed commands per script"); @@ -651,8 +651,7 @@ Transaction::MultiMode DeduceExecMode(ExecScriptUse state, const ScriptMgr& script_mgr) { // Check if script most LIKELY has global eval transactions bool contains_global = false; - Transaction::MultiMode multi_mode = - static_cast(absl::GetFlag(FLAGS_multi_exec_mode)); + Transaction::MultiMode multi_mode = Transaction::LOCK_AHEAD; if (state == ExecScriptUse::SCRIPT_RUN) { contains_global = script_mgr.AreGlobalByDefault(); @@ -2207,10 +2206,8 @@ void Service::Exec(CmdArgList args, const CommandContext& cmd_cntx) { rb->StartArray(exec_info.body.size()); if (!exec_info.body.empty()) { - if (GetFlag(FLAGS_track_exec_frequencies)) { - string descr = CreateExecDescriptor(exec_info.body, cmd_cntx.tx->GetUniqueShardCnt()); - ServerState::tlocal()->exec_freq_count[descr]++; - } + string descr = CreateExecDescriptor(exec_info.body, cmd_cntx.tx->GetUniqueShardCnt()); + ServerState::tlocal()->exec_freq_count[descr]++; if (absl::GetFlag(FLAGS_multi_exec_squash) && state != ExecScriptUse::SCRIPT_RUN && !cntx->conn_state.tracking_info_.IsTrackingOn()) { diff --git a/src/server/multi_test.cc b/src/server/multi_test.cc index 727045900..24be97d9e 100644 --- a/src/server/multi_test.cc +++ b/src/server/multi_test.cc @@ -16,7 +16,6 @@ #include "server/test_utils.h" #include "server/transaction.h" -ABSL_DECLARE_FLAG(uint32_t, multi_exec_mode); ABSL_DECLARE_FLAG(bool, multi_exec_squash); ABSL_DECLARE_FLAG(bool, lua_auto_async); ABSL_DECLARE_FLAG(bool, lua_allow_undeclared_auto_correct); @@ -194,11 +193,6 @@ TEST_F(MultiTest, MultiSeq) { } TEST_F(MultiTest, MultiConsistent) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) { - GTEST_SKIP() << "Skipped MultiConsistent test because multi_exec_mode is non atomic"; - return; - } - Run({"mset", kKey1, "base", kKey4, "base"}); auto mset_fb = pp_->at(0)->LaunchFiber([&] { @@ -244,11 +238,6 @@ TEST_F(MultiTest, MultiConsistent) { } TEST_F(MultiTest, MultiConsistent2) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) { - GTEST_SKIP() << "Skipped MultiConsistent2 test because multi_exec_mode is non atomic"; - return; - } - const int kKeyCount = 50; const int kRuns = 50; const int kJobs = 20; @@ -589,11 +578,7 @@ TEST_F(MultiTest, MultiOOO) { auto metrics = GetMetrics(); // OOO works in LOCK_AHEAD mode. - int mode = absl::GetFlag(FLAGS_multi_exec_mode); - if (mode == Transaction::LOCK_AHEAD || mode == Transaction::NON_ATOMIC) - EXPECT_EQ(200, metrics.shard_stats.tx_ooo_total); - else - EXPECT_EQ(0, metrics.shard_stats.tx_ooo_total); + EXPECT_EQ(200, metrics.shard_stats.tx_ooo_total); } // Lua scripts lock their keys ahead and thus can run out of order. @@ -693,26 +678,11 @@ TEST_F(MultiTest, MultiCauseUnblocking) { } TEST_F(MultiTest, ExecGlobalFallback) { - absl::FlagSaver fs; - // Check global command MOVE falls back to global mode from lock ahead. - absl::SetFlag(&FLAGS_multi_exec_mode, Transaction::LOCK_AHEAD); Run({"multi"}); Run({"set", "a", "1"}); // won't run ooo, because it became part of global Run({"move", "a", "1"}); Run({"exec"}); EXPECT_EQ(1, GetMetrics().coordinator_stats.tx_global_cnt); - - ClearMetrics(); - - // Check non atomic mode does not fall back to global. - absl::SetFlag(&FLAGS_multi_exec_mode, Transaction::NON_ATOMIC); - Run({"multi"}); - Run({"set", "a", "1"}); // will run ooo(quick) - Run({"move", "a", "1"}); - Run({"exec"}); - - auto stats = GetMetrics().coordinator_stats; - EXPECT_EQ(1, stats.tx_normal_cnt); // move is global } #ifndef SANITIZERS @@ -774,11 +744,6 @@ TEST_F(MultiTest, ScriptFlagsEmbedded) { // Flaky because of https://github.com/google/sanitizers/issues/1760 #ifndef SANITIZERS TEST_F(MultiTest, UndeclaredKeyFlag) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode != Transaction::LOCK_AHEAD) { - GTEST_SKIP() << "Skipped test because multi_exec_mode is not default"; - return; - } - absl::FlagSaver fs; // lua_undeclared_keys_shas changed via CONFIG cmd below const char* script = "return redis.call('GET', 'random-key');"; @@ -829,11 +794,6 @@ TEST_F(MultiTest, ScriptBadCommand) { #endif TEST_F(MultiTest, MultiEvalModeConflict) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::GLOBAL) { - GTEST_SKIP() << "Skipped MultiEvalModeConflict test because multi_exec_mode is global"; - return; - } - const char* s1 = R"( --!df flags=allow-undeclared-keys return redis.call('GET', 'random-key'); @@ -851,11 +811,6 @@ TEST_F(MultiTest, MultiEvalModeConflict) { // Run multi-exec transactions that move values from a source list // to destination list through two contended channels. TEST_F(MultiTest, ContendedList) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) { - GTEST_SKIP() << "Skipped ContendedList test because multi_exec_mode is non atomic"; - return; - } - constexpr int listSize = 50; constexpr int stepSize = 5; @@ -896,7 +851,6 @@ TEST_F(MultiTest, ContendedList) { TEST_F(MultiTest, TestSquashing) { absl::FlagSaver fs; absl::SetFlag(&FLAGS_multi_exec_squash, true); - absl::SetFlag(&FLAGS_multi_exec_mode, Transaction::LOCK_AHEAD); const char* keys[] = {kKeySid0, kKeySid1, kKeySid2}; @@ -930,11 +884,6 @@ TEST_F(MultiTest, TestSquashing) { } TEST_F(MultiTest, MultiLeavesTxQueue) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) { - GTEST_SKIP() << "Skipped MultiLeavesTxQueue test because multi_exec_mode is non atomic"; - return; - } - // Tests the scenario, where the OOO multi-tx is scheduled into tx queue and there is another // tx (mget) after it that runs and tests for atomicity. absl::FlagSaver fs; @@ -1010,10 +959,6 @@ TEST_F(MultiTest, MultiLeavesTxQueue) { } TEST_F(MultiTest, TestLockedKeys) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode != Transaction::LOCK_AHEAD) { - GTEST_SKIP() << "Skipped TestLockedKeys test because multi_exec_mode is not lock ahead"; - return; - } auto condition = [&]() { return IsLocked(0, "key1") && IsLocked(0, "key2"); }; auto fb = ExpectConditionWithSuspension(condition); @@ -1034,8 +979,6 @@ TEST_F(MultiTest, EvalExpiration) { return; } - absl::FlagSaver fs; - absl::SetFlag(&FLAGS_multi_exec_mode, Transaction::LOCK_AHEAD); Run({"eval", "redis.call('set', 'x', 0, 'ex', 5, 'nx')", "1", "x"}); EXPECT_LE(CheckedInt({"pttl", "x"}), 5000); } @@ -1064,11 +1007,6 @@ class MultiEvalTest : public BaseFamilyTest { }; TEST_F(MultiEvalTest, MultiAllEval) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) { - GTEST_SKIP() << "Skipped MultiAllEval test because multi_exec_mode is non atomic"; - return; - } - RespExpr brpop_resp; // Run the fiber at creation. @@ -1087,10 +1025,6 @@ TEST_F(MultiEvalTest, MultiAllEval) { } TEST_F(MultiEvalTest, MultiSomeEval) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) { - GTEST_SKIP() << "Skipped MultiAllEval test because multi_exec_mode is non atomic"; - return; - } RespExpr brpop_resp; // Run the fiber at creation. @@ -1131,11 +1065,6 @@ TEST_F(MultiEvalTest, ScriptSquashingUknownCmd) { #endif TEST_F(MultiEvalTest, MultiAndEval) { - if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) { - GTEST_SKIP() << "Skipped MultiAndEval test because multi_exec_mode is non atomic"; - return; - } - // We had a bug in borrowing interpreters which caused a crash in this scenario Run({"multi"}); Run({"eval", "return redis.call('set', 'x', 'y1')", "1", "x"}); diff --git a/src/server/server_family.cc b/src/server/server_family.cc index 9efcb2b73..dea8d5d29 100644 --- a/src/server/server_family.cc +++ b/src/server/server_family.cc @@ -3087,9 +3087,6 @@ void ServerFamily::ShutdownCmd(CmdArgList args, const CommandContext& cmd_cntx) } } - service_.proactor_pool().AwaitFiberOnAll( - [](ProactorBase* pb) { ServerState::tlocal()->EnterLameDuck(); }); - CHECK_NOTNULL(acceptor_)->Stop(); cmd_cntx.rb->SendOk(); }