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] tools-v2: add scan-state #2465

Merged
merged 1 commit into from
May 11, 2023

Conversation

montaguelhz
Copy link
Contributor

What problem does this PR solve?

Issue Number: #2360

Problem Summary:

support bs set scan command in [curve tool]

What is changed and how it works?

What's Changed:

  1. add tools-v2/pkg/cli/command/curvebs/update/scan_state.go
  2. modify tools-v2/pkg/cli/command/curvebs/update.go
  3. modify tools-v2/pkg/config/bs.go
  4. modify tools-v2/README.md

How it Works:

Usage:  curve bs update scan-state [flags]

enable/disable scan for logical pool

Flags:
  -c, --conf string            config file (default is $HOME/.curve/curve.yaml or /etc/curve/curve.yaml)
  -f, --format string          output format (json|plain) (default "plain")
  -h, --help                   print help
      --logicalpoolid uint32   logical pool id[required]
      --mdsaddr string         mds address, should be like 127.0.0.1:6700,127.0.0.1:6701,127.0.0.1:6702
      --rpcretrytimes int32    rpc retry times (default 1)
      --rpctimeout duration    rpc timeout (default 10s)
      --scan                   enable/disable scan for logical pool (default true)
      --showerror              display all errors in command
      --verbose                show some log

Examples:
$ curve bs update scan-state --logicalpoolid 1 [--scan=true/false]

command result

curve bs update scan-state --logicalpoolid 1 --scan=true
+----+------+---------+--------+
| ID | SCAN | RESULT  | REASON |
+----+------+---------+--------+
| 1  | true | success | null   |
+----+------+---------+--------+
curve bs update scan-state --logicalpoolid 2 --scan=false
+----+-------+--------+---------------------+
| ID | SCAN  | RESULT |       REASON        |
+----+-------+--------+---------------------+
| 2  | false | fail   | LogicalPoolNotFound |
+----+-------+--------+---------------------+

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

@montaguelhz montaguelhz reopened this May 10, 2023
@montaguelhz montaguelhz force-pushed the update-scan-state branch 3 times, most recently from c821747 to 9fdc9e5 Compare May 10, 2023 04:57
@zhanghuidinah zhanghuidinah requested a review from Cyber-SiKu May 10, 2023 09:11
config.AddRpcTimeoutFlag(sCmd.Cmd)

config.AddBSLogicalPoolIdRequiredFlag(sCmd.Cmd)
config.AddBsScanOptionFlag(sCmd.Cmd)
Copy link
Contributor

Choose a reason for hiding this comment

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

Flag scan is required, will it be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, from the user's point of view, it is more intuitive for the scan flag to default to true. At the same time, reducing parameters can reduce the user's memory burden.

@Cyber-SiKu
Copy link
Contributor

cicheck

@Cyber-SiKu
Copy link
Contributor

Please merge your commits into one

@Cyber-SiKu
Copy link
Contributor

cicheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants