Problem
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> logint currentTermint 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();
}
}
Solution
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.