Skip to content

Curvebs stop volume snapshot#2794

Merged
caoxianfei1 merged 1 commit intoopencurve:masterfrom
ZackSoul:new-curvebs-stop-snapshot
Nov 14, 2023
Merged

Curvebs stop volume snapshot#2794
caoxianfei1 merged 1 commit intoopencurve:masterfrom
ZackSoul:new-curvebs-stop-snapshot

Conversation

@ZackSoul
Copy link
Copy Markdown
Contributor

@ZackSoul ZackSoul commented Oct 11, 2023 •
edited
Loading

What problem does this PR solve?

Issue Number: #2579

Problem Summary:

What is changed and how it works?

add curve bs stop volume snapshot

What's Changed:

How it Works:
curve bs stop volume volumeSnapshot
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

@ZackSoul ZackSoul force-pushed the new-curvebs-stop-snapshot branch from 82e6e7e to c516f8d Compare October 11, 2023 12:00
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

23 similar comments
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 11, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@Cyber-SiKu
Copy link
Copy Markdown
Contributor

Cyber-SiKu commented Oct 12, 2023

cicheck

@Cyber-SiKu
Copy link
Copy Markdown
Contributor

Cyber-SiKu commented Oct 12, 2023

cicheck

3 similar comments
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Oct 12, 2023

cicheck

caoxianfei1
caoxianfei1 reviewed Oct 23, 2023
View reviewed changes
test/tools/status_tool_test.cpp
// .Times(1)
// .WillOnce(Return(-1));
ASSERT_EQ(-1, statusTool.RunCommand("chunkserver-status"));
}
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 Oct 23, 2023

Choose a reason for hiding this comment

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

Now tool code will not run the ci, so you can revert this change

caoxianfei1
caoxianfei1 reviewed Oct 23, 2023
View reviewed changes
tools-v2/internal/error/error.go Outdated
ErrListWarmup = func() *CmdError {
return NewInternalCmdError(74, "list warmup progress fail, err: %s")
}
ErrBsGetSnapShotListResult = func() *CmdError {
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

not be used?

caoxianfei1
caoxianfei1 reviewed Oct 23, 2023
View reviewed changes
tools-v2/pkg/cli/command/curvebs/stop/volumeSnapshot/stop_volume.go Outdated
return records, err
}

func (s *Stop) getSnapShotListAll() ([]*Record, *cmderror.CmdError) {
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 Oct 23, 2023

Choose a reason for hiding this comment

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

The function is intend to list all snapshot? may be we can provide a command such as curve bs list snapshot, then we invoke the interface here.

caoxianfei1
caoxianfei1 reviewed Oct 23, 2023
View reviewed changes
tools-v2/pkg/cli/command/curvebs/stop/volumeSnapshot/stop_volume.go Outdated
Comment on lines +13 to +17
const (
VERSION = "0.0.6"
LIMIT = "100"
OFFSET = "0"
)
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 Oct 23, 2023

Choose a reason for hiding this comment

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

The variable place global floder is better? because all snapshot command wiil use it like NewSubUri function.

caoxianfei1
caoxianfei1 reviewed Oct 23, 2023
View reviewed changes
tools-v2/pkg/config/bs.go Outdated
VIPER_CURVEBS_DEST = "curvebs.dest"
CURVEBS_TASKID = "taskid"
VIPER_CURVEBS_TASKID = "curvebs.taskid"
CURVEBS_SNAPSHOTSEQID = "snapshotseqid"
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 Oct 23, 2023

Choose a reason for hiding this comment

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

I think that the option name is too long.

caoxianfei1
caoxianfei1 reviewed Oct 23, 2023
View reviewed changes
tools-v2/pkg/cli/command/curvebs/stop/volumeSnapshot/stop_volume.go Outdated
OFFSET = "0"
)

type Stop struct {
Copy link
Copy Markdown

@caoxianfei1 caoxianfei1 Oct 23, 2023

Choose a reason for hiding this comment

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

The Stop struct name is inappropriate. Change another name?

@ZackSoul ZackSoul force-pushed the new-curvebs-stop-snapshot branch 2 times, most recently from 7e837a7 to 4c17129 Compare November 2, 2023 16:51
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

cicheck

8 similar comments
@Cyber-SiKu
Copy link
Copy Markdown
Contributor

Cyber-SiKu commented Nov 3, 2023

cicheck

@Cyber-SiKu
Copy link
Copy Markdown
Contributor

Cyber-SiKu commented Nov 3, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 3, 2023

@caoxianfei1 I've refactored it with listsnapshot, waiting for further review.

@ZackSoul ZackSoul force-pushed the new-curvebs-stop-snapshot branch 5 times, most recently from a5f4867 to c6b32b4 Compare November 6, 2023 01:43
@ZackSoul
add bs stop snapshot
291e3c9 
Signed-off-by: ZackSoul <2269713951@qq.com>

add bs stop snapshot

Signed-off-by: ZackSoul <2269713951@qq.com>

add bs stop snapshot

Signed-off-by: ZackSoul <2269713951@qq.com>

add bs stop snapshot

Signed-off-by: ZackSoul <2269713951@qq.com>

add bs stop snapshot

Signed-off-by: ZackSoul <2269713951@qq.com>

add bs stop snapshot

Signed-off-by: ZackSoul <2269713951@qq.com>

add bs stop snapshot

Signed-off-by: ZackSoul <2269713951@qq.com>

add bs stop snapshot

Signed-off-by: ZackSoul <2269713951@qq.com>
@ZackSoul ZackSoul force-pushed the new-curvebs-stop-snapshot branch from c6b32b4 to 291e3c9 Compare November 6, 2023 01:50
@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 6, 2023

cicheck

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 6, 2023

call for rerview @caoxianfei1

@ZackSoul
Copy link
Copy Markdown
Contributor Author

ZackSoul commented Nov 14, 2023

waiting for review

@caoxianfei1 caoxianfei1 merged commit b134e88 into opencurve:master Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Cyber-SiKu Cyber-SiKu Cyber-SiKu approved these changes

@caoxianfei1 caoxianfei1 caoxianfei1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ZackSoul @Cyber-SiKu @caoxianfei1