1
\$\begingroup\$

I have a log file which is constantly updated with new lines of data. I need to get new added data in java as soon as it's written. For now my solution is:

public static void readNonStop(String filename, boolean goToEnd, FileReadCallback readCallback) {
    if(readCallback == null) {
        return;
    }
    try {
        BufferedReader br = new BufferedReader(new FileReader(filename));
        try {
            String line = br.readLine();
            int lineNumber = 0;

            if(goToEnd) {
                while(br.readLine() != null) {}
            }

            while (true) {
                if(line != null) {
                    readCallback.onRead(lineNumber++, line);
                } else {
                    Thread.sleep(1);
                }
                line = br.readLine();
            }
        } finally {
            br.close();
        }
    } catch (Exception e) {
        e.printStackTrace();
    }
}

But I have a feeling that there should be a better way. I don't like the idea of a constant running loop with a "sleep" inside and would prefer some sort of an event driven approach.

If I rely on FileSystem events to reopen the file each time it is modified, it introduces a delay.

What is the correct way of doing it for this situation?

\$\endgroup\$
  • 1
    \$\begingroup\$ Consider Tailer class from Apache Commons. \$\endgroup\$ – vnp Dec 27 '18 at 20:30
  • \$\begingroup\$ Welcome to Code Review. I hope you get some good reviews, and I hope to see more of your contributions here in future! \$\endgroup\$ – Zeta Dec 27 '18 at 20:33
  • \$\begingroup\$ @vnp, wow, looks like what I need. While a bit more robust, Tailer uses the same logic as my code, it reads in a loop with a Thread.sleep delay. But it feels good to confirm that I had the right idea, thanks! @Zeta, thank you! \$\endgroup\$ – Arthur Dec 27 '18 at 20:43
  • 1
    \$\begingroup\$ Maybe there is a "native" solution too: dzone.com/articles/how-watch-file-system-changes or docs.oracle.com/javase/tutorial/essential/io/notification.html \$\endgroup\$ – Timothy Truckle Dec 27 '18 at 20:45
  • 1
    \$\begingroup\$ WatcherService introduces a delay up to 6 seconds in my tests, so unfortunately it is not an option. Looks like I will go with a looped read. \$\endgroup\$ – Arthur Dec 27 '18 at 20:48
2
\$\begingroup\$

You can use the Java WatchService as described here. You said that WatchService is too slow for you. There are other people who have had that problem and resolved it https://stackoverflow.com/questions/9588737/is-java-7-watchservice-slow-for-anyone-elsein this thread.

On a side note, consider using ScheduledThreadPoolExecutor rather than Thread.sleep. The former will recreate threads if they are killed by an exception, has the potential to reuse threads in a pool, and has other potential advantages.

\$\endgroup\$
1
\$\begingroup\$

try-with-resources

BufferedReader implements AutoCloseable, so instead of

    try {
        BufferedReader br = new BufferedReader(new FileReader(filename));
        try {

You can say

    try (BufferedReader br = new BufferedReader(new FileReader(filename))) {

and then get rid of your finally block.

        } finally {
            br.close();
        }

Also, this should allow you to merge the two try statements into one, as the resource declaration is inside the scope of the try if it throws an exception.

Odd behavior

            String line = br.readLine();
            int lineNumber = 0;

So you read the first line of the file. Then, if a Boolean is true, you skip all the other lines of the file without counting them (even though you just declared a variable to count them). Then you process the first line of the file. Why not

        if (goToEnd) {
            while (br.readLine() != null) {}
        }

        // we only want to count lines past the current end of file
        int lineNumber = 0;
        while (true) {
            String line = br.readline();
            while (line == null) {
                Thread.sleep(1);
                line = br.readLine();
            }

            readCallback.onRead(lineNumber++, line);
        }

Now it's clearer that lineNumber has nothing to do with the part before the end of the current file. And we don't process the first line of the file and then a much later line. Each line lasts only one iteration of the loop.

If the condition is false, a while acts just like an if. But if the condition is true, we can stay in the loop.

If this is not the behavior that you want, please add comments to your code explaining why. E.g. "We always need to read the first line of the file as line number 0, even if we skip the rest of the existing lines. This is because the first line has the column headers."

\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer, though lineNumber is not relevant anymore. \$\endgroup\$ – Arthur Dec 28 '18 at 9:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.