tail --retry not re-attempting to open file

[Noel Morrison]

tail --retry -f {FILE} or tail -f --retry {FILE} does continue to try to
open the file.
I just compiled 8.21 CoreUtils and found that the problem is still present.

I don't have any heavy programing experience. My 1st official report of any
bugs.

[Bernhard Voelker]

As the following is contradicting the "Subject:" line, I'm assuming you
meant the "--retry not re-attempting to open" variant.

tail --retry -f {FILE} or tail -f --retry {FILE} does continue to try to open the file.
I just compiled 8.21 CoreUtils and found that the problem is still present.

Thank you for the report. However, I believe it is a result of
a misinterpretation of tail's options.

As you can read from the --help output and - more thoroughly - in
the texinfo documentation, `tail -f --retry FILE` is following the
content of FILE by file descriptor:

-f, --follow[={name|descriptor}]
output appended data as the file grows;
-f, --follow, and --follow=descriptor are
equivalent
-F same as --follow=name --retry
[...]
--retry keep trying to open a file even when it is or
becomes inaccessible; useful when following by
name, i.e., with --follow=name

Once "tail -f" holds a handle to the open file descriptor, it will
not try to reopen the file again. Instead, it uses polling or inotify
(depending on the underlying file system) to follow the content of
the file.

Furthermore, you get the following warning when --retry is used
together with -f (or --follow=descriptor):

$ tail -f --retry /dev/null
tail: warning: --retry is useful mainly when following by name
^C

Does that answer your question?

My 1st official report of any bugs.

BTW: the address ***@gnu.org is the main discussion list which is
used for general questions and discussions, while real bug reports would
go to bug-***@gnu.org. An email to the latter opens a new bug report
in our bug tracking system and therefore makes it easier to follow bugs;
OTOH this induces some overhead regarding status tracking which is not
suitable for general discussions.

Have a nice day,
Berny

[Bernhard Voelker]

re-adding the list.

Sorry to inform you but in the standard tail command even if a file is
not present, the tail -f --retry would retry at some interval to attach
to the filename . If the file would become present or readable it would
at that time open and start following the appended results. I can show
proof of the command and its results. Please check this out on any 5.x
RHEL System, that uses the standard tail. I've not checked my debian box
at the house. Maybe this is a difference on how GNU handles it, but in
my experience with tail this --retry is not performing as expected.

Oooh - I tested on RHEL 5.4 with coreutils-v5.97, and it's true:
tail -f --retry continues to try to open the given file there
while tail from latest Git (or 8.21) does not.

There is no obvious commit regarding this, but this seems to be
related with the inotify support. As inotify_watch() does not
succeed (because of ENOENT), tail terminates.
If tail is forced to revert to polling, e.g. by the (undocumented)
---disable-inotify option, then tail will start to try opening the
file again and therefore waits until the given file name appears.

Furthermore, I find this warning irritating
"warning: --retry is useful mainly when following by name"
because for polling, this option really makes sense
... and a difference: if --retry is specified, tail would retry
to open the file while it would not otherwise. Or the other way
round: --retry may only not be useful together with inotify.

I don't know if this behavior is intended.
Others?

Have a nice day,
Berny

[Pádraig Brady]

Post by Bernhard Voelker
re-adding the list.

Sorry to inform you but in the standard tail command even if a file is
not present, the tail -f --retry would retry at some interval to attach
to the filename . If the file would become present or readable it would
at that time open and start following the appended results. I can show
proof of the command and its results. Please check this out on any 5.x
RHEL System, that uses the standard tail. I've not checked my debian box
at the house. Maybe this is a difference on how GNU handles it, but in
my experience with tail this --retry is not performing as expected.

tail -f --retry continues to try to open the given file there
while tail from latest Git (or 8.21) does not.
There is no obvious commit regarding this, but this seems to be
related with the inotify support. As inotify_watch() does not
succeed (because of ENOENT), tail terminates.
If tail is forced to revert to polling, e.g. by the (undocumented)
---disable-inotify option, then tail will start to try opening the
file again and therefore waits until the given file name appears.

Yes I agree that this is a regression associated with the inotify support.
I take `tail --follow=descriptor --retry` to mean,
Wait for the file to appear initially and follow even if renamed or unlinked.

Post by Bernhard Voelker
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");

Post by Bernhard Voelker
because for polling, this option really makes sense
... and a difference: if --retry is specified, tail would retry
to open the file while it would not otherwise. Or the other way
round: --retry may only not be useful together with inotify.

I couldn't really follow (pardon the pun) what you were saying there.
But there should be no functional difference between polling and inotify,
only timing and efficiency differences.

