coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: tail --retry not re-attempting to open file


From: Bernhard Voelker
Subject: Re: tail --retry not re-attempting to open file
Date: Wed, 17 Apr 2013 15:39:01 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5

On 04/04/2013 03:42 AM, Pádraig Brady wrote:
> I've nothing against simplicity though since the
> inotify code already deals with missing files when
> following by name, by watching the parent dir,
> it might be easy to adjust to handle this case.

Hi Padraig,

sorry for the late reply.

Somehow, I don't have a good feeling for using inotify in the
--follow=descriptor case because inotify works with file names.

Once tail opens the file successfully after inotify says it's
there, tail would have to go back to polling, because all further
inotify events are only relevant for the file name, not the
descriptor.
>From playing with the inotify code, I even had the impression that
there are not all events delivered to tail reliably.  Is this true?

> If not I'm fine for reverting to polling.
> 
>> @ -2189,7 +2192,8 @@ main (int argc, char **argv)
>>           is recreated, then we don't recheck any new file when
>>           follow_mode == Follow_name  */
>>        if (!disable_inotify && (tailable_stdin (F, n_files)
>> -                               || any_remote_file (F, n_files)))
>> +                               || any_remote_file (F, n_files)
> 
>> +                               || !ok))
> 
> I think that needs to be:
>   || (!ok && follow_mode == Follow_descriptor)))
> 
>>          disable_inotify = true;
>>
>>        if (!disable_inotify)

Good point, thanks!


>>>> Furthermore, I find this warning irritating
>>>>   "warning: --retry is useful mainly when following by name"
>>>
>>> I agree. It's not imparting much info, about why that's supported.
>>> What we should do is print what tail will do in this case.
>>> I propose we do:
>>>
>>> if (retry)
>>>   if (!follow_mode)
>>>     warn ("--retry ignored");
>>>   else if (follow_mode == DESC)
>>>     warn ("--retry only effective for the initial open");
>>
>> I'd go even one step further, and also remove the latter warning.
> 
> I'd be 60:40 for keeping the warning as users could
> easily specify 'tail -f --retry' not realizing it
> wouldn't handle rotated log files for example.

I agree.

>> +# Ensure that "tail --retry --follow=descriptor" waits for the file to 
>> appear.
>> +# tail-8.21 failed at this (since the implementation of the inotify 
>> support).
>> +tail -s.1 --follow=descriptor --retry missing >out 2>&1 & pid=$!
>> +sleep .2             # Wait a bit ...
>> +echo "X" > missing   # ... for the file to appear.
>> +sleep .2             # Then give tail enough time to recognize it
>> +                     # ... and to output the content to out.
>> +kill $pid            # Then kill tail ...
>> +wait $pid            # ... and wait for it to complete.
>> +rm -f missing || fail=1
>> +# Expect 3 lines in the output file ("cannot open" + "has appeared" + "X").
>> +[ $( wc -l < out ) = 3 ]   || { fail=1; cat out; }
>> +grep -F 'cannot open' out  || { fail=1; cat out; }
>> +grep -F 'has appeared' out || { fail=1; cat out; }
>> +grep '^X$' out             || { fail=1; cat out; }
> 
> This looks racy. There is nothing I see that guarantees that
> tail will be scheduled before it's killed.

Good spot.

> I'd maybe try something like:
> 
> timeout 10 tail .....
> until [ $(wc -l < out) = 3 ]; do sleep .1; done
> kill $pid
> wait $pid

Additionally to the above, I noticed that tail now would no longer
exit in the case the file appears untailable, e.g. when it appears
as directory.  I fixed this, too.

After all, I think tail.c is not in a very good, maintainable shape.
There are so many knobs, partially with overlapping meaning like
'follow_mode' vs. 'forever', or 'reopen_inaccessible_files' vs.
f[i].ignore etc., that it's rather hard to do the right change
without breaking something else.
WDYT?

Have a nice day,
Berny

Attachment: tail-f-retry.2.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]