ctubbsii commented on a change in pull request #369: [ACCUMULO-4787] Close
input stream in AccumuloReplicaSystem
URL: https://github.com/apache/accumulo/pull/369#discussion_r167680196
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
##########
@@ -414,7 +413,7 @@ public static DFSLoggerInputStreams
readHeaderAndReturnStream(VolumeManager fs,
}
} catch (EOFException e) {
- log.warn("Got EOFException trying to read WAL header information,
assuming the rest of the file (" + path + ") has no data.");
+ log.warn("Got EOFException trying to read WAL header information,
assuming the rest of the file has no data.");
Review comment:
I'm not a fan of both logging *and* throwing. I think it's generally better
to pick one or the other: handle an exception (by logging in this case) or pass
it up. Dropping the path makes it even more obvious that this might be a
redundant message... since my first instinct was to suggest adding `, e` to the
`log.warn` method call, but that's probably not needed if the
`LogHeaderIncompleteException` is keeping the stack trace for logging later.
It's not a big deal... but something to think about if there's an obvious
cleanup here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
