5

I have a batch process running under java JDK 1.7. It is running on a system with RHEL, 2.6.18-308.el5 #1 SMP.

This process gets a list of metadata objects from a database. From this metadata it extracts a path to a file. This file may or may not actually exist.

The process uses the ExecutorService (Executors.newFixedThreadPool()) to launch multiple threads. Each thread runs a Callable that launches a process that reads that file and writes another file if that input file exists (and logs the result) and does nothing if the file does not exist (except log that result).

I find the behavior is indeterminate. Although the actual existence of the each of the files is constant throughout, running this process does not give consistent results. It usually gives correct results but occasionally finds that a few files which really do exist do not. If I run the same process again, it will find the files that it previously said did not exist.

Why might this be happening, and is there an alternative way of doing that would be more reliable? Is it a mistake to be writing files in a multithreaded process while other threads are attempting to read the directory? Would a smaller Thread Pool help (currently 30)?

UPDATE: Here is the actual code of the unix process called by the worker threads in this scenario:

public int convertOutputFile(String inputFile, String outputFile)
throws IOException
{
    List<String> args = new LinkedList<String>();
    args.add("sox");
    args.add(inputFile);
    args.add(outputFile);
    args.addAll(2, this.outputArguments);
    args.addAll(1, this.inputArguments);
    long pStart = System.currentTimeMillis();
    int status = -1;
    Process soxProcess = new ProcessBuilder(args).start();

    try {
        // if we don't wait for the process to complete, player won't
        // find the converted file.
        status = soxProcess.waitFor();
        if (status == 0) {
            logger.debug(String.format("SoX conversion process took %d ms.",
                    System.currentTimeMillis() - pStart));
        } else {
            logger.error("SoX conversion process returned an error status of " + status);
        }
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    return status;
}

UPDATE #2:

I have tried the experiment of switching from java.io.File.exists() to java.nio.Files.exists() and this seems to provide more reliability. I have yet to see the failure condition over multiple attempts, where as before it occurred approximately 10% of the time. So I guess I'm looking to know whether the nio version is somehow more robust in how it handles the underlying File System. This finding was later proven false. nio is no help here.

UPDATE #3: Upon further review I still find the same failure condition occurring. So switching to nio is not a panacea. I've obtained better results by reducing the thread pool size of the executor service to 1. This seems to be more reliable and there is that way no chance of one thread reading the directory while another thread is launching a process that writes to the same directory.

One further possibility that I have not yet investigated is whether I would be better served by putting my output files in a different directory than the input files. I put them in the same directory because it was easier to code, but that may be confusing things, since the output file creation is affecting the same directory as the input directory scan.

UPDATE #4: Recoding so that the output files are written to a different directory than the input files (whose existence is being checked for) does not particularly help things. The only change that helps things is having an ExecutorService thread pool size of 1, in other words, not multithreading this operation.

Steve Cohen
  • 4,219
  • 7
  • 45
  • 84
  • 1
    Your question is very unclear. Please can you post an [MCVE](http://www.stackoverflow.com/help/mcve)? – Andy Turner Jan 06 '16 at 22:42
  • I cannot easily post an MCVE. Sorry. Still, I have indicated that each call of the worker thread launches a Process (a unix Process) and this process reads the file and writes another one (into the same directory), Would File.exists() give bad results given a multithreaded environment in which the contents of the directory are changing, and might nio.Files.exists() give more reliable results? – Steve Cohen Jan 06 '16 at 22:55
  • "Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers." – Andy Turner Jan 06 '16 at 23:04
  • @SteveCohen No. There's much, much more likely a bug in your code. But it's hard to tell what, we don't know anything about which of your jobs does what.in which order and in which thread, or whether there's any race conditions etc. – nos Jan 06 '16 at 23:04
  • Also, I doubt your program is spawning new unix processes. It is likely just spawning native threads, which is completely different than another whole process with a JVM in it. If you were to `ps --forest aux | grep java`, there would not be 30 jvm processes, just one. – MeetTitan Jan 06 '16 at 23:16
  • @MeetTitan - my program is quite definitely, and explicitly, spawning new unix processes. I will update the original question showing precisely how they are launched. These calls are all made in worker threads, after we determine that the input file exists. – Steve Cohen Jan 07 '16 at 14:43
  • And I never said that there were multiple **JVM** processes. – Steve Cohen Jan 07 '16 at 15:23
  • @SteveCohen, Unless you're using `Runtime.exec`, you are most definitely *not* spawning processes, but threads instead. Multiprocessing != multithreading. – MeetTitan Jan 07 '16 at 18:43
  • Hmm, @MeetTitan, javadoc for java.lang.Process says: "The ProcessBuilder.start() and Runtime.exec methods create a native process and return an instance of a subclass of Process that can be used to control the process and obtain information about it. The class Process provides methods for performing input from the process, performing output to the process, waiting for the process to complete, checking the exit status of the process, and destroying (killing) the process. " – Steve Cohen Jan 07 '16 at 18:52
  • Oh, your using ProcessBuilder? Carry on then, citizen. – MeetTitan Jan 07 '16 at 21:03

3 Answers3

4

The real question here is why are you calling it?

  • You have to construct a FileInputStream or FileReader to read a file, and these will throw a FileNotFoundException if the file can't be opened, with absolute reliability.
  • You have to catch exceptions anyway.
  • The operating system has to check whether the file exists anyway.
  • There is no need to check it twice.
  • Existence can change between checking it and opening the file.

So, don't check it twice. Let opening the file do all the work.

Is it a mistake to be writing files in a multithreaded process

I wouldn't say it's a mistake, but it's pretty pointless. The disk isn't multi-threaded.

Would a smaller Thread Pool help (currently 30)?

I would definitely reduce this anyway, to four or so, not to fix this problem but to reduce thrashing and almost certainly improve throughput.

assylias
  • 297,541
  • 71
  • 621
  • 741
user207421
  • 289,834
  • 37
  • 266
  • 440
  • Well, no. There would be no exception thrown, but the spawned Process would fail, and that Process (which launches the executable SoX) does not yield meaningful error codes that would enable me to determine if the reason for failure was FileNotFound which happens to be important information in this case. I may try your suggestion of reducing the number of threads. – Steve Cohen Jan 07 '16 at 14:32
  • While I had already gotten improved reliability by switching from java.io.File.exists() to java.nio.Files.exists(), I will report and upvote your answer because your suggestion of reducing the number of threads did indeed improve throughput. Thanks. – Steve Cohen Jan 07 '16 at 15:18
  • How many threads were running concurrently ? About Files.exists(), have a look at https://docs.oracle.com/javase/tutorial/essential/io/check.html : it is not much safer than File.exists(), so observed improvment migth not be related at all. – Olivier Jan 07 '16 at 15:41
  • 30. I reduced it to 6 as per @EJB and it did improve throughput. – Steve Cohen Jan 07 '16 at 16:11
4

I have marked @Olivier's answer as "the" answer, but I am providing my own here, in order to summarize the findings of my experiment. I am calling it "the" answer for getting closer to the truth than anyone else, even though his guess about File Handles does not seem to be obviously correct, although I can't disprove it either. What does ring true is his simple statement "Your application might be properly multithreaded, whenever you are accessing the FileSystem, it has limitations." This is consistent with my findings. If anyone can shed any further light, I may change this.

  1. Is it a bug in my code?

Highly doubtful. Running the same process repeatedly over the same list of files randomly shows a few files showing as non-existent when they do, in fact, exist. Running the process again, these same files are found to exist. There is zero chance that the existence of these files would have changed in the interim.

  1. Does using java.nio.Files.exists() rather than java.io.File.exists() help?

No. The underlying interface to the file system does not appear to be different. The nio improvements in this area seem to be confined to the handling of links in nio, which is not the issue here. But I can't say for sure, as this is native code.

  1. Does putting the input and output files in different directories, so that my existence checks are not reading the same directory where the output files are getting written to, help?

No. It does not appear to be two simultaneous hits on the directory that causes the problem, so much as two simultaneous hits on the file system.

  1. Does reducing the number of threads in the pool help?

Only reducing it to 1 makes it reliable, in other words only doing away with the multithreaded approach altogether, helps. This operation does not appear to be 100% reliable at least not with this OS and JDK, multithreaded.

If sox were ever to be redesigned so as to give a distinct error code for File Not Found on the input files, this might make the answer of @EJP above feasible.

Steve Cohen
  • 4,219
  • 7
  • 45
  • 84
0

Your application might be properly multithreaded, whenever you are accessing the FileSystem, it has limitations. In your case, I would bet that too many threads are accessing it at the same time, with the consequence that FS runs out of file handle. File instances have no way to tell you that, as exists() do not throw Exception, so they simply return false, even if the directory exists.

Olivier
  • 3,306
  • 2
  • 21
  • 26
  • 1
    While driveby downvotes do suck, your answer is more of a comment. – MeetTitan Jan 06 '16 at 23:27
  • 1
    @Olivier - I upvoted you since you are at least getting at the question I am trying to answer, even if not authoritatively, which is, again, how reliable is File.exists() given a machine on which many reads and writes happen simultaneously. I don't believe File Handles is the issue here. but, forgetting about multithreading in java for a moment, imagine a server in which several INDEPENDENT processes are peforming disk writes. If those writes were sometimes to the same directory, would a java app calling File.exists() occasionally fail? – Steve Cohen Jan 07 '16 at 14:38
  • 1
    Thanks for the upvote, I prefer to try to answer rather to going meta and shout "MCVE !";) File.exist() is inherently error prone as, as I explain in my answer, it returns true if everything goes well AND the directory exists, or false in any other circumstances, basically meaning "I can't confirm it exists" rather than "it doesn't exists". This is applicable to most of the File methods btw. – Olivier Jan 07 '16 at 15:39
  • 1
    My philosophy is similar to yours. There are problems that cannot be easily and concisely stated, and the discussion they provoke can be useful, even if not leading to a quick, neat solution. Might you please comment on my update which reports that java.nio.Files.exists() appears more reliable than java.io.File.exists()? – Steve Cohen Jan 07 '16 at 15:59