ctubbsii commented on a change in pull request #341: ACCUMULO-3902 Ensure [Batch]Scanners are closed in ITs URL: https://github.com/apache/accumulo/pull/341#discussion_r158130498
########## File path: test/src/main/java/org/apache/accumulo/test/FileArchiveIT.java ########## @@ -192,81 +194,88 @@ public void testUnusuedFilesAndDeletedTable() throws Exception { // Compact memory to disk conn.tableOperations().compact(tableName, null, null, true, true); - Scanner s = conn.createScanner(MetadataTable.NAME, Authorizations.EMPTY); - s.setRange(MetadataSchema.TabletsSection.getRange(tableId)); - s.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME); - - Entry<Key,Value> entry = Iterables.getOnlyElement(s); - final String file = entry.getKey().getColumnQualifier().toString(); - final Path p = new Path(file); - - // Then force another to make an unreferenced file - conn.tableOperations().compact(tableName, null, null, true, true); - - log.info("File for table: {}", file); - - FileSystem fs = getCluster().getFileSystem(); - int i = 0; - while (fs.exists(p)) { - i++; - Thread.sleep(1000); - if (0 == i % 10) { - log.info("Waited {} iterations, file still exists", i); + Scanner s = null; + try { + s = conn.createScanner(MetadataTable.NAME, Authorizations.EMPTY); + s.setRange(MetadataSchema.TabletsSection.getRange(tableId)); + s.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME); + + Entry<Key,Value> entry = Iterables.getOnlyElement(s); + final String file = entry.getKey().getColumnQualifier().toString(); + final Path p = new Path(file); + + // Then force another to make an unreferenced file + conn.tableOperations().compact(tableName, null, null, true, true); + + log.info("File for table: {}", file); + + FileSystem fs = getCluster().getFileSystem(); + int i = 0; + while (fs.exists(p)) { + i++; + Thread.sleep(1000); + if (0 == i % 10) { + log.info("Waited {} iterations, file still exists", i); + } } - } - log.info("File was removed"); + log.info("File was removed"); - String filePath = p.toUri().getPath().substring(getCluster().getConfig().getAccumuloDir().toString().length()); + String filePath = p.toUri().getPath().substring(getCluster().getConfig().getAccumuloDir().toString().length()); - log.info("File relative to accumulo dir: {}", filePath); + log.info("File relative to accumulo dir: {}", filePath); - Path fileArchiveDir = new Path(getCluster().getConfig().getAccumuloDir().toString(), ServerConstants.FILE_ARCHIVE_DIR); + Path fileArchiveDir = new Path(getCluster().getConfig().getAccumuloDir().toString(), ServerConstants.FILE_ARCHIVE_DIR); - Assert.assertTrue("File archive directory didn't exist", fs.exists(fileArchiveDir)); + Assert.assertTrue("File archive directory didn't exist", fs.exists(fileArchiveDir)); - // Remove the leading '/' to make sure Path treats the 2nd arg as a child. - Path archivedFile = new Path(fileArchiveDir, filePath.substring(1)); + // Remove the leading '/' to make sure Path treats the 2nd arg as a child. + Path archivedFile = new Path(fileArchiveDir, filePath.substring(1)); - Assert.assertTrue("File doesn't exists in archive directory: " + archivedFile, fs.exists(archivedFile)); + Assert.assertTrue("File doesn't exists in archive directory: " + archivedFile, fs.exists(archivedFile)); - // Offline the table so we can be sure there is a single file - conn.tableOperations().offline(tableName, true); + // Offline the table so we can be sure there is a single file + conn.tableOperations().offline(tableName, true); - // See that the file in metadata currently is - s = conn.createScanner(MetadataTable.NAME, Authorizations.EMPTY); - s.setRange(MetadataSchema.TabletsSection.getRange(tableId)); - s.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME); + // See that the file in metadata currently is + s = conn.createScanner(MetadataTable.NAME, Authorizations.EMPTY); + s.setRange(MetadataSchema.TabletsSection.getRange(tableId)); + s.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME); - entry = Iterables.getOnlyElement(s); - final String finalFile = entry.getKey().getColumnQualifier().toString(); - final Path finalPath = new Path(finalFile); + entry = Iterables.getOnlyElement(s); + final String finalFile = entry.getKey().getColumnQualifier().toString(); + final Path finalPath = new Path(finalFile); - conn.tableOperations().delete(tableName); + conn.tableOperations().delete(tableName); - log.info("File for table: {}", finalPath); + log.info("File for table: {}", finalPath); - i = 0; - while (fs.exists(finalPath)) { - i++; - Thread.sleep(1000); - if (0 == i % 10) { - log.info("Waited {} iterations, file still exists", i); + i = 0; + while (fs.exists(finalPath)) { + i++; + Thread.sleep(1000); + if (0 == i % 10) { + log.info("Waited {} iterations, file still exists", i); + } } - } - log.info("File was removed"); + log.info("File was removed"); - String finalFilePath = finalPath.toUri().getPath().substring(getCluster().getConfig().getAccumuloDir().toString().length()); + String finalFilePath = finalPath.toUri().getPath().substring(getCluster().getConfig().getAccumuloDir().toString().length()); - log.info("File relative to accumulo dir: {}", finalFilePath); + log.info("File relative to accumulo dir: {}", finalFilePath); - Assert.assertTrue("File archive directory didn't exist", fs.exists(fileArchiveDir)); + Assert.assertTrue("File archive directory didn't exist", fs.exists(fileArchiveDir)); - // Remove the leading '/' to make sure Path treats the 2nd arg as a child. - Path finalArchivedFile = new Path(fileArchiveDir, finalFilePath.substring(1)); + // Remove the leading '/' to make sure Path treats the 2nd arg as a child. + Path finalArchivedFile = new Path(fileArchiveDir, finalFilePath.substring(1)); - Assert.assertTrue("File doesn't exists in archive directory: " + finalArchivedFile, fs.exists(finalArchivedFile)); + s.close(); + Assert.assertTrue("File doesn't exists in archive directory: " + finalArchivedFile, fs.exists(finalArchivedFile)); + } finally { + if (s != null) { Review comment: This finally block indicates another possible opportunity for try-with-resources. ---------------------------------------------------------------- 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