Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDFS-17272. NNThroughputBenchmark should support specifying the base directory for multi-client test #6319

Merged
merged 3 commits into from
Dec 10, 2023
Merged

HDFS-17272. NNThroughputBenchmark should support specifying the base directory for multi-client test #6319

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a41f6a9
NNThroughputBenchmark support specifying the base directory
Dec 2, 2023
fa1ce21
update UT and code style
Dec 4, 2023
14566b7
fix the checkstyle
Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 35 additions & 15 deletions ...doop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ static void setNameNodeLoggingLevel(Level logLevel) {
* specific name-node operation.
*/
abstract class OperationStatsBase {
protected static final String BASE_DIR_NAME = "/nnThroughputBenchmark";
protected String baseDirName = "/nnThroughputBenchmark";
protected static final String OP_ALL_NAME = "all";
protected static final String OP_ALL_USAGE = "-op all <other ops options>";

protected final String baseDir;
protected String baseDir;
protected short replication;
protected int blockSize;
protected int numThreads = 0; // number of threads
Expand Down Expand Up @@ -228,7 +228,7 @@ abstract class OperationStatsBase {
abstract void printResults();

OperationStatsBase() {
baseDir = BASE_DIR_NAME + "/" + getOpName();
baseDir = baseDirName + "/" + getOpName();
replication = (short) config.getInt(DFSConfigKeys.DFS_REPLICATION_KEY, 3);
blockSize = config.getInt(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, BLOCK_SIZE);
numOpsRequired = 10;
Expand Down Expand Up @@ -317,6 +317,7 @@ long getAverageTime() {
}

String getBaseDir() {
baseDir = baseDirName + "/" + getOpName();
return baseDir;
}

Expand Down Expand Up @@ -494,15 +495,15 @@ long executeOp(int daemonId, int inputIdx, String ignore)
clientProto.setSafeMode(HdfsConstants.SafeModeAction.SAFEMODE_LEAVE,
false);
long start = Time.now();
clientProto.delete(BASE_DIR_NAME, true);
clientProto.delete(baseDirName, true);
long end = Time.now();
return end-start;
}

@Override
void printResults() {
LOG.info("--- " + getOpName() + " inputs ---");
LOG.info("Remove directory " + BASE_DIR_NAME);
LOG.info("Remove directory " + baseDirName);
printStats();
}
}
Expand All @@ -517,9 +518,9 @@ void printResults() {
class CreateFileStats extends OperationStatsBase {
// Operation types
static final String OP_CREATE_NAME = "create";
static final String OP_CREATE_USAGE =
static final String OP_CREATE_USAGE =
"-op create [-threads T] [-files N] [-blockSize S] [-filesPerDir P]"
+ " [-close]";
+ " [-baseDirName D] [-close]";

protected FileNameGenerator nameGenerator;
protected String[][] fileNames;
Expand Down Expand Up @@ -553,6 +554,9 @@ void parseArguments(List<String> args) {
} else if(args.get(i).equals("-filesPerDir")) {
if(i+1 == args.size()) printUsage();
nrFilesPerDir = Integer.parseInt(args.get(++i));
} else if(args.get(i).equals("-baseDirName")) {
if(i+1 == args.size()) printUsage();
baseDirName = args.get(++i);
} else if(args.get(i).equals("-close")) {
closeUponCreate = true;
} else if(!ignoreUnrelatedOptions)
Expand All @@ -568,6 +572,7 @@ void generateInputs(int[] opsPerThread) throws IOException {
false);
// int generatedFileIdx = 0;
LOG.info("Generate " + numOpsRequired + " intputs for " + getOpName());
LOG.info("basedir: " + getBaseDir());
fileNames = new String[numThreads][];
try {
for(int idx=0; idx < numThreads; idx++) {
Expand Down Expand Up @@ -618,6 +623,7 @@ long executeOp(int daemonId, int inputIdx, String clientName)
@Override
void printResults() {
LOG.info("--- " + getOpName() + " inputs ---");
LOG.info("baseDir = " + getBaseDir());
LOG.info("nrFiles = " + numOpsRequired);
LOG.info("nrThreads = " + numThreads);
LOG.info("nrFilesPerDir = " + nameGenerator.getFilesPerDirectory());
Expand All @@ -635,7 +641,7 @@ class MkdirsStats extends OperationStatsBase {
// Operation types
static final String OP_MKDIRS_NAME = "mkdirs";
static final String OP_MKDIRS_USAGE = "-op mkdirs [-threads T] [-dirs N] " +
"[-dirsPerDir P]";
"[-dirsPerDir P] [-baseDirName D]";

protected FileNameGenerator nameGenerator;
protected String[][] dirPaths;
Expand Down Expand Up @@ -664,6 +670,9 @@ void parseArguments(List<String> args) {
} else if(args.get(i).equals("-dirsPerDir")) {
if(i+1 == args.size()) printUsage();
nrDirsPerDir = Integer.parseInt(args.get(++i));
} else if(args.get(i).equals("-baseDirName")) {
if(i+1 == args.size()) printUsage();
baseDirName = args.get(++i);
} else if(!ignoreUnrelatedOptions)
printUsage();
}
Expand Down Expand Up @@ -718,6 +727,7 @@ long executeOp(int daemonId, int inputIdx, String clientName)
@Override
void printResults() {
LOG.info("--- " + getOpName() + " inputs ---");
LOG.info("baseDir = " + getBaseDir());
LOG.info("nrDirs = " + numOpsRequired);
LOG.info("nrThreads = " + numThreads);
LOG.info("nrDirsPerDir = " + nameGenerator.getFilesPerDirectory());
Expand All @@ -736,7 +746,7 @@ class OpenFileStats extends CreateFileStats {
static final String OP_OPEN_NAME = "open";
static final String OP_USAGE_ARGS =
" [-threads T] [-files N] [-blockSize S] [-filesPerDir P]"
+ " [-useExisting]";
+ " [-useExisting] [-baseDirName D]";
static final String OP_OPEN_USAGE =
"-op " + OP_OPEN_NAME + OP_USAGE_ARGS;

Expand Down Expand Up @@ -771,6 +781,7 @@ void generateInputs(int[] opsPerThread) throws IOException {
"-blockSize", String.valueOf(blockSize),
"-filesPerDir",
String.valueOf(nameGenerator.getFilesPerDirectory()),
"-baseDirName", this.baseDirName,
"-close"};
CreateFileStats opCreate = new CreateFileStats(Arrays.asList(createArgs));

Expand Down Expand Up @@ -1135,9 +1146,9 @@ private int transferBlocks( Block blocks[],
*/
class BlockReportStats extends OperationStatsBase {
static final String OP_BLOCK_REPORT_NAME = "blockReport";
static final String OP_BLOCK_REPORT_USAGE =
static final String OP_BLOCK_REPORT_USAGE =
"-op blockReport [-datanodes T] [-reports N] " +
"[-blocksPerReport B] [-blocksPerFile F] [-blockSize S]";
"[-blocksPerReport B] [-blocksPerFile F] [-blockSize S] [-baseDirName D]";

private int blocksPerReport;
private int blocksPerFile;
Expand Down Expand Up @@ -1187,6 +1198,9 @@ void parseArguments(List<String> args) {
} else if (args.get(i).equals("-blockSize")) {
if(i+1 == args.size()) printUsage();
blockSize = Integer.parseInt(args.get(++i));
} else if(args.get(i).equals("-baseDirName")) {
if(i+1 == args.size()) printUsage();
baseDirName = args.get(++i);
} else if(!ignoreUnrelatedOptions)
printUsage();
}
Expand Down Expand Up @@ -1330,6 +1344,7 @@ void printResults() {
}
blockDistribution += ")";
LOG.info("--- " + getOpName() + " inputs ---");
LOG.info("baseDir = " + getBaseDir());
LOG.info("reports = " + numOpsRequired);
LOG.info("datanodes = " + numThreads + " " + blockDistribution);
LOG.info("blocksPerReport = " + blocksPerReport);
Expand All @@ -1346,9 +1361,9 @@ void printResults() {
class ReplicationStats extends OperationStatsBase {
static final String OP_REPLICATION_NAME = "replication";
static final String OP_REPLICATION_USAGE =
"-op replication [-datanodes T] [-nodesToDecommission D] " +
"-op replication [-datanodes T] [-nodesToDecommission N] " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't change the existing commands, that is an incomatible change, whatever needs to be done should be done for new commands only.

btw. what does this letter denote?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think D denote dir.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we both -nodesToDecommission and -baseDirName use D?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it has no physical meaning, then yes, can check if other alphabets are not used, so can explore using them P as in Path or B as in BaseDir or some similar, else use whichever you like :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P and B has also been used, such as -blocksPerReport B and -filesPerDir P. And other alphabets cannot be related to directory I think. Could I keep using D?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, just double check they don't have any specific usage apart from just being alphabets in the description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. Only -nodesToDecommission used the D alphabet.

"[-nodeReplicationLimit C] [-totalBlocks B] [-blockSize S] "
+ "[-replication R]";
+ "[-replication R] [-baseDirName D]";

private final BlockReportStats blockReportObject;
private int numDatanodes;
Expand Down Expand Up @@ -1377,7 +1392,8 @@ class ReplicationStats extends OperationStatsBase {
"-datanodes", String.valueOf(numDatanodes),
"-blocksPerReport", String.valueOf(totalBlocks*replication/numDatanodes),
"-blocksPerFile", String.valueOf(numDatanodes),
"-blockSize", String.valueOf(blockSize)};
"-blockSize", String.valueOf(blockSize),
"-baseDirName", baseDirName};
blockReportObject = new BlockReportStats(Arrays.asList(blkReportArgs));
numDecommissionedBlocks = 0;
numPendingBlocks = 0;
Expand Down Expand Up @@ -1410,6 +1426,9 @@ void parseArguments(List<String> args) {
} else if (args.get(i).equals("-blockSize")) {
if(i+1 == args.size()) printUsage();
blockSize = Integer.parseInt(args.get(++i));
} else if(args.get(i).equals("-baseDirName")) {
if(i+1 == args.size()) printUsage();
baseDirName = args.get(++i);
} else if(!ignoreUnrelatedOptions)
printUsage();
}
Expand Down Expand Up @@ -1485,6 +1504,7 @@ void printResults() {
}
blockDistribution += ")";
LOG.info("--- " + getOpName() + " inputs ---");
LOG.info("baseDir = " + getBaseDir());
LOG.info("numOpsRequired = " + numOpsRequired);
LOG.info("datanodes = " + numDatanodes + " " + blockDistribution);
LOG.info("decommissioned datanodes = " + nodesToDecommission);
Expand Down Expand Up @@ -1631,7 +1651,7 @@ public int run(String[] aArgs) throws Exception {
}
// run each benchmark
for(OperationStatsBase op : ops) {
LOG.info("Starting benchmark: " + op.getOpName());
LOG.info("Starting benchmark: " + op.getOpName() + ", baseDir: " + op.getBaseDir());
op.benchmark();
op.cleanUp();
}
Expand Down
56 changes: 56 additions & 0 deletions ...-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNNThroughputBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,60 @@ public void testNNThroughputForBlockReportOp() throws Exception {
"blockReport", "-datanodes", "3", "-reports", "2"});
}
}

/**
* This test runs {@link NNThroughputBenchmark} against a mini DFS cluster
* with explicit -baseDirName option.
*/
@Test(timeout = 120000)
public void testNNThroughputWithBaseDir() throws Exception {
final Configuration conf = new HdfsConfiguration();
conf.setInt(DFSConfigKeys.DFS_NAMENODE_MIN_BLOCK_SIZE_KEY, 16);
MiniDFSCluster cluster = null;
try {
cluster = new MiniDFSCluster.Builder(conf).numDataNodes(0).build();
cluster.waitActive();
final Configuration benchConf = new HdfsConfiguration();
benchConf.setInt(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, 16);
FileSystem.setDefaultUri(benchConf, cluster.getURI());

NNThroughputBenchmark.runBenchmark(benchConf,
new String[] {"-op", "create", "-keepResults", "-files", "3", "-baseDirName",
"/nnThroughputBenchmark1", "-close"});
FSNamesystem fsNamesystem = cluster.getNamesystem();
DirectoryListing listing = fsNamesystem.getListing("/", HdfsFileStatus.EMPTY_NAME, false);
Boolean b_dir_exist1 = false;
Boolean b_dir_exist2 = false;
for (HdfsFileStatus f : listing.getPartialListing()) {
if (f.getFullName("/").equals("/nnThroughputBenchmark1")) {
b_dir_exist1 = true;
}
if (f.getFullName("/").equals("/nnThroughputBenchmark")) {
b_dir_exist2 = true;
}
}
Assert.assertEquals(b_dir_exist1, true);
Assert.assertEquals(b_dir_exist2, false);

NNThroughputBenchmark.runBenchmark(benchConf,
new String[] {"-op", "all", "-baseDirName", "/nnThroughputBenchmark1"});
listing = fsNamesystem.getListing("/", HdfsFileStatus.EMPTY_NAME, false);
lfxy marked this conversation as resolved.
Show resolved Hide resolved
b_dir_exist1 = false;
b_dir_exist2 = false;
for (HdfsFileStatus f : listing.getPartialListing()) {
if (f.getFullName("/").equals("/nnThroughputBenchmark1")) {
b_dir_exist1 = true;
}
if (f.getFullName("/").equals("/nnThroughputBenchmark")) {
b_dir_exist2 = true;
}
}
Assert.assertEquals(b_dir_exist1, true);
Assert.assertEquals(b_dir_exist2, false);
} finally {
if (cluster != null) {
cluster.shutdown();
}
}
}
}