[pve-devel] [PATCH manager v2 3/8] ceph: add autoscale_status to api calls
Dominik Csapak
d.csapak at proxmox.com
Tue Nov 24 14:53:25 CET 2020
comments inline
On 11/24/20 11:58 AM, Alwin Antreich wrote:
> the properties target_size_ratio and target_size_bytes are the only two
> options by ceph to set on a pool. The updated pool list shows now
> autoscale settings & status. Including the new target PGs. To make it
> easier for new users to get/set the correct amount of PGs.
>
> And use parameter extraction instead of the unneeded ref copy for params.
i'd rather have this as its own patch than here but ok...
>
> Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
> ---
> PVE/API2/Ceph/POOLS.pm | 131 +++++++++++++++++++++++++++++++----------
> PVE/CLI/pveceph.pm | 3 +
> PVE/Ceph/Tools.pm | 21 +++++++
> 3 files changed, 123 insertions(+), 32 deletions(-)
>
> diff --git a/PVE/API2/Ceph/POOLS.pm b/PVE/API2/Ceph/POOLS.pm
> index 19fc1b7e..324a1f79 100644
> --- a/PVE/API2/Ceph/POOLS.pm
> +++ b/PVE/API2/Ceph/POOLS.pm
> @@ -3,6 +3,7 @@ package PVE::API2::Ceph::POOLS;
> use strict;
> use warnings;
>
> +use PVE::Tools qw(extract_param);
> use PVE::Ceph::Tools;
> use PVE::Ceph::Services;
> use PVE::JSONSchema qw(get_standard_option);
> @@ -67,6 +68,19 @@ my $ceph_pool_common_options = sub {
> default => 'warn',
> optional => 1,
> },
> + target_size => {
> + description => "The estimated target size of the pool for the PG autoscaler.",
> + title => 'PG Autoscale Target Size',
> + type => 'string',
> + pattern => '(\d+(?:\.\d+)?)([KMGT])?',
> + optional => 1,
> + },
> + target_size_ratio => {
> + description => "The estimated target ratio of the pool for the PG autoscaler.",
> + title => 'PG Autoscale Target Ratio',
> + type => 'number',
> + optional => 1,
> + },
> };
>
> if ($nodefault) {
> @@ -105,6 +119,30 @@ my $get_storages = sub {
> return $res;
> };
>
> +my $get_autoscale_status = sub {
> + my ($rados, $pool) = @_;
> +
> + $rados = PVE::RADOS->new() if !defined($rados);
> +
> + my $autoscale = $rados->mon_command({
> + prefix => 'osd pool autoscale-status'});
> +
> + my $data;
> + foreach my $p (@$autoscale) {
> + $p->{would_adjust} = "$p->{would_adjust}"; # boolean
> + push @$data, $p;
> + }
> +
> + if ($pool) {
> + foreach my $p (@$data) {
> + next if $p->{pool_name} ne $pool;
> + $data = $p;
> + }
> + }
> +
> + return $data;
> +
would it not better to return a hash with mapping
poolname -> data_for_that_pool
?
this way you only need to iterate once? and save
iterations below....
};
> +
>
> __PACKAGE__->register_method ({
> name => 'lspools',
> @@ -127,16 +165,20 @@ __PACKAGE__->register_method ({
> items => {
> type => "object",
> properties => {
> - pool => { type => 'integer', title => 'ID' },
> - pool_name => { type => 'string', title => 'Name' },
> - size => { type => 'integer', title => 'Size' },
> - min_size => { type => 'integer', title => 'Min Size' },
> - pg_num => { type => 'integer', title => 'PG Num' },
> - pg_autoscale_mode => { type => 'string', optional => 1, title => 'PG Autoscale Mode' },
> - crush_rule => { type => 'integer', title => 'Crush Rule' },
> - crush_rule_name => { type => 'string', title => 'Crush Rule Name' },
> - percent_used => { type => 'number', title => '%-Used' },
> - bytes_used => { type => 'integer', title => 'Used' },
> + pool => { type => 'integer', title => 'ID' },
> + pool_name => { type => 'string', title => 'Name' },
> + size => { type => 'integer', title => 'Size' },
> + min_size => { type => 'integer', title => 'Min Size' },
> + pg_num => { type => 'integer', title => 'PG Num' },
> + pg_num_final => { type => 'integer', title => 'Optimal # of PGs' },
> + pg_autoscale_mode => { type => 'string', title => 'PG Autoscale Mode', optional => 1 },
> + crush_rule => { type => 'integer', title => 'Crush Rule' },
> + crush_rule_name => { type => 'string', title => 'Crush Rule Name' },
> + percent_used => { type => 'number', title => '%-Used' },
> + bytes_used => { type => 'integer', title => 'Used' },
> + target_size => { type => 'integer', title => 'PG Autoscale Target Size', optional => 1 },
> + target_size_ratio => { type => 'number', title => 'PG Autoscale Target Ratio',optional => 1, },
> + autoscale_status => { type => 'object', title => 'Autoscale Status', optional => 1 },
> },
> },
> links => [ { rel => 'child', href => "{pool_name}" } ],
> @@ -176,12 +218,18 @@ __PACKAGE__->register_method ({
> 'pg_autoscale_mode',
> ];
>
> + my $autoscale = $get_autoscale_status->($rados);
> +
> foreach my $e (@{$res->{pools}}) {
> my $d = {};
> foreach my $attr (@$attr_list) {
> $d->{$attr} = $e->{$attr} if defined($e->{$attr});
> }
>
> + # some info is nested under options instead
> + $d->{target_size} = $e->{options}->{target_size_bytes};
> + $d->{target_size_ratio} = $e->{options}->{target_size_ratio};
> +
> if (defined($d->{crush_rule}) && defined($rules->{$d->{crush_rule}})) {
> $d->{crush_rule_name} = $rules->{$d->{crush_rule}};
> }
> @@ -190,10 +238,17 @@ __PACKAGE__->register_method ({
> $d->{bytes_used} = $s->{bytes_used};
> $d->{percent_used} = $s->{percent_used};
> }
> +
> + foreach my $p (@$autoscale) {
> + next if ($p->{pool_id} ne $e->{pool});
> + $d->{autoscale_status} = $p;
> + # pick some info directly from the autoscale_status
> + $d->{pg_num_final} = $p->{pg_num_final};
> + }
namely here.
you could do then
$d->{autoscale_status} = $autoscale->{$e->{pool}}
or something like that, instead of iterating over the lists
everytime (this is n^2 of number of pools)
> +
> push @$data, $d;
> }
>
> -
> return $data;
> }});
>
> @@ -233,34 +288,37 @@ __PACKAGE__->register_method ({
> PVE::Cluster::check_cfs_quorum();
> PVE::Ceph::Tools::check_ceph_configured();
>
> - my $pool = $param->{name};
> + my $pool = extract_param($param, 'name');
> + my $add_storages = extract_param($param, 'add_storages');
> + delete $param->{node}; # not a ceph option
> +
> my $rpcenv = PVE::RPCEnvironment::get();
> my $user = $rpcenv->get_user();
>
> - if ($param->{add_storages}) {
> + # Ceph uses target_size_bytes
> + if (defined($param->{'target_size'})) {
> + my $target_sizestr = extract_param($param, 'target_size');
> + $param->{target_size_bytes} = PVE::Ceph::Tools::convert_to_bytes($target_sizestr);
> + }
> +
> + if (defined($add_storages)) {
this is wrong, what if i give '--add_storages 0' ?
> $rpcenv->check($user, '/storage', ['Datastore.Allocate']);
> die "pool name contains characters which are illegal for storage naming\n"
> if !PVE::JSONSchema::parse_storage_id($pool);
> }
>
> - my $ceph_param = \%$param;
> - for my $item ('add_storages', 'name', 'node') {
> - # not ceph parameters
> - delete $ceph_param->{$item};
> - }
> -
> # pool defaults
> - $ceph_param->{pg_num} //= 128;
> - $ceph_param->{size} //= 3;
> - $ceph_param->{min_size} //= 2;
> - $ceph_param->{application} //= 'rbd';
> - $ceph_param->{pg_autoscale_mode} //= 'warn';
> + $param->{pg_num} //= 128;
> + $param->{size} //= 3;
> + $param->{min_size} //= 2;
> + $param->{application} //= 'rbd';
> + $param->{pg_autoscale_mode} //= 'warn';
>
> my $worker = sub {
>
> - PVE::Ceph::Tools::create_pool($pool, $ceph_param);
> + PVE::Ceph::Tools::create_pool($pool, $param);
>
> - if ($param->{add_storages}) {
> + if (defined($add_storages)) {
same here
> my $err;
> eval { $add_storage->($pool, "${pool}"); };
> if ($@) {
> @@ -391,15 +449,17 @@ __PACKAGE__->register_method ({
> my $rpcenv = PVE::RPCEnvironment::get();
> my $authuser = $rpcenv->get_user();
>
> - my $pool = $param->{name};
> - my $ceph_param = \%$param;
> - for my $item ('name', 'node') {
> - # not ceph parameters
> - delete $ceph_param->{$item};
> + my $pool = extract_param($param, 'name');
> + delete $param->{node};
> +
> + # Ceph uses target_size_bytes
> + if (defined($param->{'target_size'})) {
> + my $target_sizestr = extract_param($param, 'target_size');
> + $param->{target_size_bytes} = PVE::Ceph::Tools::convert_to_bytes($target_sizestr);
> }
>
> my $worker = sub {
> - PVE::Ceph::Tools::set_pool($pool, $ceph_param);
> + PVE::Ceph::Tools::set_pool($pool, $param);
> };
>
> return $rpcenv->fork_worker('cephsetpool', $pool, $authuser, $worker);
> @@ -448,6 +508,7 @@ __PACKAGE__->register_method ({
> use_gmt_hitset => { type => 'boolean', title => 'use_gmt_hitset' },
> fast_read => { type => 'boolean', title => 'Fast Read' },
> statistics => { type => 'object', title => 'Statistics', optional => 1 },
> + autoscale_status => { type => 'object', title => 'Autoscale Status', optional => 1 },
> %{ $ceph_pool_common_options->() },
> },
> },
> @@ -483,8 +544,12 @@ __PACKAGE__->register_method ({
> hashpspool => "$res->{hashpspool}",
> use_gmt_hitset => "$res->{use_gmt_hitset}",
> fast_read => "$res->{fast_read}",
> + # is only avialable if set
s/avialable/available/
> + target_size => $res->{target_size_bytes} // undef,
> + target_size_ratio => $res->{target_size_ratio} // undef,
> };
>
> +
> if ($verbose) {
> my $stats;
> my $res = $rados->mon_command({ prefix => 'df' });
> @@ -498,6 +563,8 @@ __PACKAGE__->register_method ({
> my $apps = $rados->mon_command({ prefix => "osd pool application get", pool => "$pool", });
> my @application = keys %$apps;
> $data->{application} = $application[0];
> +
> + $data->{autoscale_status} = $get_autoscale_status->($rados, $pool),
> }
>
> return $data;
> diff --git a/PVE/CLI/pveceph.pm b/PVE/CLI/pveceph.pm
> index 69421ca6..b7b68540 100755
> --- a/PVE/CLI/pveceph.pm
> +++ b/PVE/CLI/pveceph.pm
> @@ -187,7 +187,10 @@ our $cmddef = {
> 'size',
> 'min_size',
> 'pg_num',
> + 'pg_num_final',
> 'pg_autoscale_mode',
> + 'target_size',
> + 'target_size_ratio',
> 'crush_rule_name',
> 'percent_used',
> 'bytes_used',
> diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> index 12d309be..d95e8676 100644
> --- a/PVE/Ceph/Tools.pm
> +++ b/PVE/Ceph/Tools.pm
> @@ -487,4 +487,25 @@ sub ceph_cluster_status {
> return $status;
> }
>
> +sub convert_to_bytes {
> + my ($sizestr) = shift;
> +
> + die "internal error" if $sizestr !~ m/^(\d+(?:\.\d+)?)([KMGT])?$/;
> + my ($newsize, $unit) = ($1, $2);
> +
> + if ($unit) {
> + if ($unit eq 'K') {
> + $newsize = $newsize * 1024;
> + } elsif ($unit eq 'M') {
> + $newsize = $newsize * 1024 * 1024;
> + } elsif ($unit eq 'G') {
> + $newsize = $newsize * 1024 * 1024 * 1024;
> + } elsif ($unit eq 'T') {
> + $newsize = $newsize * 1024 * 1024 * 1024 * 1024;
> + }
> + }
> +
> + return int($newsize);
> +}
don't we have something like this in PVE::Tools?
> +
> 1;
>
More information about the pve-devel
mailing list