From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 3E78A1FF17C
for <inbox@lore.proxmox.com>; Wed, 30 Apr 2025 13:09:22 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 064461FB97;
Wed, 30 Apr 2025 13:09:31 +0200 (CEST)
Message-ID: <00c570a5-e426-4b5a-93e3-8eaac9e96944@proxmox.com>
Date: Wed, 30 Apr 2025 13:09:26 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Daniel Kral <d.kral@proxmox.com>
To: pve-devel@lists.proxmox.com
References: <20250325151254.193177-1-d.kral@proxmox.com>
<20250325151254.193177-11-d.kral@proxmox.com>
Content-Language: en-US
In-Reply-To: <20250325151254.193177-11-d.kral@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.012 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [manager.pm]
Subject: Re: [pve-devel] [PATCH ha-manager 09/15] manager: apply colocation
rules when selecting service nodes
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
On 3/25/25 16:12, Daniel Kral wrote:
> sub select_service_node {
> - my ($groups, $online_node_usage, $sid, $service_conf, $current_node, $try_next, $tried_nodes, $maintenance_fallback, $best_scored) = @_;
> + # TODO Cleanup this signature post-RFC
> + my ($rules, $groups, $online_node_usage, $sid, $service_conf, $current_node, $try_next, $tried_nodes, $maintenance_fallback, $best_scored) = @_;
I'm currently trying to clean up the helper's signature here, but doing
something like
sub select_service_node {
my ($service_info, $affinity_info, $try_next, $best_scored) = @_;
my ($sid, $service_conf, $current_node) = $service_info->@{qw(sid
config current_node)};
my ($rules, $groups, $online_node_usage, $tried_nodes,
$maintenance_fallback) =
$affinity_info->@{qw(rules groups online_node_usage failed_nodes
maintenance_node)};
would require us to create helper structures on all four call sites (one
of them is just the test case ./test_failover1.pl), or introduce another
helper to just create them for passing it here and immediately
de-structuring it in select_service_node(...):
sub get_service_affinity_info {
my ($self, $sid, $cd, $sd) = @_;
my $service_info = {
sid => $sid,
config => $cd,
current_node => $sd->{node},
};
my $affinity_info = {
rules => $self->{rules},
groups => $self->{groups},
failed_nodes => $sd->{failed_nodes},
maintenance_node => $sd->{maintenance_node},
online_node_usage => $self->{online_node_usage},
};
return ($service_info, $affinity_info);
};
Also the call site in next_state_recovery(...) does not pass
$sd->{failed_nodes}, $sd->{maintenance_node} and $best_scored to it.
AFAICS $sd->{failed_nodes} should be undef in next_state_recovery(...)
anyway, but I feel like I have missed some states it could be in there.
And $sd->{maintenance_node} could be set anytime.
If there's nothing speaking against that, I'd prefer to elevate
select_service_node(...) to be a method as it needs quite a lot of state
anyway, especially as we will need global information about other
services than just the current one in the future anyway.
So, I'd do something like
sub select_service_node {
my ($self, $sid, $service_conf, $sd, $mode) = @_;
my ($rules, $groups, $online_node_usage) = $self->@{qw(rules groups
online_node_usage)};
my ($current_node, $tried_nodes, $maintenance_fallback) =
$self->@{qw(node failed_nodes maintenance_node)};
here. It's not fancy as in there's a well-defined interface one can
immediately see what this helper needs (as it has access to the whole
$self) and doesn't have the guarantees of a standalone helper (won't
touch $self), but I think it could be better than creating helper
structures which are only pass a message, which is immediately
destructured anyway. We could also just pass $self slightly differently,
but I don't see much difference there.
The $mode could then be a enumeration of e.g. whether $try_next (e.g.
'try_again') or $best_scored (e.g. 'rebalance') is used (and can be
extended of course). Those are mutually exclusive in the three call
sites right now. If next_state_recovery(...) really does have states
where $tried_nodes is set (and $maintenance_node too), then we can also
introduce a 'recovery' state, which will ignore them.
The names for $service_conf and $sd can also be improved, but I wanted
to introduce minimal change to select_service_node(...) as well as stay
to the $sd name for the service data as in other places of the Manager.pm.
That's still just a work in progress and I'd very appreciate some
feedback if any of the two above are viable options here. If it helps
any, I'd send the result as a separate series in advance which the HA
colocation will then be based on, so we don't loose focus in the HA
colocation patch series.
CC'd @Fiona and @Fabian here, if you have any thoughts here :).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel