public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal