Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow SELECT in multi/exec if it's a noop #4146

Merged
merged 1 commit into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/server/db_slice.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@ bool DbSlice::Acquire(IntentLock::Mode mode, const KeyLockArgs& lock_args) {
if (lock_args.fps.empty()) { // Can be empty for NO_KEY_TRANSACTIONAL commands.
return true;
}
DCHECK_LT(lock_args.db_index, db_array_size());

auto& lt = db_arr_[lock_args.db_index]->trans_locks;
bool lock_acquired = true;
Expand Down
10 changes: 10 additions & 0 deletions src/server/generic_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1607,6 +1607,16 @@ void GenericFamily::Select(CmdArgList args, Transaction*, SinkReplyBuilder* buil
if (index < 0 || index >= absl::GetFlag(FLAGS_dbnum)) {
return builder->SendError(kDbIndOutOfRangeErr);
}

if (cntx->conn_state.db_index == index) {
// accept a noop.
return builder->SendOk();
}

if (cntx->conn_state.exec_info.IsRunning()) {
return builder->SendError("SELECT is not allowed in a transaction");
}

cntx->conn_state.db_index = index;
auto cb = [ns = cntx->ns, index](EngineShard* shard) {
auto& db_slice = ns->GetDbSlice(shard->shard_id());
Expand Down
4 changes: 2 additions & 2 deletions src/server/journal/executor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ void JournalExecutor::Execute(journal::ParsedEntry::CmdData& cmd) {
}

void JournalExecutor::SelectDb(DbIndex dbid) {
conn_context_.conn_state.db_index = dbid;

if (ensured_dbs_.size() <= dbid)
ensured_dbs_.resize(dbid + 1);

if (!ensured_dbs_[dbid]) {
auto cmd = BuildFromParts("SELECT", dbid);
Execute(cmd);
ensured_dbs_[dbid] = true;
} else {
conn_context_.conn_state.db_index = dbid;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/main_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
return ErrorReply{"-READONLY You can't write against a read only replica."};

if (multi_active) {
if (cmd_name == "SELECT" || absl::EndsWith(cmd_name, "SUBSCRIBE"))
if (absl::EndsWith(cmd_name, "SUBSCRIBE"))
return ErrorReply{absl::StrCat("Can not call ", cmd_name, " within a transaction")};

if (cmd_name == "WATCH" || cmd_name == "FLUSHALL" || cmd_name == "FLUSHDB")
Expand Down
6 changes: 3 additions & 3 deletions tests/dragonfly/replication_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,7 @@ async def assert_replica_reconnections(replica_inst, initial_reconnects_count):
@pytest.mark.asyncio
async def test_replication_info(df_factory: DflyInstanceFactory, df_seeder_factory, n_keys=2000):
master = df_factory.create()
replica = df_factory.create(logtostdout=True, replication_acks_interval=100)
replica = df_factory.create(replication_acks_interval=100)
df_factory.start_all([master, replica])
c_master = master.client()
c_replica = replica.client()
Expand Down Expand Up @@ -1157,8 +1157,8 @@ async def test_flushall_in_full_sync(df_factory):

@pytest.mark.asyncio
async def test_readonly_script(df_factory):
master = df_factory.create(proactor_threads=2, logtostdout=True)
replica = df_factory.create(proactor_threads=2, logtostdout=True)
master = df_factory.create(proactor_threads=2)
replica = df_factory.create(proactor_threads=2)

df_factory.start_all([master, replica])

Expand Down
Loading