Re: [PATCH] tail should dereference watched file

[Pádraig Brady]

On 11/22/2013 02:32 AM, Bernhard Voelker wrote:
> On 11/21/2013 06:01 PM, Pádraig Brady wrote:
>> On 09/16/2013 11:11 PM, Pádraig Brady wrote:
>>> On 09/15/2013 10:43 PM, Bernhard Voelker wrote:
>>>> On 09/15/2013 08:58 PM, Pádraig Brady wrote:
>>>>> True. Perhaps we should revert to polling for symlinks?
>>>>
>>>> That seems to be the safest way, yes.
>>>
>>> Though we'd have to be careful to handle the case
>>> where a file was replaced with a symlink.
>>
>> Hopefully the attached fixes up the common case of this,
>> and diagnose the edge case where a (missing) file appears
>> as a symlink, during inotify processing.
> 
> Hi Padraig,
> 
> Thanks for continue working on this - I almost forgot it.
> 
> The first part is minor nits in the test suite:
> 
>> --- /dev/null
>> +++ b/tests/tail-2/symlink.sh
> 
>> +
>> +# Ensure changing targets of cli specified symlinks are handled
> 
> s/$/./
> 
>> +# Prior to v8.22, inotify would fail to recognize changes in the targets
> 
> s/$/./
> 
>> +# Clear 'out' so that we can check its contents without races
> 
> s/$/./
> 
>> +:>out                           || framework_failure_
>> +ln -nsf target symlink          || framework_failure_
>> +timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$!
>> +retry_delay_ wait4lines_ .1 6 1 || fail=1  # Wait for "cannot open..."
>> +echo "X" > target               || fail=1
>> +retry_delay_ wait4lines_ .1 6 3 || fail=1  # Wait for the expected output.
>> +kill $pid
>> +wait $pid
>> +# Expect 3 lines in the output file.
>> +[ $( 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; }
>> +rm -f target out           || framework_failure_
> 
> Looks good.
> 
>> +
>> +# Ensure we correctly handle the source symlink itself changing.
>> +# I.E. that we don't operate solely on the targets.
>> +# Clear 'out' so that we can check its contents without races
>> +:>out                           || framework_failure_
>> +touch target1                   || framework_failure_
> 
> Why not use ":>target1" here, too? ... or alternatively move
> the 'echo "X1" > target1' from 3 lines below to here?
> 
>> +ln -nsf target1 symlink         || framework_failure_
>> +timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$!
>> +echo "X1" > target1
>> +retry_delay_ wait4lines_ .1 6 1 || fail=1  # Wait for the expected output.
>> +ln -nsf target2 symlink         || framework_failure_
>> +retry_delay_ wait4lines_ .1 6 2 || fail=1  # Wait for "become inaccess..."
>> +echo "X2" > target2             || fail=1
>> +retry_delay_ wait4lines_ .1 6 4 || fail=1  # Wait for the expected output.
>> +kill $pid
>> +wait $pid
>> +# Expect 3 lines in the output file.
> 
> s/3/4/ ;-)
> 
>> +[ $( wc -l < out ) = 4 ]   || { fail=1; cat out; }
>> +grep -F 'become inaccessible' out  || { fail=1; cat out; }
>> +grep -F 'has appeared' out || { fail=1; cat out; }
>> +grep '^X[12]$' out             || { fail=1; cat out; }
> 
> Better to test for X1 and X2 separately, to avoid eclipsing to determine
> when one of the above wait4lines_ call ran into a timeout.
> 
> 
>> +rm -f target1 target2 out  || framework_failure_
>> +
>> +# Note other symlink edge cases are currently just diagnosed
>> +# rather than being handled. I.E. if you specify a missing item,
>> +# or existing file that later change to a symlink, if inotify
>> +# is in use, you'll get a diagnostic saying that link will
>> +# no longer be tailed.
>> +
>> +Exit $fail
> 
> 
> Now regarding the functionality change:
> I can't say what it is, but the previous behavior was somehow
> better in certain cases [*].
> 
> Compare the output of
>   tail -F -s1 symlink
> for the old (413a2415) and new program when executing the following
> in a 3rd terminal.
> 
> Preparation:
> 
>   echo X > target1
>   echo Y > target2
>   ln -nsf target1 symlink
> 
> Now start both tail-{old,new} and do the following:
> 
>   ln -nsf target2 symlink
>   ln -nsf target1 symlink
>   ln -nsf target2 symlink
>   rm symlink
>   mkdir symlink
>   rmdir symlink
>   ln -nsf target1 symlink
>   ln -nsf target2 symlink
>   rm symlink
>   echo Z > symlink
> 
> I didn't dig into what's going on, but from a certain
> point on, tail-new doesn't output anything anymore.
> Can you reproduce this?

So what's going on here is that in poll mode
we don't notice changes to the "symlink" name
after we give up on it.

So I suppose you could consider this a
limitation of poll mode but it is diagnosed
and a far out edge case.

You can get old and new to behave the same
for confirmation by running both with:

  tail -F -s.1 ---dis symlink

thanks,
Pádraig.