[pve-devel] [PATCH guest-common 2/2] ReplicationState: deterministically order replication jobs

[Dominik Csapak]

On 5/25/22 16:30, Thomas Lamprecht wrote:
> On 24/05/2022 13:41, Dominik Csapak wrote:
>> if we have multiple jobs for the same vmid with the same schedule,
>> the last_sync, next_sync and vmid will always be the same, so the order
>> depends on the order of the $jobs hash (which is random; thanks perl)
>>
>> to have a fixed order, take the jobid also into consideration
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   src/PVE/ReplicationState.pm | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/ReplicationState.pm b/src/PVE/ReplicationState.pm
>> index 8eebb42..ae6b1fb 100644
>> --- a/src/PVE/ReplicationState.pm
>> +++ b/src/PVE/ReplicationState.pm
>> @@ -322,7 +322,9 @@ sub get_next_job {
>>   	return $res if $res != 0;
>>   	$res = $joba->{next_sync} <=> $jobb->{next_sync};
>>   	return $res if $res != 0;
>> -	return  $joba->{guest} <=> $jobb->{guest};
>> +	$res = $joba->{guest} <=> $jobb->{guest};
>> +	return $res if $res != 0;
>> +	return $a cmp $b;
> 
> nit, but couldn't this be
> 
> return  $joba->{guest} <=> $jobb->{guest} || $a cmp $b;
> 
> instead, the right side of the logical OR only gets evaluated if the left side's
> result is 0 (well also on undef and empty string "", but that cannot happen
> with the spaceship operator).
> 

yeah sure, i just blindly copied from the lines above. do we want
to change that pattern for all of them? like this:

---
return $sa->{last_iteration} <=> $sb->{last_iteration} ||
     $joba->{next_sync} <=> $jobb->{next_sync} ||
     $joba->{guest} <=> $jobb->{guest} ||
     $a cmp $b;
---

>>       };
>>   
>>       foreach my $jobid (sort $sort_func keys %$jobs) {
>