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: slotmigrate return not correct #2741

Merged
merged 5 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
12 changes: 7 additions & 5 deletions src/pika_migrate_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -654,11 +654,12 @@ int PikaMigrateThread::ReqMigrateOne(const std::string& key, const std::shared_p
key_type = storage::DataTypeToTag(type);
if (type == storage::DataType::kNones) {
LOG(WARNING) << "PikaMigrateThread::ReqMigrateOne key: " << key << " type: " << static_cast<int>(type) << " is illegal";
return -1;
return 0;
}

if (slot_id != slot_id_) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个判断还需要加吗?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个判断还需要加吗?

不对slot_id校验直接走下面逻辑塞进队列?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个判断还需要加吗?

不对slot_id校验直接走下面逻辑塞进队列?

嗯嗯,等于是不管这个 key 所属的 slot 是否在迁移,我都能把这个 key 迁走

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个判断还需要加吗?

不对slot_id校验直接走下面逻辑塞进队列?

嗯嗯,等于是不管这个 key 所属的 slot 是否在迁移,我都能把这个 key 迁走

如果你不是所属迁移的slot你也不会进入到这里来吧,因为你不可能是migrating状态

LOG(WARNING) << "PikaMigrateThread::ReqMigrateOne Slot : " << slot_id << " is not the migrating slot:" << slot_id_;
return -2;
return -1;
}

// if the migrate thread exit, start it
Expand All @@ -675,7 +676,7 @@ int PikaMigrateThread::ReqMigrateOne(const std::string& key, const std::shared_p
is_migrating_ = true;
usleep(100);
}
} else {
}
// check the key is migrating
std::pair<const char, std::string> kpair = std::make_pair(key_type, key);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码往前缩一格

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码往前缩一格

done

if (IsMigrating(kpair)) {
Expand All @@ -686,7 +687,6 @@ int PikaMigrateThread::ReqMigrateOne(const std::string& key, const std::shared_p
mgrtone_queue_.emplace_back(kpair);
luky116 marked this conversation as resolved.
Show resolved Hide resolved
NotifyRequestMigrate();
}
}

return 1;
}
Expand Down Expand Up @@ -934,7 +934,9 @@ void *PikaMigrateThread::ThreadMain() {
{
std::unique_lock lw(workers_mutex_);
while (!should_exit_ && is_task_success_ && send_num_ != response_num_) {
workers_cond_.wait(lw);
if (workers_cond_.wait_for(lw, std::chrono::minutes(3)) == std::cv_status::timeout) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这行代码是否还需要?可以删了?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!should_exit_ && is_task_success_ 这里我觉得不能删除 毕竟这个参数是异步赋值的如果没有赋值成功那线程就不会销毁 占用,

break;
}
}
}
LOG(INFO) << "PikaMigrateThread::ThreadMain send_num:" << send_num_ << " response_num:" << response_num_;
Expand Down
1 change: 0 additions & 1 deletion src/pika_slot_command.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1440,7 +1440,6 @@ void SlotsMgrtExecWrapperCmd::Do() {
int ret = g_pika_server->SlotsMigrateOne(key_, db_);
switch (ret) {
case 0:
case -2:
res_.AppendInteger(0);
res_.AppendInteger(0);
return;
Expand Down
Loading