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

feat: hset/hdel wrote by braft and binlog #213

Merged

Conversation

longfar-ncy
Copy link
Collaborator

@longfar-ncy longfar-ncy commented Mar 16, 2024

完成 praft 添加 binlog 和on_apply解析binlog并写入的逻辑

可以在项目目录下 运行 consistency_test.sh 检验集群是否建立成功、数据是否能同步到 follower、以及向 follower 发送写请求的leader信息回复

@github-actions github-actions bot added the ✏️ Feature New feature or request label Mar 16, 2024
src/storage/src/redis.h Outdated Show resolved Hide resolved
src/storage/src/batch.h Outdated Show resolved Hide resolved
src/praft/praft.cc Outdated Show resolved Hide resolved
@KKorpse
Copy link
Collaborator

KKorpse commented Mar 24, 2024

要不要写个 go 单测啥的?还是说后面加一个统一的简单一致性测试?

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Do you want to write a go single test or something? Or should we add a unified simple consistency test later?

@longfar-ncy
Copy link
Collaborator Author

要不要写个 go 单测啥的?还是说后面加一个统一的简单一致性测试?

有道理,我这两天先试试加个最简单的,这样也方便其他人跑

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Do you want to write a go single test or something? Or should we add a unified simple consistency test later?

It makes sense. I will try to add the simplest one in the next two days, so that it will be easier for others to run.

src/storage/include/storage/storage.h Outdated Show resolved Hide resolved
src/storage/src/storage.cc Outdated Show resolved Hide resolved
@longfar-ncy
Copy link
Collaborator Author

要不要写个 go 单测啥的?还是说后面加一个统一的简单一致性测试?

go测试不好加,goredis库里没有raft相关命令。。。要是从外部启动、建立集群、然后再测的话,其实我感觉不如脚本方便

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Do you want to write a go single test or something? Or should we add a unified simple consistency test later?

Go testing is not easy to add. There are no raft-related commands in the goredis library. . . If you start it from the outside, build a cluster, and then test it, I actually feel it is not as convenient as a script.

@AlexStocks
Copy link
Contributor

要不要写个 go 单测啥的?还是说后面加一个统一的简单一致性测试?

go测试不好加,goredis库里没有raft相关命令。。。要是从外部启动、建立集群、然后再测的话,其实我感觉不如脚本方便

直接调用 Do 接口嘛

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


Do you want to write a go single test or something? Or should we add a unified simple consistency test later?

Go testing is not easy to add. There are no raft-related commands in the goredis library. . . If you start it from the outside, establish a cluster, and then test it, I actually feel it is not as convenient as a script.

Just call the Do interface directly

@@ -64,6 +69,8 @@ struct StorageOptions {
size_t small_compaction_duration_threshold = 10000;
size_t db_instance_num = 3; // default = 3
int db_id;
AppendLogFunction append_log_function;
uint32_t raft_timeout_s = 10;
Copy link
Collaborator

Choose a reason for hiding this comment

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

默认10s会不会有点小,感觉默认应该可以写到uint32_t::max

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已改正

@@ -118,7 +119,7 @@ Status Redis::HDel(const Slice& key, const std::vector<std::string>& fields, int
}
}

rocksdb::WriteBatch batch;
auto batch = Batch::CreateBatch(this);
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.

已改正

}
} else if (s.IsNotFound()) {
*ret = 0;
return Status::OK();
} else {
return s;
}
s = db_->Write(default_write_options_, &batch);
batch->Commit();
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.

已改正

}
} else if (s.IsNotFound()) {
*ret = 0;
return Status::OK();
} else {
return s;
}
s = db_->Write(default_write_options_, &batch);
batch->Commit();
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里的commit没有取status

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

已改正

@dingxiaoshuai123 dingxiaoshuai123 merged commit 2374c2f into OpenAtomFoundation:import-braft Apr 8, 2024
2 checks passed
@Mixficsol Mixficsol mentioned this pull request Apr 15, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✏️ Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants