[pve-devel] [PATCH ha-manager 02/15] tools: add hash set helper subroutines
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Apr 3 14:16:51 CEST 2025
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 at 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 at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
More information about the pve-devel
mailing list