Skip to content

HDFS-15621. Datanode DirectoryScanner uses excessive memory #2849

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

Merged
sodonnel merged 3 commits into from
Apr 26, 2021
Merged

HDFS-15621. Datanode DirectoryScanner uses excessive memory #2849

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
d189e66
HDFS-15621. Datanode DirectoryScanner uses excessive memory
Apr 1, 2021
e71a460
Address findbugs and checkstyle
Apr 8, 2021
74f4eb9
Fix java doc
Apr 23, 2021
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
2 changes: 1 addition & 1 deletion ...ct/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ private void addDifference(Collection<ScanInfo> diffRecord, Stats statsRecord,
long blockId, FsVolumeSpi vol) {
statsRecord.missingBlockFile++;
statsRecord.missingMetaFile++;
diffRecord.add(new ScanInfo(blockId, null, null, vol));
diffRecord.add(new ScanInfo(blockId, null, null, null, vol));
}

/**
Expand Down
109 changes: 47 additions & 62 deletions ...doop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsVolumeSpi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,27 +227,27 @@ interface BlockIterator extends Closeable {
*/
public static class ScanInfo implements Comparable<ScanInfo> {
private final long blockId;

/**
* The block file path, relative to the volume's base directory.
* If there was no block file found, this may be null. If 'vol'
* is null, then this is the full path of the block file.
* The full path to the folder containing the block / meta files.
*/
private final String blockSuffix;

private final File basePath;
/**
* The suffix of the meta file path relative to the block file.
* If blockSuffix is null, then this will be the entire path relative
* to the volume base directory, or an absolute path if vol is also
* null.
* The block file name, with no path
*/
private final String metaSuffix;
private final String blockFile;
/**
* Holds the meta file name, with no path, only if blockFile is null.
* If blockFile is not null, the meta file will be named identically to
* the blockFile, but with a suffix like "_1234.meta". If the blockFile
* is present, we store only the meta file suffix.
*/
private final String metaFile;

private final FsVolumeSpi volume;

private final FileRegion fileRegion;
/**
* Get the file's length in async block scan
* Get the file's length in async block scan.
*/
private final long blockLength;

Expand All @@ -257,35 +257,19 @@ public static class ScanInfo implements Comparable<ScanInfo> {
private final static String QUOTED_FILE_SEPARATOR =
Matcher.quoteReplacement(File.separator);

/**
* Get the most condensed version of the path.
*
* For example, the condensed version of /foo//bar is /foo/bar
* Unlike {@link File#getCanonicalPath()}, this will never perform I/O
* on the filesystem.
*
* @param path the path to condense
* @return the condensed path
*/
private static String getCondensedPath(String path) {
return CONDENSED_PATH_REGEX.matcher(path).
replaceAll(QUOTED_FILE_SEPARATOR);
}

/**
* Get a path suffix.
*
* @param f The file to get the suffix for.
* @param f The string to get the suffix for.
* @param prefix The prefix we're stripping off.
*
* @return A suffix such that prefix + suffix = path to f
* @return A suffix such that prefix + suffix = f
*/
private static String getSuffix(File f, String prefix) {
String fullPath = getCondensedPath(f.getAbsolutePath());
if (fullPath.startsWith(prefix)) {
return fullPath.substring(prefix.length());
private static String getSuffix(String f, String prefix) {
if (f.startsWith(prefix)) {
return f.substring(prefix.length());
}
throw new RuntimeException(prefix + " is not a prefix of " + fullPath);
throw new RuntimeException(prefix + " is not a prefix of " + f);
}

/**
Expand All @@ -297,23 +281,18 @@ private static String getSuffix(File f, String prefix) {
* @param metaFile the path to the block meta-data file
Copy link
Contributor

@jojochuang jojochuang Apr 13, 2021

Choose a reason for hiding this comment

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

need a @param for basePath. Also add that metaFile stores only the suffix.

* @param vol the volume that contains the block
*/
public ScanInfo(long blockId, File blockFile, File metaFile,
FsVolumeSpi vol) {
public ScanInfo(long blockId, File basePath, String blockFile,
String metaFile, FsVolumeSpi vol) {
this.blockId = blockId;
String condensedVolPath =
(vol == null || vol.getBaseURI() == null) ? null :
getCondensedPath(new File(vol.getBaseURI()).getAbsolutePath());
this.blockSuffix = blockFile == null ? null :
getSuffix(blockFile, condensedVolPath);
this.blockLength = (blockFile != null) ? blockFile.length() : 0;
if (metaFile == null) {
this.metaSuffix = null;
} else if (blockFile == null) {
this.metaSuffix = getSuffix(metaFile, condensedVolPath);
this.basePath = basePath;
this.blockFile = blockFile;
if (blockFile != null && metaFile != null) {
this.metaFile = getSuffix(metaFile, blockFile);
} else {
this.metaSuffix = getSuffix(metaFile,
condensedVolPath + blockSuffix);
this.metaFile = metaFile;
}
this.blockLength = (blockFile != null) ?
new File(basePath, blockFile).length() : 0;
this.volume = vol;
this.fileRegion = null;
}
Expand All @@ -333,8 +312,9 @@ public ScanInfo(long blockId, FsVolumeSpi vol, FileRegion fileRegion,
this.blockLength = length;
this.volume = vol;
this.fileRegion = fileRegion;
this.blockSuffix = null;
this.metaSuffix = null;
this.basePath = null;
this.blockFile = null;
this.metaFile = null;
}

/**
Expand All @@ -343,8 +323,8 @@ public ScanInfo(long blockId, FsVolumeSpi vol, FileRegion fileRegion,
* @return the block data file
*/
public File getBlockFile() {
return (blockSuffix == null) ? null :
new File(new File(volume.getBaseURI()).getAbsolutePath(), blockSuffix);
return (blockFile == null) ? null :
new File(basePath.getAbsolutePath(), blockFile);
}

/**
Expand All @@ -363,15 +343,10 @@ public long getBlockLength() {
* @return the block meta data file
*/
public File getMetaFile() {
if (metaSuffix == null) {
if (metaFile == null) {
return null;
}
String fileSuffix = metaSuffix;
if (blockSuffix != null) {
fileSuffix = blockSuffix + metaSuffix;
}
return new File(new File(volume.getBaseURI()).getAbsolutePath(),
fileSuffix);
return new File(basePath.getAbsolutePath(), fullMetaFile());
}

/**
Expand Down Expand Up @@ -414,14 +389,24 @@ public int hashCode() {
}

public long getGenStamp() {
return metaSuffix != null ? Block.getGenerationStamp(
getMetaFile().getName()) :
HdfsConstants.GRANDFATHER_GENERATION_STAMP;
return metaFile != null ? Block.getGenerationStamp(fullMetaFile())
: HdfsConstants.GRANDFATHER_GENERATION_STAMP;
}

public FileRegion getFileRegion() {
return fileRegion;
}

private String fullMetaFile() {
if (metaFile == null) {
return null;
}
if (blockFile == null) {
return metaFile;
} else {
return blockFile + metaFile;
}
}
}

/**
Expand Down
5 changes: 3 additions & 2 deletions ...dfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1451,7 +1451,7 @@ private void compileReport(File bpFinalizedDir, File dir,
long blockId = Block.getBlockId(file.getName());
verifyFileLocation(file, bpFinalizedDir,
blockId);
report.add(new ScanInfo(blockId, null, file, this));
report.add(new ScanInfo(blockId, dir, null, fileNames.get(i), this));
}
continue;
}
Expand All @@ -1474,7 +1474,8 @@ private void compileReport(File bpFinalizedDir, File dir,
}
}
verifyFileLocation(blockFile, bpFinalizedDir, blockId);
report.add(new ScanInfo(blockId, blockFile, metaFile, this));
report.add(new ScanInfo(blockId, dir, blockFile.getName(),
metaFile == null ? null : metaFile.getName(), this));
}
}

Expand Down
37 changes: 17 additions & 20 deletions ...adoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDirectoryScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,19 +1040,21 @@ public VolumeCheckResult check(VolumeCheckContext context)

private final static String BPID_2 = "BP-367845636-127.0.0.1-5895645674231";

void testScanInfoObject(long blockId, File blockFile, File metaFile)
void testScanInfoObject(long blockId, File baseDir, String blockFile,
String metaFile)
throws Exception {
FsVolumeSpi.ScanInfo scanInfo =
new FsVolumeSpi.ScanInfo(blockId, blockFile, metaFile, TEST_VOLUME);
new FsVolumeSpi.ScanInfo(blockId, baseDir, blockFile, metaFile,
TEST_VOLUME);
assertEquals(blockId, scanInfo.getBlockId());
if (blockFile != null) {
assertEquals(blockFile.getAbsolutePath(),
assertEquals(new File(baseDir, blockFile).getAbsolutePath(),
scanInfo.getBlockFile().getAbsolutePath());
} else {
assertNull(scanInfo.getBlockFile());
}
if (metaFile != null) {
assertEquals(metaFile.getAbsolutePath(),
assertEquals(new File(baseDir, metaFile).getAbsolutePath(),
scanInfo.getMetaFile().getAbsolutePath());
} else {
assertNull(scanInfo.getMetaFile());
Expand All @@ -1062,7 +1064,7 @@ void testScanInfoObject(long blockId, File blockFile, File metaFile)

void testScanInfoObject(long blockId) throws Exception {
FsVolumeSpi.ScanInfo scanInfo =
new FsVolumeSpi.ScanInfo(blockId, null, null, null);
new FsVolumeSpi.ScanInfo(blockId, null, null, null, null);
assertEquals(blockId, scanInfo.getBlockId());
assertNull(scanInfo.getBlockFile());
assertNull(scanInfo.getMetaFile());
Expand All @@ -1071,24 +1073,19 @@ void testScanInfoObject(long blockId) throws Exception {
@Test(timeout = 120000)
public void TestScanInfo() throws Exception {
testScanInfoObject(123,
new File(TEST_VOLUME.getFinalizedDir(BPID_1).getAbsolutePath(),
"blk_123"),
new File(TEST_VOLUME.getFinalizedDir(BPID_1).getAbsolutePath(),
"blk_123__1001.meta"));
new File(TEST_VOLUME.getFinalizedDir(BPID_1).getAbsolutePath()),
"blk_123", "blk_123__1001.meta");
testScanInfoObject(464,
new File(TEST_VOLUME.getFinalizedDir(BPID_1).getAbsolutePath(),
"blk_123"),
null);
testScanInfoObject(523, null,
new File(TEST_VOLUME.getFinalizedDir(BPID_1).getAbsolutePath(),
"blk_123__1009.meta"));
testScanInfoObject(789, null, null);
new File(TEST_VOLUME.getFinalizedDir(BPID_1).getAbsolutePath()),
"blk_123", null);
testScanInfoObject(523,
new File(TEST_VOLUME.getFinalizedDir(BPID_1).getAbsolutePath()),
null, "blk_123__1009.meta");
testScanInfoObject(789, null, null, null);
testScanInfoObject(456);
testScanInfoObject(123,
new File(TEST_VOLUME.getFinalizedDir(BPID_2).getAbsolutePath(),
"blk_567"),
new File(TEST_VOLUME.getFinalizedDir(BPID_2).getAbsolutePath(),
"blk_567__1004.meta"));
new File(TEST_VOLUME.getFinalizedDir(BPID_2).getAbsolutePath()),
"blk_567", "blk_567__1004.meta");
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ...rc/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1785,8 +1785,8 @@ public void testNotifyNamenodeMissingOrNewBlock() throws Exception {
assertFalse(metaFile.exists());

FsVolumeSpi.ScanInfo info = new FsVolumeSpi.ScanInfo(
replicaInfo.getBlockId(), blockFile.getAbsoluteFile(),
metaFile.getAbsoluteFile(), replicaInfo.getVolume());
replicaInfo.getBlockId(), blockFile.getParentFile().getAbsoluteFile(),
blockFile.getName(), metaFile.getName(), replicaInfo.getVolume());
fsdataset.checkAndUpdate(bpid, info);

BlockManager blockManager = cluster.getNameNode().
Expand Down