Skip to content

HDFS-15937. Reduce memory used during datanode layout upgrade #2838

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 8, 2021
Merged

HDFS-15937. Reduce memory used during datanode layout upgrade #2838

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57b6db1
HDFS-15937. Reduce memory used during datanode layout upgrade
Mar 30, 2021
6c30ed2
Remove unused method parameter
Mar 30, 2021
559216c
Use a.blockFile() in duplicate compare
Mar 31, 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
98 changes: 62 additions & 36 deletions ...project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,12 +1071,26 @@ private static void linkAllBlocks(File fromDir, File fromBbwDir, File toDir,
}

private static class LinkArgs {
File src;
File dst;
private File srcDir;
private File dstDir;
private String blockFile;

LinkArgs(File srcDir, File dstDir, String blockFile) {
this.srcDir = srcDir;
this.dstDir = dstDir;
this.blockFile = blockFile;
}

public File src() {
return new File(srcDir, blockFile);
}

LinkArgs(File src, File dst) {
this.src = src;
this.dst = dst;
public File dst() {
return new File(dstDir, blockFile);
}

public String blockFile() {
return blockFile;
}
}

Expand All @@ -1102,8 +1116,9 @@ private static void linkBlocks(File from, File to, int oldLV,
}

final ArrayList<LinkArgs> idBasedLayoutSingleLinks = Lists.newArrayList();
linkBlocksHelper(from, to, oldLV, hl, upgradeToIdBasedLayout, to,
idBasedLayoutSingleLinks);
final Map<File, File> pathCache = new HashMap<>();
linkBlocksHelper(from, to, hl, upgradeToIdBasedLayout, to,
idBasedLayoutSingleLinks, pathCache);

// Detect and remove duplicate entries.
final ArrayList<LinkArgs> duplicates =
Expand All @@ -1129,7 +1144,7 @@ public Void call() throws IOException {
idBasedLayoutSingleLinks.size());
for (int j = iCopy; j < upperBound; j++) {
LinkArgs cur = idBasedLayoutSingleLinks.get(j);
HardLink.createHardLink(cur.src, cur.dst);
HardLink.createHardLink(cur.src(), cur.dst());
}
return null;
}
Expand Down Expand Up @@ -1162,9 +1177,9 @@ static ArrayList<LinkArgs> findDuplicateEntries(ArrayList<LinkArgs> all) {
@Override
public int compare(LinkArgs a, LinkArgs b) {
return ComparisonChain.start().
compare(a.src.getName(), b.src.getName()).
compare(a.src, b.src).
compare(a.dst, b.dst).
compare(a.blockFile(), b.blockFile()).
compare(a.src(), b.src()).
compare(a.dst(), b.dst()).
result();
}
});
Expand All @@ -1174,8 +1189,8 @@ public int compare(LinkArgs a, LinkArgs b) {
boolean addedPrev = false;
for (int i = 0; i < all.size(); i++) {
LinkArgs args = all.get(i);
long blockId = Block.getBlockId(args.src.getName());
boolean isMeta = Block.isMetaFilename(args.src.getName());
long blockId = Block.getBlockId(args.blockFile());
boolean isMeta = Block.isMetaFilename(args.blockFile());
if ((prevBlockId == null) ||
(prevBlockId.longValue() != blockId)) {
prevBlockId = blockId;
Expand Down Expand Up @@ -1214,10 +1229,10 @@ private static void removeDuplicateEntries(ArrayList<LinkArgs> all,
TreeMap<Long, List<LinkArgs>> highestGenstamps =
new TreeMap<Long, List<LinkArgs>>();
for (LinkArgs duplicate : duplicates) {
if (!Block.isMetaFilename(duplicate.src.getName())) {
if (!Block.isMetaFilename(duplicate.blockFile())) {
continue;
}
long blockId = Block.getBlockId(duplicate.src.getName());
long blockId = Block.getBlockId(duplicate.blockFile());
List<LinkArgs> prevHighest = highestGenstamps.get(blockId);
if (prevHighest == null) {
List<LinkArgs> highest = new LinkedList<LinkArgs>();
Expand All @@ -1226,8 +1241,8 @@ private static void removeDuplicateEntries(ArrayList<LinkArgs> all,
continue;
}
long prevGenstamp =
Block.getGenerationStamp(prevHighest.get(0).src.getName());
long genstamp = Block.getGenerationStamp(duplicate.src.getName());
Block.getGenerationStamp(prevHighest.get(0).blockFile());
long genstamp = Block.getGenerationStamp(duplicate.blockFile());
if (genstamp < prevGenstamp) {
continue;
}
Expand All @@ -1241,19 +1256,19 @@ private static void removeDuplicateEntries(ArrayList<LinkArgs> all,
// from the duplicates list.
for (Iterator<LinkArgs> iter = duplicates.iterator(); iter.hasNext(); ) {
LinkArgs duplicate = iter.next();
long blockId = Block.getBlockId(duplicate.src.getName());
long blockId = Block.getBlockId(duplicate.blockFile());
List<LinkArgs> highest = highestGenstamps.get(blockId);
if (highest != null) {
boolean found = false;
for (LinkArgs high : highest) {
if (high.src.getParent().equals(duplicate.src.getParent())) {
if (high.src().getParent().equals(duplicate.src().getParent())) {
found = true;
break;
}
}
if (!found) {
LOG.warn("Unexpectedly low genstamp on {}.",
duplicate.src.getAbsolutePath());
duplicate.src().getAbsolutePath());
iter.remove();
}
}
Expand All @@ -1264,25 +1279,25 @@ private static void removeDuplicateEntries(ArrayList<LinkArgs> all,
// preserving one block file / metadata file pair.
TreeMap<Long, LinkArgs> longestBlockFiles = new TreeMap<Long, LinkArgs>();
for (LinkArgs duplicate : duplicates) {
if (Block.isMetaFilename(duplicate.src.getName())) {
if (Block.isMetaFilename(duplicate.blockFile())) {
continue;
}
long blockId = Block.getBlockId(duplicate.src.getName());
long blockId = Block.getBlockId(duplicate.blockFile());
LinkArgs prevLongest = longestBlockFiles.get(blockId);
if (prevLongest == null) {
longestBlockFiles.put(blockId, duplicate);
continue;
}
long blockLength = duplicate.src.length();
long prevBlockLength = prevLongest.src.length();
long blockLength = duplicate.src().length();
long prevBlockLength = prevLongest.src().length();
if (blockLength < prevBlockLength) {
LOG.warn("Unexpectedly short length on {}.",
duplicate.src.getAbsolutePath());
duplicate.src().getAbsolutePath());
continue;
}
if (blockLength > prevBlockLength) {
LOG.warn("Unexpectedly short length on {}.",
prevLongest.src.getAbsolutePath());
prevLongest.src().getAbsolutePath());
}
longestBlockFiles.put(blockId, duplicate);
}
Expand All @@ -1291,21 +1306,22 @@ private static void removeDuplicateEntries(ArrayList<LinkArgs> all,
// arbitrarily selected by us.
for (Iterator<LinkArgs> iter = all.iterator(); iter.hasNext(); ) {
LinkArgs args = iter.next();
long blockId = Block.getBlockId(args.src.getName());
long blockId = Block.getBlockId(args.blockFile());
LinkArgs bestDuplicate = longestBlockFiles.get(blockId);
if (bestDuplicate == null) {
continue; // file has no duplicates
}
if (!bestDuplicate.src.getParent().equals(args.src.getParent())) {
LOG.warn("Discarding {}.", args.src.getAbsolutePath());
if (!bestDuplicate.src().getParent().equals(args.src().getParent())) {
LOG.warn("Discarding {}.", args.src().getAbsolutePath());
iter.remove();
}
}
}

static void linkBlocksHelper(File from, File to, int oldLV, HardLink hl,
boolean upgradeToIdBasedLayout, File blockRoot,
List<LinkArgs> idBasedLayoutSingleLinks) throws IOException {
static void linkBlocksHelper(File from, File to, HardLink hl,
boolean upgradeToIdBasedLayout, File blockRoot,
List<LinkArgs> idBasedLayoutSingleLinks, Map<File, File> pathCache)
throws IOException {
if (!from.exists()) {
return;
}
Expand Down Expand Up @@ -1345,8 +1361,18 @@ public boolean accept(File dir, String name) {
throw new IOException("Failed to mkdirs " + blockLocation);
}
}
idBasedLayoutSingleLinks.add(new LinkArgs(new File(from, blockName),
new File(blockLocation, blockName)));
/**
* The destination path is 32x32, so 1024 distinct paths. Therefore
* we cache the destination path and reuse the same File object on
* potentially thousands of blocks located on this volume.
* This method is called recursively so the cache is passed through
* each recursive call. There is one cache per volume, and it is only
* accessed by a single thread so no locking is needed.
*/
File cachedDest = pathCache
.computeIfAbsent(blockLocation, k -> blockLocation);
idBasedLayoutSingleLinks.add(new LinkArgs(from,
cachedDest, blockName));
hl.linkStats.countSingleLinks++;
}
} else {
Expand All @@ -1369,8 +1395,8 @@ public boolean accept(File dir, String name) {
if (otherNames != null) {
for (int i = 0; i < otherNames.length; i++) {
linkBlocksHelper(new File(from, otherNames[i]),
new File(to, otherNames[i]), oldLV, hl, upgradeToIdBasedLayout,
blockRoot, idBasedLayoutSingleLinks);
new File(to, otherNames[i]), hl, upgradeToIdBasedLayout,
blockRoot, idBasedLayoutSingleLinks, pathCache);
}
}
}
Expand Down