2
\$\begingroup\$

I have a server which has a permanent state which has to be saved on stable storage. It is composed by three fields:

  • List<LogEntry> log
  • int currentTerm
  • int votedFor

They have to be set during the server setup (in other words, when the server object is created). Every time that I update their "volatile" value, the relative file must be updated. Until the first time that any of these fields is updated, the value of the relative file must not be changed (in this way if the server goes down before the first udpate, then the state files remain untouched). As last thing, usually the only operation made on the log field is the append operation, but it is possible to replace elements too.

I'd like to have your opinion about the server setup operation (in other words, the constructor). Below you will find my code and solution, but I don't like the fact that when you create a ObjectOutPutStream the original file is overwritten with a new one, but I didn't think anything better than this.

public ServerRMI(...)
{
    ...
    ObjectInputStream reader;
    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
            {
                //until there are LogEntry objects in the file, read them and add them to log
                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);
    } catch (FileNotFoundException e) {
        e.printStackTrace();
    } catch (IOException e) {
        e.printStackTrace();
    }
}
\$\endgroup\$
2
\$\begingroup\$

Because you can't update a written value without replacing the value, you have no other possibility to overwrite your metadata (currentTerm and votedFor) if they had changed.

For the log file you can use another approach by using an appendable FileOutputStream, but you will get problems if you just use

logWriter = new ObjectOutputStream(new FileOutputStream("Server"+id+"Log", true));  

because the ObjectOutputStream is always writing a header first.

To overcome this problem you need to subclass the ObjectOutputStream like

public class AppendableObjectOutputStream extends ObjectOutputStream {

    public AppendableObjectOutputStream(OutputStream out) throws IOException {
        super(out);
    }
    private boolean append = false;
    public AppendableObjectOutputStream(String fileName, boolean appendIfExists) throws IOException {
        super(new FileOutputStream(fileName, appendIfExists && Files.exists(Paths.get(fileName), LinkOption.NOFOLLOW_LINKS)));
        append = appendIfExists && Files.exists(Paths.get(fileName), LinkOption.NOFOLLOW_LINKS);
    }

    @Override
    protected void writeStreamHeader() throws IOException {
        if (append) {
            reset();
        } else {
            super.writeStreamHeader();
        }
    }
}

See also: https://stackoverflow.com/a/1195078/2655508


Using braces {} for single statements while loops too will make your code less error prone.


A ObjectInputStream should like any other stream be closed after the usage is finished.


//if Log file exists then should exists Metadata too  

Assuming that something should be can lead to problems. One could have deleted the metadatafile and you would get problems.


Your method is doing to many things. You should extract parts of it to separate methods. Like

private List<LogEntry> readLogFile(String fileName) throws IOException {
    List<LogEntry> logs = new ArrayList<>();
    if (Files.exists(Paths.get(fileName), LinkOption.NOFOLLOW_LINKS)) {
        ObjectInputStream reader = new ObjectInputStream(new FileInputStream(fileName));
        try {
            //until there are LogEntry objects in the file, read them and add them to log
            while (true) {
                logs.add((LogEntry) reader.readObject());
            }
        } catch (EOFException e) {
        } catch (ClassNotFoundException e) {
            e.printStackTrace();
        } finally {
            if(reader != null){
                reader.close();
            }
        }
    }
    return logs;
}  

For reading the metadatafile I would suggest you create a MetaData class which holds the voteFor and Term and is beeing written by using writeObject() and read by readObject() methods. The voteFor and Term should be initialized in the constructor to voteFor = -1 and Term = 1.

This would give you the possibility to use this method

private MetaData readMetaDataFile(String fileName) throws IOException {
    if (Files.exists(Paths.get(fileName), LinkOption.NOFOLLOW_LINKS)) {
        ObjectInputStream reader = new ObjectInputStream(new FileInputStream(fileName));
        try {
            return ((MetaData) reader.readObject());

        } catch (EOFException e) {
        } catch (ClassNotFoundException e) {
            e.printStackTrace();
        } finally {
            if(reader != null){
                reader.close();
            }
        }
    }
    return new MetaData ();
}

Implementing the above mentioned points will lead to

MetaData metaData = null;
String logFileName = "Server" + id + "Log";
String metaDataFileName = "Server"+id+"Metadata";
try {
    log = readLogFile(logFileName );
    metaData = readMetaData(metaDataFileName);

    logWriter = new AppendableObjectOutputStream(logFileName, true);
    metadataWriter = new AppendableObjectOutputStream(metaDataFileName, false);

} catch (FileNotFoundException e) {
    e.printStackTrace();
} catch (IOException e) {
    e.printStackTrace();
}  

but you should create the metadataWriter only if you really write the data.

\$\endgroup\$

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.