Re: [pve-devel] [PATCH cluster v4 1/2] status: introduce new pve-{type}- rrd and metric format

From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Lukas Wagner <l.wagner@proxmox.com>,
	Aaron Lauterer <a.lauterer@proxmox.com>
Subject: Re: [pve-devel] [PATCH cluster v4 1/2] status: introduce new pve-{type}- rrd and metric format
Date: Thu, 31 Jul 2025 05:23:07 +0200	[thread overview]
Message-ID: <a43de1d9-9ae4-445d-868a-f0ab33057546@proxmox.com> (raw)
In-Reply-To: <DBOFX55QESED.2T6OETJWKRYXW@proxmox.com>

Am 29.07.25 um 11:44 schrieb Lukas Wagner:
>> +// A 4k buffer should be plenty to temporarily store RRD data. 64 bit integers are 20 chars long,
>> +// plus the separator char: (4096-1)/21~195 columns This buffer is only used in the
>> +// `update_rrd_data` function. It is safe to use as the calling sites get the global mutex:
>> +// rrd_update_data -> rrdentry_hash_set -> cfs_status_set / and cfs_kvstore_node_set
>> +static char rrd_format_update_buffer[4096];
> Maybe I'm missing something, but since this is only used in the
> update_rrd_data function, is there any reason why this statically
> allocated and not just malloc'd as needed?

For the record: I nudged Aaron in that direction in a previous review, IMO a
small static buffer can sometimes be easier to handle compared to malloc, and
definitively could be more performant that frequent malloc+free sequences, but
performance probably is not really impacted here in any case.

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel