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