[pve-devel] [PATCH manager] Fix #352: Limit the length of backup logs for mails

[Thomas Lamprecht]

On 5/20/19 9:49 AM, Dominic Jaeger wrote:
> Thank you Thomas and Dominik for the feedback!
> 
> 
>>     Am 5/17/19 um 12:16 PM schrieb Dominik Csapak:
>>     it is quite a bit of code, for not that much gain
>>
> I agree. My idea was to stick with the proposed solution in Bugzilla for the first patch.
> 
> 
>>     and does not even prevent the problem if someone has many vms that
>>     get backed up (or am i missing that bit?)
>>
> You are not missing it. Having too many VMs would still result in a rejected mail.
> 
> 
>>     why hardcoded here? (...)
>>     i would rather have them as parameter, so that someone can tune this (if necessary)
>>
> As I was not sure where the most appropriate place for tuning this would be.
> Anyway, they are not needed anymore as I will send a patch with your two ideas
> a)
> 
>>     one approach i would favor is to generate the mail, check the size,
>>     and only send the initial overview table with a notice that the
>>     mail was too long and where to find the log
>>
> and b)
> 
>>     Thomas Lamprecht < t.lamprecht at proxmox.com mailto:t.lamprecht at proxmox.com > hat am 17. Mai 2019 um 16:07 geschrieben:
>>     next if $line =~ /^status: \d+/;
>>
>>     would omit all status line,
>>
> With a lot of tests of course :)

It's a simple regeex, which perl is pretty good and fast at.

Also, there's already a check for a single line in HTML, or not:
> if ($line =~ m/^.+(ERROR|WARN):.*/) {

While true, doing a regex check on each line _can_ be costly, it
totally depends on the regex, here you effectively do:

if (s[0] == 's' && s[1] == 't' && s[2] == a & ...)

So about 9 single boolean comparission, which are all AND in series,
so if the first fails we can continue with the current loop operation,
or not doing "next". Shouldn't be to costly, and if it avoids this
huge pile of code I gladly pay the little (probably unoticed) runtime cost
above maintenance and review cost :)