Don't call file.length() for every append if we can avoid it #790
Conversation
This change introduces a way for the ResiliantFileOutputStream to track its own position in the file. This means that instead of calling File.length() (which ends up being a syscall) for each event, we only call it when we're recovering a stream.
|
@axiak Hi Michael, Thank you for this contribution. When you say that |
|
|
||
| private final OutputStream delegate; | ||
|
|
||
| private long count; |
There was a problem hiding this comment.
This should probably be volatile. While there is external synchronization using various locks when calling write(), the new state of the variable may not be reflected immediately in other threads after this thread updates it. The increment operations += and ++ are actually each 2 operations, a read and then a write, this means the count may be incorrectly updated by another thread afterwards, as the value it reads could be stale.
|
|
||
| spy.getChannel().close(); | ||
| spy.write("b".getBytes()); | ||
| spy.flush(); | ||
| // we have 2 in our countingoutput stream | ||
| // but the 'b' write failed due to the channel closing |
There was a problem hiding this comment.
This means the count can become inaccurate if any IOException occurs, I think it would be best to mitigate that by either only updating the count after the operation succeeds, or alternatively, if we still want to eagerly increment the count we should catch any exceptions, decrement the count, then rethrow the exception again. This would ensure the count remains accurate.
From my read through the file length check occurs only when the rollover check interval is passed. For all standard This would be an optimization when checking for rollover if someone has a low rollover check interval. Depending on the triggering policy this can vary, for In these limited cases this could be an improvement, in exchange for extra state and complexity. |
This change introduces a way for the ResiliantFileOutputStream to track its own position in the file. This means that instead of calling File.length() (which ends up being a syscall) for each event, we only call it when we're recovering a stream.