Search code examples
javafileoutputstream

How to correctly close OutputStream?


Below you find the constructor of a server in order to setup it. In a few words, it restore its state from the last time that it was up reading a List of objects (called log) and two int (called currentTerm and votedFor). Any time that any of these "volatile" fields will be updated, the relative file will have to be updated too (and so make upadate the consistent state), so I need the two ObjectOutPutStream called metadataWriter and logWriter. Since the server could go down in any moment, I cannot write any close() method. Do you think that the only possible solution in order to avoid an EOFException the next time that I setup the server (during the reading operation) is to flush() the output stream everytime (like I've done on the last lines of the code)?

   public ServerRMI(...)
    {
        ...
        log = new ArrayList<>();
        try {
            //if Log file exists then should exists Metadata too
            if(Files.exists(Paths.get("Server" + id + "Log"), LinkOption.NOFOLLOW_LINKS))
            {
                reader = new ObjectInputStream(new FileInputStream("Server"+id+"Log"));
                try
                {
                    while(true)
                        log.add((LogEntry) reader.readObject());
                }
                catch (EOFException e){}
                catch (ClassNotFoundException e) {
                    e.printStackTrace();
                }
                reader = new ObjectInputStream(new FileInputStream("Server"+id+"Metadata"));
                currentTerm = reader.readInt();
                votedFor = reader.readInt();
            }
            else//if it is the first time that the server is set up initialize all persistent fields
            {
                currentTerm = 1;
                votedFor = -1;
                log = new ArrayList<LogEntry>();
            }
            logWriter = new ObjectOutputStream(new FileOutputStream("Server"+id+"Log"));
            metadataWriter = new ObjectOutputStream(new FileOutputStream("Server" + id + "Metadata"));
            //since creating a new ObjectOutputStream overwrite the old file with the an empty one, as first thing we rewrite the old content
            for(LogEntry entry : log)
                logWriter.writeObject(entry);
            metadataWriter.writeInt(currentTerm);
            metadataWriter.writeInt(votedFor);
            metadataWriter.flush();
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }
        ...
    }

IMPORTANT NOTE:

I don't think that the try-with-resources in every method which use them (and so each method that update server's state) as someone have suggested is a feasible solution, since it would be ok for the metadata file (the two int written on it are always replaced at every update/write to file) but for the log file it would mean to write the whole list every time that it is changed (the operation made on it are not only appen, but replacement too!), which I think that it's not so good for performance!


Solution

  • I suggest you use try-with-resource and write the whole collection at once and you don't have to read until you get an exception. You can write meta data at the start.

    List<LogEntry> entries = ..
    try (ObjectOutputStream out = new ObjectOutputStream(....)) {
        out.writeObject(currentTerm);
        out.writeObject(votedeFor);
        out.writeObject(entries);
    }
    

    to read you can do

    String currentTerm = "none";
    String votedFor = "no-one";
    List<LogEntry> entries = Collections.emptyList();
    try (ObjectInputStream in = new ObjectInputStream(....)) {
        currentTerm = (String) in.readObject();
        votedFor = (String) in.readObject();
        entries = (List<LogEntry>) in.readObject();
    }
    

    Note: this needs Java 7, but given it's about to be EOL, I suggest upgrading to Java 8 if you can.

    which I think that it's not so good for performance!

    This is a very different question but you would still use try-with-resource. A solution is to only append new entries. To do this you need,

    • a protocol on top of ObjectOutputStream which supports appending to a file. ObjectOutputStream won't do this for you, instead you need to write blocks of data and you need a means to sticking these together.
    • a way to determine what has changed/added since the last write to disk and only write the changes to disk.

    Befor eyou assume performance is bad, you should test it as you might find it is not worth the added complexity to save a few milli-second.

    BTW if you really care about performance, don't use ObjectOutputStream, it is generic and flexible, but very slow.

    so the OutpuStreamObject (which are class field, not constructor local variable) are not correctly closed.

    In this case the file will be corrupted. You need a means of validating the file and truncating invalid data.


    Take a look at a library I wrote. Chronicle Queue It may solve many of your problems as it supports

    • continuous writing from multiple threads, processes or machines.
    • if a process or thread dies while writing it, the corrupted entry is truncated or ignored.
    • if a process dies between writes, there is no loss of data.
    • it support more efficient means of serialization.
    • it can be read by multiple threads/processes while it is being written with micro-second latency.
    • it can be replicated to multiple machines for distribution.
    • entries can be randomly accesses by an index or by binary search.