all lists on 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 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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal