kevinrr888 commented on code in PR #4465:
URL: https://github.com/apache/accumulo/pull/4465#discussion_r2754792070
##########
server/base/src/main/java/org/apache/accumulo/server/util/FindCompactionTmpFiles.java:
##########
Review Comment:
```suggestion
```
Logged again in execute()
##########
test/src/main/java/org/apache/accumulo/test/functional/FindCompactionTmpFilesIT_SimpleSuite.java:
##########
@@ -216,4 +218,57 @@ public void testFindCompactionTmpFilesMainWithDelete()
throws Exception {
}
}
+ @Test
+ public void testFindCompactionTmpFilesAdminCommand() throws Exception {
+
+ try (AccumuloClient c =
Accumulo.newClient().from(getClientProps()).build()) {
+
+ String tableName = getUniqueNames(1)[0];
+ c.tableOperations().create(tableName);
+ ReadWriteIT.ingest(c, 100, 1, 1, 0, tableName);
+ c.tableOperations().flush(tableName);
+
+ String tableId = c.tableOperations().tableIdMap().get(tableName);
+ TableId tid = TableId.of(tableId);
+
+ ServerContext ctx = getCluster().getServerContext();
+ FileSystem fs = getCluster().getFileSystem();
+
+ Set<String> tablesDirs = ctx.getTablesDirs();
+ assertEquals(1, tablesDirs.size());
+
+ String tdir = tablesDirs.iterator().next() + "/" + tid.canonical() +
"/default_tablet";
+ Path defaultTabletPath = new Path(tdir);
+ assertTrue(fs.exists(defaultTabletPath));
+
+ Set<Path> generatedPaths = generateTmpFilePaths(ctx, tid,
defaultTabletPath, 100);
+ for (Path p : generatedPaths) {
+ assertFalse(fs.exists(p));
+ assertTrue(fs.createNewFile(p));
+ assertTrue(fs.exists(p));
+ }
+
+ Set<Path> foundPaths = FindCompactionTmpFiles.findTempFiles(ctx,
tid.canonical());
+ assertEquals(100, foundPaths.size());
+ assertEquals(foundPaths, generatedPaths);
+
+ System.setProperty("accumulo.properties",
+ "file://" + getCluster().getAccumuloPropertiesPath());
+ assertEquals(0, getCluster().exec(Admin.class, "compactionTempFiles",
"-t", tableName)
+ .getProcess().waitFor());
+
+ foundPaths = FindCompactionTmpFiles.findTempFiles(ctx, tid.canonical());
+ assertEquals(100, foundPaths.size());
+ assertEquals(foundPaths, generatedPaths);
+
+ assertEquals(0, getCluster().exec(Admin.class, "compactionTempFiles",
"-t", tableName, "-d")
+ .getProcess().waitFor());
Review Comment:
These command executions should have their output validated. The first
should output something like "Found the following compaction tmp files..."
followed by each of the 100 temp files. The second should output something like
"Removed old temp file ..." for each of the files deleted. See something like
FateOpsCommandsITBase for an example of verifying output of a command.
Also wondering if 100 files here is overkill. If the test runs significantly
faster with less files, I think that should be changed as well. The number of
files doesn't really matter so less would be good if it speeds the test up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