thanks,
Pádraig.

[Bernhard Voelker]

Hi Padraig,

Post by Pádraig Brady
Yes I agree that this is a regression associated with the inotify support.
I take `tail --follow=descriptor --retry` to mean,
Wait for the file to appear initially and follow even if renamed or unlinked.

The problem is that tail_forever_inotify() returns false when the
initial open in tail_file() failed. This makes tail exit immediately.
Without intense reading of the source, I guess it's not easy to
use inotify in such a case. Therefore, falling back to polling
seems to be quite okay.
The attached patch implement this in a one-liner:

@ -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))
disable_inotify = true;

if (!disable_inotify)

Post by Pádraig Brady

Post by Bernhard Voelker
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.
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.
It fits better into the texinfo documentation.
The attached patch includes that, too.

Finally, the patch comes with a new test, NEWS etc.

WDYT?

Have a nice day,
Berny

[Pádraig Brady]

Post by Bernhard Voelker
Hi Padraig,

Post by Pádraig Brady
Yes I agree that this is a regression associated with the inotify support.
I take `tail --follow=descriptor --retry` to mean,
Wait for the file to appear initially and follow even if renamed or unlinked.

The problem is that tail_forever_inotify() returns false when the
initial open in tail_file() failed. This makes tail exit immediately.
Without intense reading of the source, I guess it's not easy to
use inotify in such a case. Therefore, falling back to polling
seems to be quite okay.

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.
If not I'm fine for reverting to polling.

Post by Bernhard Voelker
@ -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)))

Post by Bernhard Voelker
disable_inotify = true;
if (!disable_inotify)

Post by Pádraig Brady

Post by Bernhard Voelker
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.
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.

Post by Bernhard Voelker
+# 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.
I'd maybe try something like:

timeout 10 tail .....
until [ $(wc -l < out) = 3 ]; do sleep .1; done
kill $pid
wait $pid

thanks,
Pádraig.

[Bernhard Voelker]

Post by Pádraig Brady
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.

[Pádraig Brady]

Post by Bernhard Voelker

Post by Pádraig Brady
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.

Thanks for the update.
Will review ASAP.

Post by Bernhard Voelker
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?

I agree that the current inotify functionality isn't ideal.

inotify advantages
immediate output of changes
(but one can always use -s.1)
could be used to watch *.log in a local directory
(but we don't currently leverage inotify to provide this functionality)

inotify disadvantages
not used for stdin
can't be used for remote files
doesn't work for remounts
only works on linux
more complicated

Also in the meantime the more encompassing fanotify Linux API
has become available which might simplify implementation
and/or provide new functionality.

At this stage I wouldn't be on for removing the current
inotify implementation, but also I wouldn't be enthused about
adding any large refactoring or new functionality based on it.
Any large changes (like supporting watching any new files in a dir for e.g.)
should probably consider using fanotify or some other mechanism instead.

thanks,
Pádraig.

[Pádraig Brady]

Post by Bernhard Voelker

Post by Pádraig Brady
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.

[Bernhard Voelker]

Post by Bernhard Voelker
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.

[Bernhard Voelker]

You may want to consider using the existing retry_delay_()
rather than wait4lines() in the tests, but otherwise it
looks good to push.

Thanks for the review.
I'll have a look at retry_delay_() again tomorrow, and see
if it makes waiting for tail's output easier.

Hi Padraig,

as there are different line counts expected in the 'out' file,
I needed to change retry_delay_(). it can now be called with
more than three arguments (test_func, init_delay, max_n_tries)
in which case it passes such values on to test_func.

Unless there are objection, I'd push the attached tomorrow.

Have a nice day,
Berny

[Pádraig Brady]

Post by Bernhard Voelker

You may want to consider using the existing retry_delay_()
rather than wait4lines() in the tests, but otherwise it
looks good to push.

Thanks for the review.
I'll have a look at retry_delay_() again tomorrow, and see
if it makes waiting for tail's output easier.

Hi Padraig,
as there are different line counts expected in the 'out' file,
I needed to change retry_delay_(). it can now be called with
more than three arguments (test_func, init_delay, max_n_tries)
in which case it passes such values on to test_func.
Unless there are objection, I'd push the attached tomorrow.

Looks great,

thanks,
Pádraig.

[Bernhard Voelker]

Post by Pádraig Brady
Looks great,

Thanks for the review!

Pushed:
http://git.sv.gnu.org/cgit/coreutils.git/commit/?id=8f976798
http://git.sv.gnu.org/cgit/coreutils.git/commit/?id=d461bfd2

Have a nice day,
Berny