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_r167682325
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java
##########
@@ -113,44 +113,46 @@ public void sort(String name, Path srcPath, String
destPath) {
// the following call does not throw an exception if the file/dir does
not exist
fs.deleteRecursively(new Path(destPath));
- DFSLoggerInputStreams inputStreams;
- try {
- inputStreams = DfsLogger.readHeaderAndReturnStream(fs, srcPath,
conf);
- } catch (LogHeaderIncompleteException e) {
- log.warn("Could not read header from write-ahead log " + srcPath +
". Not sorting.");
- // Creating a 'finished' marker will cause recovery to proceed
normally and the
- // empty file will be correctly ignored downstream.
- fs.mkdirs(new Path(destPath));
- writeBuffer(destPath, Collections.<Pair<LogFileKey,LogFileValue>>
emptyList(), part++);
- fs.create(SortedLogState.getFinishedMarkerPath(destPath)).close();
- return;
- }
+ try (final FSDataInputStream fsinput = fs.open(srcPath)) {
+ DFSLoggerInputStreams inputStreams;
+ try {
+ inputStreams = DfsLogger.readHeaderAndReturnStream(fsinput, conf);
Review comment:
I'm not exactly sure what kind of additional resources the crypto stream
might use, but it seems to me that since `inputStreams` is not `Closeable`, it
may be possible for an the `fsinput` to be closed, but the crypto stream in
`inputStreams` to still be holding resources. Would need to dig further to be
sure, but it would be nice if `DFSLoggerInputStreams` was `Closeable` and this
was created as a second resource, in the `try-with-resources` block on line 116.
Same comment applies to other uses of `DFSLoggerInputStreams`.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services