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 E176E1FF16B
for <inbox@lore.proxmox.com>; Thu, 3 Apr 2025 14:17:45 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 6E41B3EE9C;
Thu, 3 Apr 2025 14:17:34 +0200 (CEST)
Date: Thu, 03 Apr 2025 14:16:51 +0200
From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= <f.gruenbichler@proxmox.com>
To: Daniel Kral <d.kral@proxmox.com>, Proxmox VE development discussion
<pve-devel@lists.proxmox.com>
References: <20250325151254.193177-1-d.kral@proxmox.com>
<20250325151254.193177-4-d.kral@proxmox.com>
<990ee499-ab4f-4aaa-8df1-b2d1dad1309e@proxmox.com>
In-Reply-To: <990ee499-ab4f-4aaa-8df1-b2d1dad1309e@proxmox.com>
MIME-Version: 1.0
User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid)
Message-Id: <1743670423.cy7yczwutf.astroid@yuna.none>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.044 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
Subject: Re: [pve-devel] [PATCH ha-manager 02/15] tools: add hash set helper
subroutines
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-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
On March 25, 2025 6:53 pm, Thomas Lamprecht wrote:
> Am 25.03.25 um 16:12 schrieb Daniel Kral:
>> Implement helper subroutines, which implement basic set operations done
>> on hash sets, i.e. hashes with elements set to a true value, e.g. 1.
>>
>> These will be used for various tasks in the HA Manager colocation rules,
>> e.g. for verifying the satisfiability of the rules or applying the
>> colocation rules on the allowed set of nodes.
>>
>> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
>> ---
>> If they're useful somewhere else, I can move them to PVE::Tools
>> post-RFC, but it'd be probably useful to prefix them with `hash_` there.
>
> meh, not a big fan of growing the overly generic PVE::Tools more, if, this
> should go into a dedicated module for hash/data structure helpers ...
>
>> AFAICS there weren't any other helpers for this with a quick grep over
>> all projects and `PVE::Tools::array_intersect()` wasn't what I needed.
>
> ... which those existing one should then also move into, but out of scope
> of this series.
>
>>
>> src/PVE/HA/Tools.pm | 42 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm
>> index 0f9e9a5..fc3282c 100644
>> --- a/src/PVE/HA/Tools.pm
>> +++ b/src/PVE/HA/Tools.pm
>> @@ -115,6 +115,48 @@ sub write_json_to_file {
>> PVE::Tools::file_set_contents($filename, $raw);
>> }
>>
>> +sub is_disjoint {
>
> IMO a bit too generic name for being in a Tools named module, maybe
> prefix them all with hash_ or hashes_ ?
is_disjoint also only really makes sense as a name if you see it as an
operation *on* $hash1, rather than an operation involving both hashes..
i.e., in Rust
set1.is_disjoint(&set2);
makes sense..
in Perl
is_disjoint($set1, $set2)
reads weird, and should maybe be
check_disjoint($set1, $set2)
or something like that?
>
>> + my ($hash1, $hash2) = @_;
>> +
>> + for my $key (keys %$hash1) {
>> + return 0 if exists($hash2->{$key});
>> + }
>> +
>> + return 1;
>> +};
>> +
>> +sub intersect {
>> + my ($hash1, $hash2) = @_;
>> +
>> + my $result = { map { $_ => $hash2->{$_} } keys %$hash1 };
this is a bit dangerous if $hash2->{$key} is itself a reference? if I
later modify $result I'll modify $hash2.. I know the commit message says
that the hashes are all just of the form key => 1, but nothing here
tells me that a year later when I am looking for a generic hash
intersection helper ;) I think this should also be clearly mentioned in
the module, and ideally, also in the helper names (i.e., have "set"
there everywhere and a comment above each that it only works for
hashes-as-sets and not generic hashes).
wouldn't it be faster/simpler to iterate over either hash once?
my $result = {};
for my $key (keys %$hash1) {
$result->{$key} = 1 if $hash1->{$key} && $hash2->{$key};
}
return $result;
>> +
>> + for my $key (keys %$result) {
>> + delete $result->{$key} if !defined($result->{$key});
>> + }
>> +
>> + return $result;
>> +};
>> +
>> +sub set_difference {
>> + my ($hash1, $hash2) = @_;
>> +
>> + my $result = { map { $_ => 1 } keys %$hash1 };
if $hash1 is only of the form key => 1, then this is just
my $result = { %$hash1 };
>> +
>> + for my $key (keys %$result) {
>> + delete $result->{$key} if defined($hash2->{$key});
>> + }
>> +
but the whole thing can be
return { map { $hash2->{$_} ? ($_ => 1) : () } keys %$hash1 };
this transforms hash1 into its keys, and then returns either ($key => 1)
if the key is true in $hash2, or the empty tuple if not. the outer {}
then turn this sequence of tuples into a hash again, which skips empty
tuples ;) can of course also be adapted to use the value from either
hash, check for definedness instead of truthiness, ..
>> + return $result;
>> +};
>> +
>> +sub union {
>> + my ($hash1, $hash2) = @_;
>> +
>> + my $result = { map { $_ => 1 } keys %$hash1, keys %$hash2 };
>> +
>> + return $result;
>> +};
>> +
>> sub count_fenced_services {
>> my ($ss, $node) = @_;
>>
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel