* [pve-devel] [PATCH installer 1/2] tui: expose arc size setting for zfs bootdisks for all products
2023-11-30 10:01 [pve-devel] [PATCH installer 0/2] expose zfs arc size setting for all products Christoph Heiss
@ 2023-11-30 10:01 ` Christoph Heiss
2023-11-30 10:01 ` [pve-devel] [PATCH installer 2/2] proxinstall: " Christoph Heiss
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2023-11-30 10:01 UTC (permalink / raw)
To: pve-devel
For non-PVE products, this defaults to 50% of available system memory
(aka. ZFS default).
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/src/options.rs | 45 ++++++++++++---------
proxmox-tui-installer/src/views/bootdisk.rs | 19 ++++-----
2 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index afd12cd..87931d1 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -206,14 +206,15 @@ impl ZfsBootdiskOptions {
/// # Returns
/// The default ZFS maximum ARC size in MiB for this system.
fn default_zfs_arc_max(product: ProxmoxProduct, total_memory: usize) -> usize {
- if product != ProxmoxProduct::PVE {
- // Use ZFS default for non-PVE
- 0
+ let mem_mib = if product == ProxmoxProduct::PVE {
+ // For PVE, use 10% as default and clamp to 16 GiB max
+ ((total_memory as f64) / 10.).round().min(16. * 1024.)
} else {
- ((total_memory as f64) / 10.)
- .round()
- .clamp(64., 16. * 1024.) as usize
- }
+ // Use 50% default for non-PVE
+ ((total_memory as f64) / 2.).round()
+ };
+
+ mem_mib.max(64.) as usize
}
#[derive(Clone, Debug)]
@@ -417,23 +418,29 @@ mod tests {
#[test]
fn zfs_arc_limit() {
- const TESTS: &[(usize, usize)] = &[
- (16, 64), // at least 64 MiB
- (1024, 102),
- (4 * 1024, 410),
- (8 * 1024, 819),
- (150 * 1024, 15360),
- (160 * 1024, 16384),
- (1024 * 1024, 16384), // maximum of 16 GiB
+ const TESTS: &[(usize, (usize, usize, usize))] = &[
+ (16, (64, 64, 64)), // at least 64 MiB for all products
+ (1024, (102, 512, 512)),
+ (4 * 1024, (410, 2048, 2048)),
+ (8 * 1024, (819, 4096, 4096)),
+ (150 * 1024, (15360, 76800, 76800)),
+ (160 * 1024, (16384, 81920, 81920)),
+ (1024 * 1024, (16384, 524288, 524288)), // maximum of 16 GiB for PVE
];
- for (total_memory, expected) in TESTS {
+ for (total_memory, (pve_expected, pbs_expected, pmg_expected)) in TESTS {
assert_eq!(
default_zfs_arc_max(ProxmoxProduct::PVE, *total_memory),
- *expected
+ *pve_expected
+ );
+ assert_eq!(
+ default_zfs_arc_max(ProxmoxProduct::PBS, *total_memory),
+ *pbs_expected
+ );
+ assert_eq!(
+ default_zfs_arc_max(ProxmoxProduct::PMG, *total_memory),
+ *pmg_expected
);
- assert_eq!(default_zfs_arc_max(ProxmoxProduct::PBS, *total_memory), 0);
- assert_eq!(default_zfs_arc_max(ProxmoxProduct::PMG, *total_memory), 0);
}
}
}
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 7e13e91..7efe953 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -155,7 +155,7 @@ impl AdvancedBootdiskOptionsView {
&product_conf,
)),
AdvancedBootdiskOptions::Zfs(zfs) => {
- view.add_child(ZfsBootdiskOptionsView::new(runinfo, zfs, &product_conf))
+ view.add_child(ZfsBootdiskOptionsView::new(runinfo, zfs))
}
AdvancedBootdiskOptions::Btrfs(btrfs) => {
view.add_child(BtrfsBootdiskOptionsView::new(&runinfo.disks, btrfs))
@@ -559,13 +559,7 @@ struct ZfsBootdiskOptionsView {
impl ZfsBootdiskOptionsView {
// TODO: Re-apply previous disk selection from `options` correctly
- fn new(
- runinfo: &RuntimeInfo,
- options: &ZfsBootdiskOptions,
- product_conf: &ProductConfig,
- ) -> Self {
- let is_pve = product_conf.product == ProxmoxProduct::PVE;
-
+ fn new(runinfo: &RuntimeInfo, options: &ZfsBootdiskOptions) -> Self {
let inner = FormView::new()
.child("ashift", IntegerEditView::new().content(options.ashift))
.child(
@@ -592,9 +586,11 @@ impl ZfsBootdiskOptionsView {
.unwrap_or_default(),
),
)
- .child("copies", IntegerEditView::new().content(options.copies).max_value(3))
- .child_conditional(
- is_pve,
+ .child(
+ "copies",
+ IntegerEditView::new().content(options.copies).max_value(3),
+ )
+ .child(
"ARC max size",
IntegerEditView::new_with_suffix("MiB")
.max_value(runinfo.total_memory)
@@ -614,7 +610,6 @@ impl ZfsBootdiskOptionsView {
Self::new(
runinfo,
&ZfsBootdiskOptions::defaults_from(runinfo, product_conf),
- product_conf,
)
}
--
2.42.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH installer 2/2] proxinstall: expose arc size setting for zfs bootdisks for all products
2023-11-30 10:01 [pve-devel] [PATCH installer 0/2] expose zfs arc size setting for all products Christoph Heiss
2023-11-30 10:01 ` [pve-devel] [PATCH installer 1/2] tui: expose arc size setting for zfs bootdisks " Christoph Heiss
@ 2023-11-30 10:01 ` Christoph Heiss
2024-01-24 9:52 ` [pve-devel] [PATCH installer 0/2] expose zfs arc size setting " Christoph Heiss
2024-02-06 12:49 ` Thomas Lamprecht
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Heiss @ 2023-11-30 10:01 UTC (permalink / raw)
To: pve-devel
For non-PVE products, this defaults to 50% of available system memory
(aka. ZFS default).
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Proxmox/Install/RunEnv.pm | 20 +++++++++++++-------
proxinstall | 24 +++++++++++-------------
test/zfs-arc-max.pl | 26 +++++++++++++-------------
3 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index c393f67..25a8b49 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -310,20 +310,26 @@ sub query_installation_environment : prototype() {
our $ZFS_ARC_MIN_SIZE_MIB = 64; # MiB
# See https://bugzilla.proxmox.com/show_bug.cgi?id=4829
-our $ZFS_ARC_MAX_SIZE_MIB = 16 * 1024; # 16384 MiB = 16 GiB
-our $ZFS_ARC_SYSMEM_PERCENTAGE = 0.1; # use 10% of available system memory by default
+our $ZFS_ARC_PVE_MAX_SIZE_MIB = 16 * 1024; # 16384 MiB = 16 GiB
+our $ZFS_ARC_PVE_SYSMEM_PERCENTAGE = 0.1; # use 10% of available system memory for PVE as default
+our $ZFS_ARC_DEFAULT_SYSMEM_PERCENTAGE = 0.5; # 50% of available system memory otherwise
# Calculates the default upper limit for the ZFS ARC size.
# Returns the default ZFS maximum ARC size in MiB.
sub default_zfs_arc_max {
- # Use ZFS default on non-PVE
- return 0 if Proxmox::Install::ISOEnv::get('product') ne 'pve';
+ my $percentage = $ZFS_ARC_DEFAULT_SYSMEM_PERCENTAGE;
+ my $max_mib = get('total_memory');
- my $default_mib = get('total_memory') * $ZFS_ARC_SYSMEM_PERCENTAGE;
+ if (Proxmox::Install::ISOEnv::get('product') eq 'pve') {
+ $percentage = $ZFS_ARC_PVE_SYSMEM_PERCENTAGE;
+ $max_mib = $ZFS_ARC_PVE_MAX_SIZE_MIB;
+ }
+
+ my $default_mib = get('total_memory') * $percentage;
my $rounded_mib = int(sprintf('%.0f', $default_mib));
- if ($rounded_mib > $ZFS_ARC_MAX_SIZE_MIB) {
- return $ZFS_ARC_MAX_SIZE_MIB;
+ if ($rounded_mib > $max_mib) {
+ return $max_mib;
} elsif ($rounded_mib < $ZFS_ARC_MIN_SIZE_MIB) {
return $ZFS_ARC_MIN_SIZE_MIB;
}
diff --git a/proxinstall b/proxinstall
index 01d4cfe..97e9462 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1167,20 +1167,18 @@ my $create_raid_advanced_grid = sub {
$spinbutton_copies->set_value($copies);
push @$labeled_widgets, ['copies', $spinbutton_copies];
- if ($iso_env->{product} eq 'pve') {
- my $total_memory = Proxmox::Install::RunEnv::get('total_memory');
+ my $total_memory = Proxmox::Install::RunEnv::get('total_memory');
- my $spinbutton_arc_max = Gtk3::SpinButton->new_with_range(
- $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1);
- $spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes');
- $spinbutton_arc_max->signal_connect('value-changed' => sub {
- my $w = shift;
- Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int());
- });
- my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max');
- $spinbutton_arc_max->set_value($arc_max);
- push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
- }
+ my $spinbutton_arc_max = Gtk3::SpinButton->new_with_range(
+ $Proxmox::Install::RunEnv::ZFS_ARC_MIN_SIZE_MIB, $total_memory, 1);
+ $spinbutton_arc_max->set_tooltip_text('Maximum ARC size in megabytes');
+ $spinbutton_arc_max->signal_connect('value-changed' => sub {
+ my $w = shift;
+ Proxmox::Install::Config::set_zfs_opt('arc_max', $w->get_value_as_int());
+ });
+ my $arc_max = Proxmox::Install::Config::get_zfs_opt('arc_max');
+ $spinbutton_arc_max->set_value($arc_max);
+ push @$labeled_widgets, ['ARC max size', $spinbutton_arc_max, 'MiB'];
push @$labeled_widgets, ['hdsize', $hdsize_btn, 'GB'];
return $create_label_widget_grid->($labeled_widgets);;
diff --git a/test/zfs-arc-max.pl b/test/zfs-arc-max.pl
index 74cb9b5..a554d56 100755
--- a/test/zfs-arc-max.pl
+++ b/test/zfs-arc-max.pl
@@ -22,13 +22,13 @@ sub mock_product {
}
my %default_tests = (
- 16 => 64, # at least 64 MiB
- 1024 => 102,
- 4 * 1024 => 410,
- 8 * 1024 => 819,
- 150 * 1024 => 15360,
- 160 * 1024 => 16384,
- 1024 * 1024 => 16384, # maximum of 16 GiB
+ 16 => [64, 64, 64], # at least 64 MiB in any case
+ 1024 => [102, 512, 512],
+ 4 * 1024 => [410, 2048, 2048],
+ 8 * 1024 => [819, 4096, 4096],
+ 150 * 1024 => [15360, 76800, 76800],
+ 160 * 1024 => [16384, 81920, 81920],
+ 1024 * 1024 => [16384, 524288, 524288], # maximum of 16 GiB for PVE
);
while (my ($total_mem, $expected) = each %default_tests) {
@@ -41,16 +41,16 @@ while (my ($total_mem, $expected) = each %default_tests) {
);
mock_product('pve');
- is(Proxmox::Install::RunEnv::default_zfs_arc_max(), $expected,
- "$expected MiB should be zfs_arc_max for PVE with $total_mem MiB system memory");
+ is(Proxmox::Install::RunEnv::default_zfs_arc_max(), $expected->[0],
+ "$expected->[0] MiB should be zfs_arc_max for PVE with $total_mem MiB system memory");
mock_product('pbs');
- is(Proxmox::Install::RunEnv::default_zfs_arc_max(), 0,
- "zfs_arc_max should default to `0` for PBS with $total_mem MiB system memory");
+ is(Proxmox::Install::RunEnv::default_zfs_arc_max(), $expected->[1],
+ "$expected->[1] MiB should be zfs_arc_max for PBS with $total_mem MiB system memory");
mock_product('pmg');
- is(Proxmox::Install::RunEnv::default_zfs_arc_max(), 0,
- "zfs_arc_max should default to `0` for PMG with $total_mem MiB system memory");
+ is(Proxmox::Install::RunEnv::default_zfs_arc_max(), $expected->[2],
+ "$expected->[2] MiB should be zfs_arc_max for PMG with $total_mem MiB system memory");
}
my @clamp_tests = (
--
2.42.0
^ permalink raw reply [flat|nested] 7+ messages in thread