From: Lukas Wagner <l.wagner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Thomas Lamprecht <t.lamprecht@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data
Date: Fri, 28 Mar 2025 11:04:09 +0100 [thread overview]
Message-ID: <b05adc0a-27c5-495b-acc1-badbd644fa91@proxmox.com> (raw)
In-Reply-To: <c1339bec-d90b-4ad6-babe-bbe40c1f7b57@proxmox.com>
On 2025-03-28 10:38, Thomas Lamprecht wrote:
> Am 28.03.25 um 09:28 schrieb Lukas Wagner:
>> We of course can cache the FQDN, but realistically speaking, this is only called once per
>> notification being sent, thus any real-world performance impact is absolutely tiny.
>
> Not so sure about that in general, e.g. sending out notifications could
> correlate with an overloaded system, and for really overloaded systems
> things that are normally cheap suddenly ain't – e.g., on low memory
> situations even a doing a plain fork+exec of a tiny binary can hang for
> a long time then, socket operations like our helper does are definitively
> less problematic (I think, as I did not evaluate it [0] and that's something
> one can less easily experience directly compared to the former, where even
> starting a new basic dash shell on such overloaded system can need minutes).
Yes, my 'impact is tiny' was mostly based on already using PVE::Tools::get_fqdn. For the
old fork+exec version you are absolutely right, this could take very long on a
system at its limits.
>
> And as the notification system now also handles things like HA events it's
> definitively part of the more critical systems which _can_ justify some extra
> scrutiny. That said, switching to the get_fqdn method makes this indeed quite
> cheap to get [0], so I'm fine with not doing any caching here for now, but
> let's not underestimate the impact of such things too much, especially for
> anything in critical chains that can be important in critical (load) situation
> (as general strategy for all, as I'm really not thinking about you here, and
> it certainly is a balance).
That makes a lot of sense. Thanks for the detailed explanation, highly appreciated.
Actually I will add caching right away, it a pretty trivial change anyways.
>
> [0]: FWIW, I just did a quick evaluation of querying the fqdn 100 000 times
> with the socket variant and the hostname one, this was done on a very healthy
> system though, I'd expect that the fork+exec one degrades a lot worse with
> higher cpu/memory pressure. Test and result:
>
> # perl -wE 'use PVE::Tools; use Time::HiRes qw(gettimeofday tv_interval); my $t0 = [gettimeofday]; for(my $i = 0; $i <= 100_000; $i++) { my $fqdn = PVE::Tools::get_fqdn("nina"); } my $elapsed = tv_interval ( $t0, [gettimeofday]); say "elapsed (s): ". $elapsed;'
> elapsed (s): 0.436712
>
> Same with 1 million runs gets me 4.368217 s, so seems to scale quite linearly.
>
> # perl -wE 'use Time::HiRes qw(gettimeofday tv_interval); my $t0 = [gettimeofday]; for(my $i = 0; $i <= 100_000; $i++) { my $fqdn = `hostname -f`; } my $elapsed = tv_interval ( $t0, [gettimeofday]); say "elapsed: ". $elapsed;'
> elapsed (s): 82.484177
>
> Same with 1 million runs gets me 577.653117 s, so not fully linearly, but
> in any way about 188x and 132x times slower, respectively.
Good to know, thanks for backing that up with some concrete data! :)
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-03-28 10:04 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data Lukas Wagner
2025-03-27 15:31 ` Thomas Lamprecht
2025-03-28 8:28 ` Lukas Wagner
2025-03-28 9:38 ` Thomas Lamprecht
2025-03-28 10:04 ` Lukas Wagner [this message]
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 1/4] notification templates: vzdump: generate HTML table in template Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 2/4] notifications: apt: clean up notification template Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 3/4] notification: replication: add common properties to template data Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 4/4] notifications: test: style fixup Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-ha-manager 1/1] notifications: overhaul fence notification Lukas Wagner
2025-03-28 10:21 ` [pve-devel] superseded: [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b05adc0a-27c5-495b-acc1-badbd644fa91@proxmox.com \
--to=l.wagner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.lamprecht@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.