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

tool support copyset snapshot #188

Merged
merged 1 commit into from
Dec 28, 2020

Conversation

bai-charisu
Copy link
Contributor

@bai-charisu bai-charisu commented Dec 15, 2020

Change-Id: Icf403e4beb5a2fb6532eae8e7de465dd085148cf

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

What is changed and how it works?

What's Changed:

How it Works:

Side effects(Breaking backward compatibility? Performance regression?):

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

src/chunkserver/cli2.cpp Outdated Show resolved Hide resolved
@@ -1383,6 +1405,28 @@ void TopologyServiceManager::GetCopySetsInCluster(
CopysetInfo *info = response->add_copysetinfos();
info->set_logicalpoolid(copyset.first);
info->set_copysetid(copyset.second);
if (request->getcsloc()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这部分代码和上面的是不是一样的?如果是一样的,最好封装一下。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这一段逻辑去掉了,改为在chunkserver里面增加打全部快照的接口

src/tools/curve_cli.cpp Outdated Show resolved Hide resolved
@@ -415,13 +415,15 @@ message GetChunkServerListInCopySetsResponse {
message CopysetInfo {
required uint32 logicalPoolId = 1;
required uint32 copysetId = 2;
repeated ChunkServerLocation csLocs = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

这里直接发两次rpc,一次获取copsetInfo, 一次获取chunkserverIno就行?

opt.timeout_ms = FLAGS_timeout_ms;
opt.max_retry = FLAGS_max_retry;
butil::Status st = curve::chunkserver::Snapshot(
FLAGS_logicalPoolId,
Copy link
Contributor

Choose a reason for hiding this comment

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

参数合法性检查?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gflag自己有合法性检查的,定义了参数为uint32的话,指定其他类型参数会报错

src/tools/curve_cli.cpp Outdated Show resolved Hide resolved
@@ -214,12 +277,21 @@ void CurveCli::PrintHelp(const std::string &cmd) {
} else if (cmd == kRemovePeerCmd || cmd == kTransferLeaderCmd) {
std::cout << "curve_ops_tool " << cmd << " -logicalPoolId=1 -copysetId=10001 -peer=127.0.0.1:8080:0 " // NOLINT
"-conf=127.0.0.1:8080:0,127.0.0.1:8081:0,127.0.0.1:8082:0 -max_retry=3 -timeout_ms=100" << std::endl; // NOLINT
} else if (cmd == kDoSnapshot) {
std::cout << "curve_ops_tool " << cmd << " -logicalPoolId=1 -copysetId=10001 -peer=127.0.0.1:8080:0 " // NOLINT
Copy link
Contributor

Choose a reason for hiding this comment

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

为什么提供了logicalpoolid和copysetid之后还要提供peer,这样使用这个命令需要先查询copyset的peer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

不通副本打快照是独立的,目前先提供这个功能,有需求给全部副本打快照再开发吧

Change-Id: Icf403e4beb5a2fb6532eae8e7de465dd085148cf
CopysetID copysetId = request->copysetid();
butil::Status st =
get_node(&node, logicPoolId, copysetId,
request->peer().address());
Copy link
Contributor

Choose a reason for hiding this comment

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

调整为两行?

@ilixiaocui ilixiaocui merged commit d0360b8 into opencurve:master Dec 28, 2020
ilixiaocui pushed a commit to ilixiaocui/curve that referenced this pull request Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants