Re: [pbs-devel] [PATCH proxmox-backup] server/worker_task: fix 'unknown' status for some big task logs

[Dominik Csapak <d.csapak@proxmox.com>]

On 9/3/20 12:36 PM, Dietmar Maurer wrote:
> cant we simply skip the first line?
> 
> let reader = BufReader::new(file);
> let lines = reader.lines();
> lines.next()?; // skip first
> 
> // Then parse the rest
> for line in lines {
> 
> }

no for two reasons:

if the task log contains only one line (this happens, e.g. 'TASK OK')
or if the last Log line is exactly 8k long (i am not sure that could happen)

for the first we could check if the file is < 8k long

> 
>> On 09/03/2020 11:39 AM Dominik Csapak <d.csapak@proxmox.com> wrote:
>>
>>   
>> when trying to parse the task status, we seek 8k from the end
>> which may be into the middle of a line, so the datetime parsing
>> can fail (when the log message contains ': ')
>> so try the next line when the first datetime parsing fails
>>
>> if it was really a broken datetime, we now return either
>> the datetime of the next line or an 'Unknown' TaskState
>> (which was the fallback of most call sites anyway)
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> we could save the error for later returning, but
>> since that does not matter most of the time i did not do that
>> but if wanted i can send a v2 or fixup for that
>>
>>   src/server/worker_task.rs | 17 ++++++++++++++---
>>   1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
>> index a9e4a36a..f6d12ac3 100644
>> --- a/src/server/worker_task.rs
>> +++ b/src/server/worker_task.rs
>> @@ -209,14 +209,25 @@ pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
>>   
>>       let reader = BufReader::new(file);
>>   
>> +    let mut first_line = true;
>>       for line in reader.lines() {
>>           let line = line?;
>>   
>>           let mut iter = line.splitn(2, ": ");
>>           if let Some(time_str) = iter.next() {
>> -            endtime = chrono::DateTime::parse_from_rfc3339(time_str)
>> -                .map_err(|err| format_err!("cannot parse '{}': {}", time_str, err))?
>> -                .timestamp();
>> +            let dt_parse_res = chrono::DateTime::parse_from_rfc3339(time_str)
>> +                .map_err(|err| format_err!("cannot parse timestamp '{}': {}", time_str, err));
>> +            endtime = if first_line {
>> +                first_line = false;
>> +                match dt_parse_res {
>> +                    Ok(dt) => dt.timestamp(),
>> +                    // we maybe seeked into the middle of a line,
>> +                    // so ignore errors on the first
>> +                    Err(_) => continue,
>> +                }
>> +            } else {
>> +                dt_parse_res?.timestamp()
>> +            };
>>           } else {
>>               continue;
>>           }
>> -- 
>> 2.20.1
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel