Skip to content

Commit

Permalink
[#23332] Docdb: Fix yb-admin to handle snapshot_retention correctly
Browse files Browse the repository at this point in the history
Summary:
After adding a Time To Live to the snapshots created by the user. The default TTL for snapshots before deleting them is 24 hours or the flag `default_snapshot_retention_hours`. However, users can override this default behaviour when creating a snapshot using `yb-admin` by providing the desired `retention_duration_hours` as a second argument to the `create_database_snapshot`. So if the user wants to leave the snapshot for 72 hours, he can issue:

```
./bin/yb-admin --master_addresses $MASTERS create_database_snapshot ysql.db1 72
```
However, yb_admin CLI is not accepting that the user set the second argument and returns `Invalid argument` error by mistake.
Additionally, if the user wants to retain the snapshot forever, the instructions say:
`(set a <= 0 value to retain the snapshot forever. If not specified then takes the default value controlled by gflag default_retention_hours)`
However, if the user tries to set a negative value, the negative sign will be parsed as an option for yb-admin. Ex:
```
$ ./build/latest/bin/yb-admin --master_addresses 127.0.0.1:7100,127.0.0.2:7100,127.0.0.3:7100 create_database_snapshot ysql.db1 '-9'
ERROR: unknown command line flag '9'
```
The diff fixes these issues by:
1- Accepting 1 or 2 arguments for `create_database_snapshot`
2- A simpler message to the user if he wants to retain the snapshot forever: `set retention_duration_hours to 0 to retain the snapshot forever.` Instead of telling the user to set it to negative value while that is not possible.
Jira: DB-12259

Test Plan:
Tested manually, by running the yb-admin command `create_database_snapshot` with and without the `retention_duration_hours` option set. The command was failing before this diff.

Before this diff:
```

$ ./build/latest/bin/yb-admin --master_addresses 127.0.0.1:7100,127.0.0.2:7100,127.0.0.3:7100 create_database_snapshot ysql.db1 72
Error running create_database_snapshot: Invalid argument (yb/tools/yb-admin_cli.cc:91): Invalid arguments for operation
Usage: ./build/latest/bin/yb-admin create_database_snapshot [ysql.]<database_name> [retention_duration_hours] (set a <= 0 value to retain the snapshot forever. If not specified then takes the default value controlled by gflag default_retention_hours)
```
After this diff it works as desired.

Reviewers: zdrudi

Reviewed By: zdrudi

Subscribers: ybase, slingam

Differential Revision: https://phorge.dev.yugabyte.com/D36936
  • Loading branch information
yamen-haddad committed Jul 31, 2024
1 parent b2f24f2 commit 5891faa
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/yb/tools/yb-admin_cli.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1596,12 +1596,13 @@ Status create_keyspace_snapshot_action(
return Status::OK();
}

const auto create_database_snapshot_args = "[ysql.]<database_name> [retention_duration_hours] "
"(set a <= 0 value to retain the snapshot forever. If not specified "
const auto create_database_snapshot_args =
"[ysql.]<database_name> [retention_duration_hours] "
"(set retention_duration_hours to 0 to retain the snapshot forever. If not specified "
"then takes the default value controlled by gflag default_retention_hours)";
Status create_database_snapshot_action(
const ClusterAdminCli::CLIArguments& args, ClusterAdminClient* client) {
if (args.size() != 1) {
if (args.size() < 1 && args.size() > 2) {
return ClusterAdminCli::kInvalidArguments;
}

Expand Down

0 comments on commit 5891faa

Please sign in to comment.