From ce755b1cdd08a02d320a02b31bd4699c0fc80c20 Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Wed, 15 Jul 2020 00:12:31 -0700 Subject: [PATCH 1/8] HDFS-15463. Add a tool to validate FsImage. --- .../hadoop-hdfs/src/main/bin/hdfs | 4 + .../hadoop-hdfs/src/main/bin/hdfs.cmd | 8 +- .../server/namenode/FsImageValidation.java | 153 ++++++++++++++ .../hadoop/hdfs/server/namenode/INode.java | 10 +- .../hdfs/server/namenode/INodeReference.java | 143 ++++++++++++- .../namenode/INodeReferenceValidation.java | 196 ++++++++++++++++++ .../namenode/TestFsImageValidation.java | 41 ++++ 7 files changed, 548 insertions(+), 7 deletions(-) create mode 100644 hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java create mode 100644 hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java create mode 100644 hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs b/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs index 94426a561fb6d..7a8bf8dbe0deb 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs @@ -47,6 +47,7 @@ function hadoop_usage hadoop_add_subcommand "ec" admin "run a HDFS ErasureCoding CLI" hadoop_add_subcommand "fetchdt" client "fetch a delegation token from the NameNode" hadoop_add_subcommand "fsck" admin "run a DFS filesystem checking utility" + hadoop_add_subcommand "fsImageValidation" admin "run FsImageValidation to check an fsimage" hadoop_add_subcommand "getconf" client "get config values from configuration" hadoop_add_subcommand "groups" client "get the groups which users belong to" hadoop_add_subcommand "haadmin" admin "run a DFS HA admin client" @@ -143,6 +144,9 @@ function hdfscmd_case fsck) HADOOP_CLASSNAME=org.apache.hadoop.hdfs.tools.DFSck ;; + fsImageValidation) + HADOOP_CLASSNAME=org.apache.hadoop.hdfs.server.namenode.FsImageValidation + ;; getconf) HADOOP_CLASSNAME=org.apache.hadoop.hdfs.tools.GetConf ;; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs.cmd b/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs.cmd index a9a7852fa6172..23d6a5aa1c301 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs.cmd +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs.cmd @@ -59,7 +59,7 @@ if "%1" == "--loglevel" ( ) ) - set hdfscommands=dfs namenode secondarynamenode journalnode zkfc datanode dfsadmin haadmin fsck balancer jmxget oiv oev fetchdt getconf groups snapshotDiff lsSnapshottableDir cacheadmin mover storagepolicies classpath crypto dfsrouter dfsrouteradmin debug + set hdfscommands=dfs namenode secondarynamenode journalnode zkfc datanode dfsadmin haadmin fsck fsImageValidation balancer jmxget oiv oev fetchdt getconf groups snapshotDiff lsSnapshottableDir cacheadmin mover storagepolicies classpath crypto dfsrouter dfsrouteradmin debug for %%i in ( %hdfscommands% ) do ( if %hdfs-command% == %%i set hdfscommand=true ) @@ -121,6 +121,11 @@ goto :eof set HADOOP_OPTS=%HADOOP_OPTS% %HADOOP_CLIENT_OPTS% goto :eof +:fsImageValidation + set CLASS=org.apache.hadoop.hdfs.server.namenode.FsImageValidation + set HADOOP_OPTS=%HADOOP_OPTS% %HADOOP_CLIENT_OPTS% + goto :eof + :balancer set CLASS=org.apache.hadoop.hdfs.server.balancer.Balancer set HADOOP_OPTS=%HADOOP_OPTS% %HADOOP_BALANCER_OPTS% @@ -236,6 +241,7 @@ goto :eof @echo dfsadmin run a DFS admin client @echo haadmin run a DFS HA admin client @echo fsck run a DFS filesystem checking utility + @echo fsImageValidation run FsImageValidation to check an fsimage @echo balancer run a cluster balancing utility @echo jmxget get JMX exported values from NameNode or DataNode. @echo oiv apply the offline fsimage viewer to an fsimage diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java new file mode 100644 index 0000000000000..857d118da5ac6 --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.server.namenode; + +import org.apache.hadoop.HadoopIllegalArgumentException; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.conf.Configured; +import org.apache.hadoop.hdfs.DFSUtil; +import org.apache.hadoop.hdfs.HdfsConfiguration; +import org.apache.hadoop.hdfs.server.namenode.startupprogress.Phase; +import org.apache.hadoop.hdfs.server.namenode.startupprogress.StartupProgressView; +import org.apache.hadoop.hdfs.server.protocol.NamespaceInfo; +import org.apache.hadoop.util.Tool; +import org.apache.hadoop.util.ToolRunner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.util.Arrays; +import java.util.Timer; +import java.util.TimerTask; + +/** + * For validating {@link FSImage}. + * + * This tool will load the user specified {@link FSImage} + * and then build the namespace tree in order to run the validation. + * + * The main difference of this tool and + * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer} + * is that + * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer} + * only loads {@link FSImage} but it does not build the namespace tree. + */ +public class FsImageValidation { + static final Logger LOG = LoggerFactory.getLogger(FsImageValidation.class); + + static final String FS_IMAGE_FILE = "FS_IMAGE_FILE"; + + static int checkINodeReference(File fsImageFile) throws Exception { + LOG.info("Check INodeReference for {}: {}", FS_IMAGE_FILE, fsImageFile); + + final TimerTask checkProgress = new TimerTask() { + @Override + public void run() { + final StartupProgressView view = NameNode.getStartupProgress().createView(); + final double percent = view.getPercentComplete(Phase.LOADING_FSIMAGE); + LOG.info("{} Progress: {}%", Phase.LOADING_FSIMAGE, + String.format("%.1f", 100 * percent)); + } + }; + final Timer t = new Timer(); + t.scheduleAtFixedRate(checkProgress, 0, 60_000); + + final Configuration conf = new HdfsConfiguration(); + final FSImage fsImage = new FSImage(conf); + final FSNamesystem namesystem = new FSNamesystem(conf, fsImage, false); + + final NamespaceInfo namespaceInfo = NNStorage.newNamespaceInfo(); + namespaceInfo.clusterID = "cluster0"; + fsImage.getStorage().setStorageInfo(namespaceInfo); + + final FSImageFormat.LoaderDelegator loader = FSImageFormat.newLoader(conf, namesystem); + INodeReferenceValidation.start(); + namesystem.writeLock(); + namesystem.getFSDirectory().writeLock(); + try { + loader.load(fsImageFile, false); + } finally { + namesystem.getFSDirectory().writeUnlock(); + namesystem.writeUnlock(); + t.cancel(); + } + return INodeReferenceValidation.end(); + } + + static File getFsImageFile(String... args) { + final String f = parse(args); + if (f == null) { + throw new HadoopIllegalArgumentException( + FS_IMAGE_FILE + " is not specified."); + } + return new File(f); + } + + static String parse(String... args) { + final String f; + if (args == null || args.length == 0) { + f = System.getenv().get(FS_IMAGE_FILE); + if (f != null) { + LOG.info("Environment variable {} = {}", FS_IMAGE_FILE, f); + } + } else if (args.length == 1) { + f = args[0]; + } else { + throw new HadoopIllegalArgumentException( + "args = " + Arrays.toString(args)); + } + + LOG.info("{} = {}", FS_IMAGE_FILE, f); + return f; + } + + static class Cli extends Configured implements Tool { + static final String COMMAND; + static final String USAGE; + static { + final String clazz = FsImageValidation.class.getSimpleName(); + COMMAND = Character.toLowerCase(clazz.charAt(0)) + clazz.substring(1); + USAGE = "Usage: hdfs " + COMMAND + " <" + FS_IMAGE_FILE + ">"; + } + + @Override + public int run(String[] args) throws Exception { + final int errorCount = checkINodeReference(getFsImageFile(args)); + LOG.info("Error Count: {}", errorCount); + return errorCount == 0? 0: 1; + } + } + + public static void main(String[] args) { + if (DFSUtil.parseHelpArgument(args, Cli.USAGE, System.out, true)) { + System.exit(0); + } + + try { + System.exit(ToolRunner.run(new HdfsConfiguration(), new Cli(), args)); + } catch (HadoopIllegalArgumentException e) { + e.printStackTrace(System.err); + System.err.println(Cli.USAGE); + System.exit(-1); + ToolRunner.printGenericCommandUsage(System.err); + } catch (Throwable e) { + LOG.error("Failed to run " + Cli.COMMAND, e); + System.exit(-2); + } + } +} diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java index 6334ed2f15502..64099dd88a06a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java @@ -660,9 +660,15 @@ public final String getParentString() { } } + @VisibleForTesting + public String getFullPathAndObjectString() { + return getFullPathName() + "(" + getId() + ", " + getObjectString() + ")"; + } + @VisibleForTesting public String toDetailString() { - return toString() + "(" + getObjectString() + "), " + getParentString(); + return toString() + "(" + getId() + ", " + getObjectString() + + ", " + getParentString() + ")"; } /** @return the parent directory */ @@ -697,7 +703,7 @@ public final void setParent(INodeDirectory parent) { } /** Set container. */ - public final void setParentReference(INodeReference parent) { + public void setParentReference(INodeReference parent) { this.parent = parent; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java index 8de0ed6d5de22..0ca5a11d5f480 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.Iterator; import java.util.List; import org.apache.hadoop.fs.permission.FsPermission; @@ -61,6 +62,15 @@ * inode(id=1000,name=bar).getParent() returns /xyz but not /abc. */ public abstract class INodeReference extends INode { + /** Assert the relationship this node and the references. */ + abstract void assertReferences(); + + @Override + public String toDetailString() { + final String s = referred == null? null: referred.getFullPathAndObjectString(); + return super.toDetailString() + ", ->" + s; + } + /** * Try to remove the given reference and then return the reference count. * If the given inode is not a reference, return -1; @@ -346,7 +356,7 @@ public void dumpTreeRecursively(PrintWriter out, StringBuilder prefix, out.print(", dstSnapshotId=" + ((DstReference) this).dstSnapshotId); } if (this instanceof WithCount) { - out.print(", count=" + ((WithCount)this).getReferenceCount()); + out.print(", " + ((WithCount)this).getCountDetails()); } out.println(); @@ -382,7 +392,77 @@ public int compare(WithName left, WithName right) { public WithCount(INodeReference parent, INode referred) { super(parent, referred); Preconditions.checkArgument(!referred.isReference()); + Preconditions.checkArgument(parent == null); referred.setParentReference(this); + + INodeReferenceValidation.addWithCount(this); + } + + private String getCountDetails() { + final StringBuilder b = new StringBuilder("["); + if (!withNameList.isEmpty()) { + final Iterator i = withNameList.iterator(); + b.append(i.next().getFullPathAndObjectString()); + for(; i.hasNext(); ) { + b.append(", ").append(i.next().getFullPathAndObjectString()); + } + } + b.append("]"); + return ", count=" + getReferenceCount() + ", names=" + b; + } + + @Override + public String toDetailString() { + return super.toDetailString() + getCountDetails(); + } + + @Override + public void setParentReference(INodeReference parentRef) { + if (parentRef != null) { + assertDstReference(parentRef); + + if (getParentReference() != null) { + throw new IllegalArgumentException("Overwriting Parent Reference:" + + "\n parentRef: " + parentRef.toDetailString() + + "\n withCount: " + this.toDetailString()); + } + } else { + final INodeReference old = getParentReference(); + LOG.debug("remove parent reference: parent={}, this={}", + old == null? null: old.toDetailString(), this.toDetailString()); + } + super.setParentReference(parentRef); + } + + private void assertDstReference(INodeReference parentRef) { + if (parentRef instanceof DstReference) { + return; + } + throw new IllegalArgumentException("Unexpected non-DstReference:" + + "\n parentRef: " + parentRef.toDetailString() + + "\n withCount: " + this.toDetailString()); + } + + private void assertReferredINode(INodeReference ref, String name) { + if (ref.getReferredINode() == this) { + return; + } + throw new IllegalStateException("Inconsistent Reference:" + + "\n " + name + ": " + ref.toDetailString() + + "\n withCount: " + this.toDetailString()); + } + + @Override + void assertReferences() { + for(WithName withName : withNameList) { + assertReferredINode(withName, " withName"); + } + + final INodeReference parentRef = getParentReference(); + if (parentRef != null) { + assertDstReference(parentRef); + assertReferredINode(parentRef, "parentRef"); + } } public int getReferenceCount() { @@ -403,19 +483,26 @@ public void addReference(INodeReference ref) { withNameList.add(-i - 1, refWithName); } else if (ref instanceof DstReference) { setParentReference(ref); + } else { + throw new IllegalStateException("Failed to add reference:" + + "\n ref: " + ref.toDetailString() + + "\n withCount: " + this.toDetailString()); } } + private int search(WithName ref) { + return Collections.binarySearch(withNameList, ref, WITHNAME_COMPARATOR); + } + /** Decrement and then return the reference count. */ public void removeReference(INodeReference ref) { if (ref instanceof WithName) { - int i = Collections.binarySearch(withNameList, (WithName) ref, - WITHNAME_COMPARATOR); + final int i = search((WithName) ref); if (i >= 0) { withNameList.remove(i); } } else if (ref == getParentReference()) { - setParent(null); + setParentReference(null); } } @@ -481,6 +568,33 @@ public WithName(INodeDirectory parent, WithCount referred, byte[] name, this.name = name; this.lastSnapshotId = lastSnapshotId; referred.addReference(this); + + INodeReferenceValidation.addWithName(this); + } + + @Override + void assertReferences() { + final INode ref= getReferredINode(); + final String err; + if (ref instanceof WithCount) { + final WithCount withCount = (WithCount)ref; + final int i = withCount.search(this); + if (i >= 0) { + if (withCount.withNameList.get(i) == this) { + return; + } else { + err = "OBJECT MISMATCH, withNameList.get(" + i + ") != this"; + } + } else { + err = "NOT FOUND in withNameList"; + } + } else { + err = "UNEXPECTED CLASS, expecting WithCount"; + } + + throw new IllegalStateException(err + ":" + + "\n ref: " + (ref == null? null : ref.toDetailString()) + + "\n this: " + this.toDetailString()); } @Override @@ -642,6 +756,27 @@ public DstReference(INodeDirectory parent, WithCount referred, super(parent, referred); this.dstSnapshotId = dstSnapshotId; referred.addReference(this); + + INodeReferenceValidation.addDstReference(this); + } + + @Override + void assertReferences() { + final INode ref = getReferredINode(); + final String err; + if (ref instanceof WithCount) { + if (ref.getParentReference() == this) { + return; + } else { + err = "OBJECT MISMATCH, ref.getParentReference() != this"; + } + } else { + err = "UNEXPECTED CLASS, expecting WithCount"; + } + + throw new IllegalStateException(err + ":" + + "\n ref: " + (ref == null? null : ref.toDetailString()) + + "\n this: " + this.toDetailString()); } @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java new file mode 100644 index 0000000000000..d475e2e4c5b35 --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java @@ -0,0 +1,196 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.server.namenode; + +import org.apache.hadoop.hdfs.server.namenode.startupprogress.Phase; +import org.apache.hadoop.hdfs.server.namenode.startupprogress.StartupProgressView; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.*; +import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +/** For validating {@link INodeReference} subclasses. */ +public class INodeReferenceValidation { + public static final Logger LOG = LoggerFactory.getLogger(INodeReference.class); + + private static final AtomicReference INSTANCE = new AtomicReference<>(); + + public static void start() { + INSTANCE.compareAndSet(null, new INodeReferenceValidation()); + LOG.info("Validation started"); + } + + public static int end() { + final INodeReferenceValidation instance = INSTANCE.getAndSet(null); + if (instance == null) { + return 0; + } + + final int errorCount = instance.assertReferences(); + LOG.info("Validation ended successfully: {} error(s) found.", errorCount); + return errorCount; + } + + static void addWithCount(INodeReference.WithCount c) { + final INodeReferenceValidation validation = INSTANCE.get(); + if (validation != null) { + validation.withCounts.add(c); + LOG.info("addWithCount: " + c.toDetailString()); + } + } + + static void addWithName(INodeReference.WithName n) { + final INodeReferenceValidation validation = INSTANCE.get(); + if (validation != null) { + validation.withNames.add(n); + LOG.info("addWithName: {}", n.toDetailString()); + } + } + + static void addDstReference(INodeReference.DstReference d) { + final INodeReferenceValidation validation = INSTANCE.get(); + if (validation != null) { + validation.dstReferences.add(d); + LOG.info("addDstReference: {}", d.toDetailString()); + } + } + + static class Ref { + private final Class clazz; + private final List references = new LinkedList<>(); + private volatile List> tasks; + private volatile List> futures; + private final AtomicInteger taskCompleted = new AtomicInteger(); + + Ref(Class clazz) { + this.clazz = clazz; + } + + void add(REF ref) { + references.add(ref); + } + + void submit(AtomicInteger errorCount, ExecutorService service) + throws InterruptedException { + final int size = references.size(); + tasks = createTasks(references, errorCount); + LOG.info("Submitting {} tasks for validating {} {}(s)", + tasks.size(), size, clazz.getSimpleName()); + futures = service.invokeAll(tasks); + } + + void waitForFutures() throws Exception { + for(Future f : futures) { + f.get(); + taskCompleted.incrementAndGet(); + } + } + + double getTaskCompletedPercent() { + final List> t = tasks; + return t == null? 0 + : t.isEmpty()? 100 + : taskCompleted.get()*100.0/tasks.size(); + } + + @Override + public String toString() { + return String.format("%s %.1f%%", + clazz.getSimpleName(), getTaskCompletedPercent()); + } + } + + private final Ref withCounts + = new Ref<>(INodeReference.WithCount.class); + private final Ref withNames + = new Ref<>(INodeReference.WithName.class); + private final Ref dstReferences + = new Ref<>(INodeReference.DstReference.class); + + private int assertReferences() { + final int availableProcessors = Runtime.getRuntime().availableProcessors(); + LOG.info("Available Processors: {}", availableProcessors); + final ExecutorService service = Executors.newFixedThreadPool(availableProcessors); + + final TimerTask checkProgress = new TimerTask() { + @Override + public void run() { + LOG.info("ASSERT_REFERENCES Progress: {}, {}, {}", + dstReferences, withCounts, withNames); + } + }; + final Timer t = new Timer(); + t.scheduleAtFixedRate(checkProgress, 0, 60_000); + + final AtomicInteger errorCount = new AtomicInteger(); + try { + dstReferences.submit(errorCount, service); + withCounts.submit(errorCount, service); + withNames.submit(errorCount, service); + + dstReferences.waitForFutures(); + withCounts.waitForFutures(); + withNames.waitForFutures(); + } catch (Throwable e) { + LOG.error("Failed to assertReferences", e); + } finally { + service.shutdown(); + t.cancel(); + } + return errorCount.get(); + } + + static List> createTasks( + List references, AtomicInteger errorCount) { + final List> tasks = new LinkedList<>(); + for (final Iterator i = references.iterator(); i.hasNext(); ) { + tasks.add(new Task<>(i, errorCount)); + } + return tasks; + } + + static class Task implements Callable { + static final int BATCH_SIZE = 1000; + + private final List references = new LinkedList<>(); + private final AtomicInteger errorCount; + + Task(Iterator i, AtomicInteger errorCount) { + for(int n = 0; i.hasNext() && n < BATCH_SIZE; n++) { + references.add(i.next()); + i.remove(); + } + this.errorCount = errorCount; + } + + @Override + public Integer call() throws Exception { + for (final REF ref : references) { + try { + ref.assertReferences(); + } catch (Throwable t) { + LOG.error("{}: {}", errorCount.incrementAndGet(), t); + } + } + return references.size(); + } + } +} \ No newline at end of file diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java new file mode 100644 index 0000000000000..1e953de9bc713 --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.server.namenode; + +import org.apache.hadoop.test.GenericTestUtils; +import org.apache.log4j.Level; +import org.junit.Test; + +public class TestFsImageValidation { + static { + GenericTestUtils.setLogLevel(INode.LOG, Level.ALL); + GenericTestUtils.setLogLevel(INodeReferenceValidation.LOG, Level.ALL); + GenericTestUtils.setLogLevel(FsImageValidation.LOG, Level.ALL); + } + + /** Set environment variable FS_IMAGE_FILE to the path of the fsimage file being tested. */ + @Test + public void testINodeReference() throws Exception { + try { + FsImageValidation.checkINodeReference(FsImageValidation.getFsImageFile()); + } catch (IllegalArgumentException e) { + FsImageValidation.LOG.warn("The environment variable " + + FsImageValidation.FS_IMAGE_FILE + " is not set.", e); + } + } +} \ No newline at end of file From 8eaffd4fd40024e0e6aa46e38ee766b209411221 Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Wed, 15 Jul 2020 10:46:51 -0700 Subject: [PATCH 2/8] HDFS-15463. Fix warnings, unit tests. --- .../server/namenode/FsImageValidation.java | 119 +++++++++++------- .../hadoop/hdfs/server/namenode/INode.java | 2 +- .../hdfs/server/namenode/INodeReference.java | 23 +--- .../namenode/INodeReferenceValidation.java | 56 ++++++--- .../namenode/TestFsImageValidation.java | 21 ++-- 5 files changed, 125 insertions(+), 96 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java index 857d118da5ac6..89fca74f2befd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java @@ -18,17 +18,14 @@ package org.apache.hadoop.hdfs.server.namenode; import org.apache.hadoop.HadoopIllegalArgumentException; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configured; +import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.DFSUtil; import org.apache.hadoop.hdfs.HdfsConfiguration; import org.apache.hadoop.hdfs.server.namenode.startupprogress.Phase; -import org.apache.hadoop.hdfs.server.namenode.startupprogress.StartupProgressView; import org.apache.hadoop.hdfs.server.protocol.NamespaceInfo; import org.apache.hadoop.util.Tool; import org.apache.hadoop.util.ToolRunner; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.File; import java.util.Arrays; @@ -37,37 +34,59 @@ /** * For validating {@link FSImage}. - * - * This tool will load the user specified {@link FSImage} - * and then build the namespace tree in order to run the validation. + * This tool will load the user specified {@link FSImage}, + * build the namespace tree, + * and then run validations over the namespace tree. * * The main difference of this tool and * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer} * is that * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer} * only loads {@link FSImage} but it does not build the namespace tree. + * Therefore, running validations over the namespace tree is impossible in + * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer}. */ public class FsImageValidation { - static final Logger LOG = LoggerFactory.getLogger(FsImageValidation.class); - static final String FS_IMAGE_FILE = "FS_IMAGE_FILE"; - static int checkINodeReference(File fsImageFile) throws Exception { - LOG.info("Check INodeReference for {}: {}", FS_IMAGE_FILE, fsImageFile); + static HdfsConfiguration newHdfsConfiguration() { + final HdfsConfiguration conf = new HdfsConfiguration(); + final int aDay = 24*3600_000; + conf.setInt(DFSConfigKeys.DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY, aDay); + conf.setInt(DFSConfigKeys.DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY, aDay); + return conf; + } + + static FsImageValidation newInstance(String... args) { + final String f = Cli.parse(args); + if (f == null) { + throw new HadoopIllegalArgumentException( + FS_IMAGE_FILE + " is not specified."); + } + return new FsImageValidation(new File(f)); + } + + private final File fsImageFile; + + FsImageValidation(File fsImageFile) { + this.fsImageFile = fsImageFile; + } + + int checkINodeReference() throws Exception { + Cli.println("Check INodeReference for %d: %d", FS_IMAGE_FILE, fsImageFile); final TimerTask checkProgress = new TimerTask() { @Override public void run() { - final StartupProgressView view = NameNode.getStartupProgress().createView(); - final double percent = view.getPercentComplete(Phase.LOADING_FSIMAGE); - LOG.info("{} Progress: {}%", Phase.LOADING_FSIMAGE, - String.format("%.1f", 100 * percent)); + final double percent = NameNode.getStartupProgress().createView() + .getPercentComplete(Phase.LOADING_FSIMAGE); + Cli.println("%s Progress: %.1f%%", Phase.LOADING_FSIMAGE, 100*percent); } }; final Timer t = new Timer(); t.scheduleAtFixedRate(checkProgress, 0, 60_000); - final Configuration conf = new HdfsConfiguration(); + final HdfsConfiguration conf = newHdfsConfiguration(); final FSImage fsImage = new FSImage(conf); final FSNamesystem namesystem = new FSNamesystem(conf, fsImage, false); @@ -75,7 +94,8 @@ public void run() { namespaceInfo.clusterID = "cluster0"; fsImage.getStorage().setStorageInfo(namespaceInfo); - final FSImageFormat.LoaderDelegator loader = FSImageFormat.newLoader(conf, namesystem); + final FSImageFormat.LoaderDelegator loader + = FSImageFormat.newLoader(conf, namesystem); INodeReferenceValidation.start(); namesystem.writeLock(); namesystem.getFSDirectory().writeLock(); @@ -89,33 +109,6 @@ public void run() { return INodeReferenceValidation.end(); } - static File getFsImageFile(String... args) { - final String f = parse(args); - if (f == null) { - throw new HadoopIllegalArgumentException( - FS_IMAGE_FILE + " is not specified."); - } - return new File(f); - } - - static String parse(String... args) { - final String f; - if (args == null || args.length == 0) { - f = System.getenv().get(FS_IMAGE_FILE); - if (f != null) { - LOG.info("Environment variable {} = {}", FS_IMAGE_FILE, f); - } - } else if (args.length == 1) { - f = args[0]; - } else { - throw new HadoopIllegalArgumentException( - "args = " + Arrays.toString(args)); - } - - LOG.info("{} = {}", FS_IMAGE_FILE, f); - return f; - } - static class Cli extends Configured implements Tool { static final String COMMAND; static final String USAGE; @@ -125,12 +118,44 @@ static class Cli extends Configured implements Tool { USAGE = "Usage: hdfs " + COMMAND + " <" + FS_IMAGE_FILE + ">"; } + @Override public int run(String[] args) throws Exception { - final int errorCount = checkINodeReference(getFsImageFile(args)); - LOG.info("Error Count: {}", errorCount); + final FsImageValidation validation = FsImageValidation.newInstance(args); + final int errorCount = validation.checkINodeReference(); + println("Error Count: %s", errorCount); return errorCount == 0? 0: 1; } + + static String parse(String... args) { + final String f; + if (args == null || args.length == 0) { + f = System.getenv().get(FS_IMAGE_FILE); + if (f != null) { + println("Environment variable %s = %s", FS_IMAGE_FILE, f); + } + } else if (args.length == 1) { + f = args[0]; + } else { + throw new HadoopIllegalArgumentException( + "args = " + Arrays.toString(args)); + } + + println("%s = %s", FS_IMAGE_FILE, f); + return f; + } + + static void println(String format, Object... args) { + final String s = String.format(format, args); + System.out.println(s); + } + + static void printError(String message, Throwable t) { + System.out.println(message); + if (t != null) { + t.printStackTrace(System.out); + } + } } public static void main(String[] args) { @@ -146,7 +171,7 @@ public static void main(String[] args) { System.exit(-1); ToolRunner.printGenericCommandUsage(System.err); } catch (Throwable e) { - LOG.error("Failed to run " + Cli.COMMAND, e); + Cli.printError("Failed to run " + Cli.COMMAND, e); System.exit(-2); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java index 64099dd88a06a..fd416f72a3a20 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java @@ -703,7 +703,7 @@ public final void setParent(INodeDirectory parent) { } /** Set container. */ - public void setParentReference(INodeReference parent) { + public final void setParentReference(INodeReference parent) { this.parent = parent; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java index 0ca5a11d5f480..12ad703e7f98b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java @@ -67,7 +67,8 @@ public abstract class INodeReference extends INode { @Override public String toDetailString() { - final String s = referred == null? null: referred.getFullPathAndObjectString(); + final String s = referred == null? null + : referred.getFullPathAndObjectString(); return super.toDetailString() + ", ->" + s; } @@ -403,7 +404,7 @@ private String getCountDetails() { if (!withNameList.isEmpty()) { final Iterator i = withNameList.iterator(); b.append(i.next().getFullPathAndObjectString()); - for(; i.hasNext(); ) { + for(; i.hasNext();) { b.append(", ").append(i.next().getFullPathAndObjectString()); } } @@ -416,24 +417,6 @@ public String toDetailString() { return super.toDetailString() + getCountDetails(); } - @Override - public void setParentReference(INodeReference parentRef) { - if (parentRef != null) { - assertDstReference(parentRef); - - if (getParentReference() != null) { - throw new IllegalArgumentException("Overwriting Parent Reference:" - + "\n parentRef: " + parentRef.toDetailString() - + "\n withCount: " + this.toDetailString()); - } - } else { - final INodeReference old = getParentReference(); - LOG.debug("remove parent reference: parent={}, this={}", - old == null? null: old.toDetailString(), this.toDetailString()); - } - super.setParentReference(parentRef); - } - private void assertDstReference(INodeReference parentRef) { if (parentRef instanceof DstReference) { return; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java index d475e2e4c5b35..23848fd40da2e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java @@ -17,25 +17,35 @@ */ package org.apache.hadoop.hdfs.server.namenode; -import org.apache.hadoop.hdfs.server.namenode.startupprogress.Phase; -import org.apache.hadoop.hdfs.server.namenode.startupprogress.StartupProgressView; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.*; -import java.util.concurrent.*; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import static org.apache.hadoop.hdfs.server.namenode.FsImageValidation.Cli.printError; +import static org.apache.hadoop.hdfs.server.namenode.FsImageValidation.Cli.println; + /** For validating {@link INodeReference} subclasses. */ public class INodeReferenceValidation { - public static final Logger LOG = LoggerFactory.getLogger(INodeReference.class); + public static final Logger LOG = LoggerFactory.getLogger( + INodeReferenceValidation.class); - private static final AtomicReference INSTANCE = new AtomicReference<>(); + private static final AtomicReference INSTANCE + = new AtomicReference<>(); public static void start() { INSTANCE.compareAndSet(null, new INodeReferenceValidation()); - LOG.info("Validation started"); + println("Validation started"); } public static int end() { @@ -45,7 +55,7 @@ public static int end() { } final int errorCount = instance.assertReferences(); - LOG.info("Validation ended successfully: {} error(s) found.", errorCount); + println("Validation ended successfully: %d error(s) found.", errorCount); return errorCount; } @@ -53,7 +63,9 @@ static void addWithCount(INodeReference.WithCount c) { final INodeReferenceValidation validation = INSTANCE.get(); if (validation != null) { validation.withCounts.add(c); - LOG.info("addWithCount: " + c.toDetailString()); + if (LOG.isTraceEnabled()) { + LOG.trace("addWithCount: " + c.toDetailString()); + } } } @@ -61,7 +73,9 @@ static void addWithName(INodeReference.WithName n) { final INodeReferenceValidation validation = INSTANCE.get(); if (validation != null) { validation.withNames.add(n); - LOG.info("addWithName: {}", n.toDetailString()); + if (LOG.isTraceEnabled()) { + LOG.trace("addWithName: {}", n.toDetailString()); + } } } @@ -69,7 +83,9 @@ static void addDstReference(INodeReference.DstReference d) { final INodeReferenceValidation validation = INSTANCE.get(); if (validation != null) { validation.dstReferences.add(d); - LOG.info("addDstReference: {}", d.toDetailString()); + if (LOG.isTraceEnabled()) { + LOG.trace("addDstReference: {}", d.toDetailString()); + } } } @@ -92,7 +108,7 @@ void submit(AtomicInteger errorCount, ExecutorService service) throws InterruptedException { final int size = references.size(); tasks = createTasks(references, errorCount); - LOG.info("Submitting {} tasks for validating {} {}(s)", + println("Submitting %d tasks for validating %d %s(s)", tasks.size(), size, clazz.getSimpleName()); futures = service.invokeAll(tasks); } @@ -126,19 +142,19 @@ public String toString() { = new Ref<>(INodeReference.DstReference.class); private int assertReferences() { - final int availableProcessors = Runtime.getRuntime().availableProcessors(); - LOG.info("Available Processors: {}", availableProcessors); - final ExecutorService service = Executors.newFixedThreadPool(availableProcessors); + final int p = Runtime.getRuntime().availableProcessors(); + LOG.info("Available Processors: {}", p); + final ExecutorService service = Executors.newFixedThreadPool(p); final TimerTask checkProgress = new TimerTask() { @Override public void run() { - LOG.info("ASSERT_REFERENCES Progress: {}, {}, {}", + println("ASSERT_REFERENCES Progress: %s, %s, %s", dstReferences, withCounts, withNames); } }; final Timer t = new Timer(); - t.scheduleAtFixedRate(checkProgress, 0, 60_000); + t.scheduleAtFixedRate(checkProgress, 0, 10_000); final AtomicInteger errorCount = new AtomicInteger(); try { @@ -150,7 +166,7 @@ public void run() { withCounts.waitForFutures(); withNames.waitForFutures(); } catch (Throwable e) { - LOG.error("Failed to assertReferences", e); + printError("Failed to assertReferences", e); } finally { service.shutdown(); t.cancel(); @@ -161,7 +177,7 @@ public void run() { static List> createTasks( List references, AtomicInteger errorCount) { final List> tasks = new LinkedList<>(); - for (final Iterator i = references.iterator(); i.hasNext(); ) { + for (final Iterator i = references.iterator(); i.hasNext();) { tasks.add(new Task<>(i, errorCount)); } return tasks; @@ -187,7 +203,7 @@ public Integer call() throws Exception { try { ref.assertReferences(); } catch (Throwable t) { - LOG.error("{}: {}", errorCount.incrementAndGet(), t); + println("%d: %s", errorCount.incrementAndGet(), t); } } return references.size(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java index 1e953de9bc713..21ead96b04a11 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java @@ -17,24 +17,29 @@ */ package org.apache.hadoop.hdfs.server.namenode; +import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.test.GenericTestUtils; -import org.apache.log4j.Level; import org.junit.Test; +import org.slf4j.event.Level; public class TestFsImageValidation { static { - GenericTestUtils.setLogLevel(INode.LOG, Level.ALL); - GenericTestUtils.setLogLevel(INodeReferenceValidation.LOG, Level.ALL); - GenericTestUtils.setLogLevel(FsImageValidation.LOG, Level.ALL); + GenericTestUtils.setLogLevel(INode.LOG, Level.TRACE); + GenericTestUtils.setLogLevel(INodeReferenceValidation.LOG, Level.TRACE); } - /** Set environment variable FS_IMAGE_FILE to the path of the fsimage file being tested. */ + /** + * Run validation as a unit test. + * The path of the fsimage file being tested is specified + * by the environment variable FS_IMAGE_FILE. + */ @Test public void testINodeReference() throws Exception { try { - FsImageValidation.checkINodeReference(FsImageValidation.getFsImageFile()); - } catch (IllegalArgumentException e) { - FsImageValidation.LOG.warn("The environment variable " + final FsImageValidation validation = FsImageValidation.newInstance(); + validation.checkINodeReference(); + } catch (HadoopIllegalArgumentException e) { + FsImageValidation.Cli.printError("The environment variable " + FsImageValidation.FS_IMAGE_FILE + " is not set.", e); } } From 57c3b0ed56f31b6b65708525c78ae96fb9881a44 Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Wed, 15 Jul 2020 16:32:57 -0700 Subject: [PATCH 3/8] HDFS-15463. Support validation over a fsimag directory. --- .../server/namenode/FsImageValidation.java | 67 +++++++++++-------- .../namenode/TestFsImageValidation.java | 6 +- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java index 89fca74f2befd..ac89f4e280808 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java @@ -47,7 +47,7 @@ * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer}. */ public class FsImageValidation { - static final String FS_IMAGE_FILE = "FS_IMAGE_FILE"; + static final String FS_IMAGE = "FS_IMAGE"; static HdfsConfiguration newHdfsConfiguration() { final HdfsConfiguration conf = new HdfsConfiguration(); @@ -61,19 +61,19 @@ static FsImageValidation newInstance(String... args) { final String f = Cli.parse(args); if (f == null) { throw new HadoopIllegalArgumentException( - FS_IMAGE_FILE + " is not specified."); + FS_IMAGE + " is not specified."); } return new FsImageValidation(new File(f)); } - private final File fsImageFile; + private final File fsImage; - FsImageValidation(File fsImageFile) { - this.fsImageFile = fsImageFile; + FsImageValidation(File fsImage) { + this.fsImage = fsImage; } int checkINodeReference() throws Exception { - Cli.println("Check INodeReference for %d: %d", FS_IMAGE_FILE, fsImageFile); + Cli.println("Check INodeReference for %s", fsImage); final TimerTask checkProgress = new TimerTask() { @Override @@ -83,29 +83,39 @@ public void run() { Cli.println("%s Progress: %.1f%%", Phase.LOADING_FSIMAGE, 100*percent); } }; - final Timer t = new Timer(); - t.scheduleAtFixedRate(checkProgress, 0, 60_000); final HdfsConfiguration conf = newHdfsConfiguration(); - final FSImage fsImage = new FSImage(conf); - final FSNamesystem namesystem = new FSNamesystem(conf, fsImage, false); - final NamespaceInfo namespaceInfo = NNStorage.newNamespaceInfo(); - namespaceInfo.clusterID = "cluster0"; - fsImage.getStorage().setStorageInfo(namespaceInfo); - - final FSImageFormat.LoaderDelegator loader - = FSImageFormat.newLoader(conf, namesystem); INodeReferenceValidation.start(); - namesystem.writeLock(); - namesystem.getFSDirectory().writeLock(); - try { - loader.load(fsImageFile, false); - } finally { - namesystem.getFSDirectory().writeUnlock(); - namesystem.writeUnlock(); - t.cancel(); + final Timer t = new Timer(); + t.scheduleAtFixedRate(checkProgress, 0, 60_000); + if (fsImage.isDirectory()) { + Cli.println("Loading %s as a directory.", fsImage); + final String dir = fsImage.getCanonicalPath(); + conf.set(DFSConfigKeys.DFS_NAMENODE_NAME_DIR_KEY, dir); + conf.set(DFSConfigKeys.DFS_NAMENODE_EDITS_DIR_KEY, dir); + FSNamesystem.loadFromDisk(conf); + } else { + Cli.println("Loading %s as a file.", fsImage); + final FSImage fsImage = new FSImage(conf); + final FSNamesystem namesystem = new FSNamesystem(conf, fsImage, false); + + final NamespaceInfo namespaceInfo = NNStorage.newNamespaceInfo(); + namespaceInfo.clusterID = "cluster0"; + fsImage.getStorage().setStorageInfo(namespaceInfo); + + final FSImageFormat.LoaderDelegator loader + = FSImageFormat.newLoader(conf, namesystem); + namesystem.writeLock(); + namesystem.getFSDirectory().writeLock(); + try { + loader.load(this.fsImage, false); + } finally { + namesystem.getFSDirectory().writeUnlock(); + namesystem.writeUnlock(); + } } + t.cancel(); return INodeReferenceValidation.end(); } @@ -115,10 +125,9 @@ static class Cli extends Configured implements Tool { static { final String clazz = FsImageValidation.class.getSimpleName(); COMMAND = Character.toLowerCase(clazz.charAt(0)) + clazz.substring(1); - USAGE = "Usage: hdfs " + COMMAND + " <" + FS_IMAGE_FILE + ">"; + USAGE = "Usage: hdfs " + COMMAND + " <" + FS_IMAGE + ">"; } - @Override public int run(String[] args) throws Exception { final FsImageValidation validation = FsImageValidation.newInstance(args); @@ -130,9 +139,9 @@ public int run(String[] args) throws Exception { static String parse(String... args) { final String f; if (args == null || args.length == 0) { - f = System.getenv().get(FS_IMAGE_FILE); + f = System.getenv().get(FS_IMAGE); if (f != null) { - println("Environment variable %s = %s", FS_IMAGE_FILE, f); + println("Environment variable %s = %s", FS_IMAGE, f); } } else if (args.length == 1) { f = args[0]; @@ -141,7 +150,7 @@ static String parse(String... args) { "args = " + Arrays.toString(args)); } - println("%s = %s", FS_IMAGE_FILE, f); + println("%s = %s", FS_IMAGE, f); return f; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java index 21ead96b04a11..994f4efa14e31 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java @@ -19,6 +19,7 @@ import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.test.GenericTestUtils; +import org.junit.Assert; import org.junit.Test; import org.slf4j.event.Level; @@ -37,10 +38,11 @@ public class TestFsImageValidation { public void testINodeReference() throws Exception { try { final FsImageValidation validation = FsImageValidation.newInstance(); - validation.checkINodeReference(); + final int errorCount = validation.checkINodeReference(); + Assert.assertEquals("Error Count: " + errorCount, 0, errorCount); } catch (HadoopIllegalArgumentException e) { FsImageValidation.Cli.printError("The environment variable " - + FsImageValidation.FS_IMAGE_FILE + " is not set.", e); + + FsImageValidation.FS_IMAGE + " is not set.", e); } } } \ No newline at end of file From 8f3ff4eede9a7cdad7402754c433906600ec5c33 Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Fri, 17 Jul 2020 16:13:08 -0700 Subject: [PATCH 4/8] HDFS-15463. When an INodeReference object is removed from namespace, remove it also from INodeReferenceValidation. --- .../hdfs/server/namenode/FSNamesystem.java | 2 +- .../server/namenode/FsImageValidation.java | 143 ++++++++++++++---- .../hdfs/server/namenode/INodeReference.java | 17 ++- .../namenode/INodeReferenceValidation.java | 85 ++++++----- .../namenode/TestFsImageValidation.java | 13 +- 5 files changed, 185 insertions(+), 75 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 993c2832dce94..fe39b071e207c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -1179,7 +1179,7 @@ private List initAuditLoggers(Configuration conf) { return Collections.unmodifiableList(auditLoggers); } - private void loadFSImage(StartupOption startOpt) throws IOException { + void loadFSImage(StartupOption startOpt) throws IOException { final FSImage fsImage = getFSImage(); // format before starting up if requested diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java index ac89f4e280808..539a45f078452 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java @@ -17,21 +17,40 @@ */ package org.apache.hadoop.hdfs.server.namenode; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.commons.logging.impl.Log4JLogger; import org.apache.hadoop.HadoopIllegalArgumentException; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configured; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.DFSUtil; -import org.apache.hadoop.hdfs.HdfsConfiguration; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; +import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeManager; +import org.apache.hadoop.hdfs.server.common.HdfsServerConstants; import org.apache.hadoop.hdfs.server.namenode.startupprogress.Phase; +import org.apache.hadoop.hdfs.server.namenode.top.metrics.TopMetrics; import org.apache.hadoop.hdfs.server.protocol.NamespaceInfo; +import org.apache.hadoop.util.GSet; +import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.util.Tool; import org.apache.hadoop.util.ToolRunner; +import org.apache.log4j.Level; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.File; import java.util.Arrays; import java.util.Timer; import java.util.TimerTask; +import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_HA_NAMENODES_KEY_PREFIX; +import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_ENABLE_RETRY_CACHE_KEY; +import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY; +import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_RPC_ADDRESS_KEY; +import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY; +import static org.apache.hadoop.util.Time.now; + /** * For validating {@link FSImage}. * This tool will load the user specified {@link FSImage}, @@ -47,15 +66,10 @@ * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer}. */ public class FsImageValidation { - static final String FS_IMAGE = "FS_IMAGE"; + public static final Logger LOG = LoggerFactory.getLogger( + FsImageValidation.class); - static HdfsConfiguration newHdfsConfiguration() { - final HdfsConfiguration conf = new HdfsConfiguration(); - final int aDay = 24*3600_000; - conf.setInt(DFSConfigKeys.DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY, aDay); - conf.setInt(DFSConfigKeys.DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY, aDay); - return conf; - } + static final String FS_IMAGE = "FS_IMAGE"; static FsImageValidation newInstance(String... args) { final String f = Cli.parse(args); @@ -66,39 +80,105 @@ static FsImageValidation newInstance(String... args) { return new FsImageValidation(new File(f)); } - private final File fsImage; + static void initConf(Configuration conf) { + final int aDay = 24*3600_000; + conf.setInt(DFS_NAMENODE_READ_LOCK_REPORTING_THRESHOLD_MS_KEY, aDay); + conf.setInt(DFS_NAMENODE_WRITE_LOCK_REPORTING_THRESHOLD_MS_KEY, aDay); + conf.setBoolean(DFS_NAMENODE_ENABLE_RETRY_CACHE_KEY, false); + } - FsImageValidation(File fsImage) { - this.fsImage = fsImage; + /** Simulate HA so that the editlogs are not loaded. */ + static void setHaConf(String nsId, Configuration conf) { + conf.set(DFSConfigKeys.DFS_NAMESERVICES, nsId); + final String haNNKey = DFS_HA_NAMENODES_KEY_PREFIX + "." + nsId; + conf.set(haNNKey, "nn0,nn1"); + final String rpcKey = DFS_NAMENODE_RPC_ADDRESS_KEY + "." + nsId + "."; + conf.set(rpcKey + "nn0", "127.0.0.1:8080"); + conf.set(rpcKey + "nn1", "127.0.0.1:8080"); } - int checkINodeReference() throws Exception { - Cli.println("Check INodeReference for %s", fsImage); + static void initLogLevels() { + Util.setLogLevel(FSImage.class, Level.TRACE); + Util.setLogLevel(FileJournalManager.class, Level.TRACE); + + Util.setLogLevel(GSet.class, Level.OFF); + Util.setLogLevel(BlockManager.class, Level.OFF); + Util.setLogLevel(DatanodeManager.class, Level.OFF); + Util.setLogLevel(TopMetrics.class, Level.OFF); + } + + static class Util { + static String memoryInfo() { + final Runtime runtime = Runtime.getRuntime(); + return "Memory Info: free=" + StringUtils.byteDesc(runtime.freeMemory()) + + ", total=" + StringUtils.byteDesc(runtime.totalMemory()) + + ", max=" + StringUtils.byteDesc(runtime.maxMemory()); + } + + static void setLogLevel(Class clazz, Level level) { + final Log log = LogFactory.getLog(clazz); + if (log instanceof Log4JLogger) { + final org.apache.log4j.Logger logger = ((Log4JLogger) log).getLogger(); + logger.setLevel(level); + LOG.info("setLogLevel {} to {}, getEffectiveLevel() = {}", + clazz.getName(), level, logger.getEffectiveLevel()); + } else { + LOG.warn("Failed setLogLevel {} to {}", clazz.getName(), level); + } + } + + static String toCommaSeparatedNumber(long n) { + final StringBuilder b = new StringBuilder(); + for(; n > 999;) { + b.insert(0, String.format(",%03d", n%1000)); + n /= 1000; + } + return b.insert(0, n).toString(); + } + } + + private final File fsImageFile; + + FsImageValidation(File fsImageFile) { + this.fsImageFile = fsImageFile; + } + + int checkINodeReference(Configuration conf) throws Exception { + LOG.info(Util.memoryInfo()); + initConf(conf); final TimerTask checkProgress = new TimerTask() { @Override public void run() { final double percent = NameNode.getStartupProgress().createView() .getPercentComplete(Phase.LOADING_FSIMAGE); - Cli.println("%s Progress: %.1f%%", Phase.LOADING_FSIMAGE, 100*percent); + LOG.info(String.format("%s Progress: %.1f%%", + Phase.LOADING_FSIMAGE, 100*percent)); } }; - final HdfsConfiguration conf = newHdfsConfiguration(); - INodeReferenceValidation.start(); final Timer t = new Timer(); t.scheduleAtFixedRate(checkProgress, 0, 60_000); - if (fsImage.isDirectory()) { - Cli.println("Loading %s as a directory.", fsImage); - final String dir = fsImage.getCanonicalPath(); + final long loadStart = now(); + final FSNamesystem namesystem; + if (fsImageFile.isDirectory()) { + Cli.println("Loading %s as a directory.", fsImageFile); + final String dir = fsImageFile.getCanonicalPath(); conf.set(DFSConfigKeys.DFS_NAMENODE_NAME_DIR_KEY, dir); conf.set(DFSConfigKeys.DFS_NAMENODE_EDITS_DIR_KEY, dir); - FSNamesystem.loadFromDisk(conf); + + + final FSImage fsImage = new FSImage(conf); + namesystem = new FSNamesystem(conf, fsImage, true); + // Avoid saving fsimage + namesystem.setRollingUpgradeInfo(false, 0); + + namesystem.loadFSImage(HdfsServerConstants.StartupOption.REGULAR); } else { - Cli.println("Loading %s as a file.", fsImage); + Cli.println("Loading %s as a file.", fsImageFile); final FSImage fsImage = new FSImage(conf); - final FSNamesystem namesystem = new FSNamesystem(conf, fsImage, false); + namesystem = new FSNamesystem(conf, fsImage, true); final NamespaceInfo namespaceInfo = NNStorage.newNamespaceInfo(); namespaceInfo.clusterID = "cluster0"; @@ -109,14 +189,19 @@ public void run() { namesystem.writeLock(); namesystem.getFSDirectory().writeLock(); try { - loader.load(this.fsImage, false); + loader.load(fsImageFile, false); } finally { namesystem.getFSDirectory().writeUnlock(); namesystem.writeUnlock(); } } t.cancel(); - return INodeReferenceValidation.end(); + Cli.println("Loaded %s %s successfully in %s", + FS_IMAGE, fsImageFile, StringUtils.formatTime(now() - loadStart)); + LOG.info(Util.memoryInfo()); + final int errorCount = INodeReferenceValidation.end(); + LOG.info(Util.memoryInfo()); + return errorCount; } static class Cli extends Configured implements Tool { @@ -130,8 +215,10 @@ static class Cli extends Configured implements Tool { @Override public int run(String[] args) throws Exception { + initLogLevels(); + final FsImageValidation validation = FsImageValidation.newInstance(args); - final int errorCount = validation.checkINodeReference(); + final int errorCount = validation.checkINodeReference(getConf()); println("Error Count: %s", errorCount); return errorCount == 0? 0: 1; } @@ -157,6 +244,7 @@ static String parse(String... args) { static void println(String format, Object... args) { final String s = String.format(format, args); System.out.println(s); + LOG.info(s); } static void printError(String message, Throwable t) { @@ -164,6 +252,7 @@ static void printError(String message, Throwable t) { if (t != null) { t.printStackTrace(System.out); } + LOG.error(message, t); } } @@ -173,7 +262,7 @@ public static void main(String[] args) { } try { - System.exit(ToolRunner.run(new HdfsConfiguration(), new Cli(), args)); + System.exit(ToolRunner.run(new Configuration(), new Cli(), args)); } catch (HadoopIllegalArgumentException e) { e.printStackTrace(System.err); System.err.println(Cli.USAGE); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java index 12ad703e7f98b..fca320daedcf0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java @@ -396,7 +396,7 @@ public WithCount(INodeReference parent, INode referred) { Preconditions.checkArgument(parent == null); referred.setParentReference(this); - INodeReferenceValidation.addWithCount(this); + INodeReferenceValidation.add(this, WithCount.class); } private String getCountDetails() { @@ -480,12 +480,19 @@ private int search(WithName ref) { /** Decrement and then return the reference count. */ public void removeReference(INodeReference ref) { if (ref instanceof WithName) { - final int i = search((WithName) ref); + final WithName withName = (WithName) ref; + final int i = search(withName); if (i >= 0) { withNameList.remove(i); + INodeReferenceValidation.remove(withName, WithName.class); } } else if (ref == getParentReference()) { - setParentReference(null); + setParent(null); + INodeReferenceValidation.remove((DstReference) ref, DstReference.class); + } + + if (getReferenceCount() == 0) { + INodeReferenceValidation.remove(this, WithCount.class); } } @@ -552,7 +559,7 @@ public WithName(INodeDirectory parent, WithCount referred, byte[] name, this.lastSnapshotId = lastSnapshotId; referred.addReference(this); - INodeReferenceValidation.addWithName(this); + INodeReferenceValidation.add(this, WithName.class); } @Override @@ -740,7 +747,7 @@ public DstReference(INodeDirectory parent, WithCount referred, this.dstSnapshotId = dstSnapshotId; referred.addReference(this); - INodeReferenceValidation.addDstReference(this); + INodeReferenceValidation.add(this, DstReference.class); } @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java index 23848fd40da2e..d6c3003275885 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hdfs.server.namenode; +import com.google.common.base.Preconditions; +import org.apache.hadoop.hdfs.server.namenode.FsImageValidation.Util; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,8 +34,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import static org.apache.hadoop.hdfs.server.namenode.FsImageValidation.Cli.printError; -import static org.apache.hadoop.hdfs.server.namenode.FsImageValidation.Cli.println; +import static org.apache.hadoop.hdfs.server.namenode.FsImageValidation.Cli.*; /** For validating {@link INodeReference} subclasses. */ public class INodeReferenceValidation { @@ -59,57 +60,56 @@ public static int end() { return errorCount; } - static void addWithCount(INodeReference.WithCount c) { + static void add(REF ref, Class clazz) { final INodeReferenceValidation validation = INSTANCE.get(); if (validation != null) { - validation.withCounts.add(c); - if (LOG.isTraceEnabled()) { - LOG.trace("addWithCount: " + c.toDetailString()); - } - } - } - - static void addWithName(INodeReference.WithName n) { - final INodeReferenceValidation validation = INSTANCE.get(); - if (validation != null) { - validation.withNames.add(n); - if (LOG.isTraceEnabled()) { - LOG.trace("addWithName: {}", n.toDetailString()); - } + final boolean added = validation.getReferences(clazz).add(ref); + Preconditions.checkState(added); + LOG.trace("add {}: {}", clazz, ref.toDetailString()); } } - static void addDstReference(INodeReference.DstReference d) { + static void remove(REF ref, Class clazz) { final INodeReferenceValidation validation = INSTANCE.get(); if (validation != null) { - validation.dstReferences.add(d); - if (LOG.isTraceEnabled()) { - LOG.trace("addDstReference: {}", d.toDetailString()); - } + final boolean removed = validation.getReferences(clazz).remove(ref); + Preconditions.checkState(removed); + LOG.trace("remove {}: {}", clazz, ref.toDetailString()); } } - static class Ref { + static class ReferenceSet { private final Class clazz; private final List references = new LinkedList<>(); private volatile List> tasks; private volatile List> futures; private final AtomicInteger taskCompleted = new AtomicInteger(); - Ref(Class clazz) { + ReferenceSet(Class clazz) { this.clazz = clazz; } - void add(REF ref) { - references.add(ref); + boolean add(REF ref) { + return references.add(ref); + } + + boolean remove(REF ref) { + for(final Iterator i = references.iterator(); i.hasNext();) { + if (i.next() == ref) { + i.remove(); + return true; + } + } + return false; } void submit(AtomicInteger errorCount, ExecutorService service) throws InterruptedException { final int size = references.size(); tasks = createTasks(references, errorCount); - println("Submitting %d tasks for validating %d %s(s)", - tasks.size(), size, clazz.getSimpleName()); + println("Submitting %d tasks for validating %s %s(s)", + tasks.size(), Util.toCommaSeparatedNumber(size), + clazz.getSimpleName()); futures = service.invokeAll(tasks); } @@ -134,12 +134,23 @@ public String toString() { } } - private final Ref withCounts - = new Ref<>(INodeReference.WithCount.class); - private final Ref withNames - = new Ref<>(INodeReference.WithName.class); - private final Ref dstReferences - = new Ref<>(INodeReference.DstReference.class); + private final ReferenceSet withCounts + = new ReferenceSet<>(INodeReference.WithCount.class); + private final ReferenceSet withNames + = new ReferenceSet<>(INodeReference.WithName.class); + private final ReferenceSet dstReferences + = new ReferenceSet<>(INodeReference.DstReference.class); + + ReferenceSet getReferences(Class clazz) { + if (clazz == INodeReference.WithCount.class) { + return (ReferenceSet) withCounts; + } else if (clazz == INodeReference.WithName.class) { + return (ReferenceSet) withNames; + } else if (clazz == INodeReference.DstReference.class) { + return (ReferenceSet) dstReferences; + } + throw new IllegalArgumentException("References not found for " + clazz); + } private int assertReferences() { final int p = Runtime.getRuntime().availableProcessors(); @@ -149,12 +160,12 @@ private int assertReferences() { final TimerTask checkProgress = new TimerTask() { @Override public void run() { - println("ASSERT_REFERENCES Progress: %s, %s, %s", + LOG.info("ASSERT_REFERENCES Progress: {}, {}, {}", dstReferences, withCounts, withNames); } }; final Timer t = new Timer(); - t.scheduleAtFixedRate(checkProgress, 0, 10_000); + t.scheduleAtFixedRate(checkProgress, 0, 1_000); final AtomicInteger errorCount = new AtomicInteger(); try { @@ -184,7 +195,7 @@ static List> createTasks( } static class Task implements Callable { - static final int BATCH_SIZE = 1000; + static final int BATCH_SIZE = 100_000; private final List references = new LinkedList<>(); private final AtomicInteger errorCount; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java index 994f4efa14e31..b3b008d4cebcc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java @@ -18,15 +18,16 @@ package org.apache.hadoop.hdfs.server.namenode; import org.apache.hadoop.HadoopIllegalArgumentException; -import org.apache.hadoop.test.GenericTestUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.log4j.Level; import org.junit.Assert; import org.junit.Test; -import org.slf4j.event.Level; public class TestFsImageValidation { static { - GenericTestUtils.setLogLevel(INode.LOG, Level.TRACE); - GenericTestUtils.setLogLevel(INodeReferenceValidation.LOG, Level.TRACE); + FsImageValidation.Util.setLogLevel(FsImageValidation.class, Level.TRACE); + FsImageValidation.Util.setLogLevel(INodeReferenceValidation.class, Level.TRACE); + FsImageValidation.Util.setLogLevel(INode.class, Level.TRACE); } /** @@ -36,9 +37,11 @@ public class TestFsImageValidation { */ @Test public void testINodeReference() throws Exception { + FsImageValidation.initLogLevels(); + try { final FsImageValidation validation = FsImageValidation.newInstance(); - final int errorCount = validation.checkINodeReference(); + final int errorCount = validation.checkINodeReference(new Configuration()); Assert.assertEquals("Error Count: " + errorCount, 0, errorCount); } catch (HadoopIllegalArgumentException e) { FsImageValidation.Cli.printError("The environment variable " From cd37d796f6cdb151d6f054974bf760a00a3f2ece Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Fri, 17 Jul 2020 16:20:50 -0700 Subject: [PATCH 5/8] revert a minor change. --- .../apache/hadoop/hdfs/server/namenode/INodeReference.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java index fca320daedcf0..69a92706ab50b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReference.java @@ -466,10 +466,6 @@ public void addReference(INodeReference ref) { withNameList.add(-i - 1, refWithName); } else if (ref instanceof DstReference) { setParentReference(ref); - } else { - throw new IllegalStateException("Failed to add reference:" - + "\n ref: " + ref.toDetailString() - + "\n withCount: " + this.toDetailString()); } } From 078233f9a9657c8730a259f4120e8142455f3ac1 Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Fri, 17 Jul 2020 16:33:32 -0700 Subject: [PATCH 6/8] Add toString() to EditLogInputStream. --- .../hadoop/hdfs/server/namenode/EditLogInputStream.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogInputStream.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogInputStream.java index a4377cddcde7b..8f324fbcc0382 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogInputStream.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogInputStream.java @@ -209,4 +209,9 @@ FSEditLogOp getCachedOp() { * even faster data source (e.g. a byte buffer). */ public abstract boolean isLocalLog(); + + @Override + public String toString() { + return getName(); + } } From 3d94ac15eaa412d814a1245ffd94273777175b93 Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Sat, 18 Jul 2020 10:35:27 -0700 Subject: [PATCH 7/8] Fix checkstyle warnings and add unit tests. --- .../server/namenode/FsImageValidation.java | 5 +- .../namenode/INodeReferenceValidation.java | 3 +- .../namenode/TestFsImageValidation.java | 54 ++++++++++++++++--- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java index 539a45f078452..5dcb5069e870c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FsImageValidation.java @@ -66,8 +66,7 @@ * {@link org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer}. */ public class FsImageValidation { - public static final Logger LOG = LoggerFactory.getLogger( - FsImageValidation.class); + static final Logger LOG = LoggerFactory.getLogger(FsImageValidation.class); static final String FS_IMAGE = "FS_IMAGE"; @@ -87,7 +86,7 @@ static void initConf(Configuration conf) { conf.setBoolean(DFS_NAMENODE_ENABLE_RETRY_CACHE_KEY, false); } - /** Simulate HA so that the editlogs are not loaded. */ + /** Set (fake) HA so that edit logs will not be loaded. */ static void setHaConf(String nsId, Configuration conf) { conf.set(DFSConfigKeys.DFS_NAMESERVICES, nsId); final String haNNKey = DFS_HA_NAMENODES_KEY_PREFIX + "." + nsId; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java index d6c3003275885..d3faf43074161 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeReferenceValidation.java @@ -141,7 +141,8 @@ public String toString() { private final ReferenceSet dstReferences = new ReferenceSet<>(INodeReference.DstReference.class); - ReferenceSet getReferences(Class clazz) { + ReferenceSet getReferences( + Class clazz) { if (clazz == INodeReference.WithCount.class) { return (ReferenceSet) withCounts; } else if (clazz == INodeReference.WithName.class) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java index b3b008d4cebcc..882aff512a21e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java @@ -19,15 +19,22 @@ import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hdfs.HAUtil; import org.apache.log4j.Level; import org.junit.Assert; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class TestFsImageValidation { + static final Logger LOG = LoggerFactory.getLogger( + TestFsImageValidation.class); + static { - FsImageValidation.Util.setLogLevel(FsImageValidation.class, Level.TRACE); - FsImageValidation.Util.setLogLevel(INodeReferenceValidation.class, Level.TRACE); - FsImageValidation.Util.setLogLevel(INode.class, Level.TRACE); + final Level t = Level.TRACE; + FsImageValidation.Util.setLogLevel(FsImageValidation.class, t); + FsImageValidation.Util.setLogLevel(INodeReferenceValidation.class, t); + FsImageValidation.Util.setLogLevel(INode.class, t); } /** @@ -40,12 +47,47 @@ public void testINodeReference() throws Exception { FsImageValidation.initLogLevels(); try { + final Configuration conf = new Configuration(); final FsImageValidation validation = FsImageValidation.newInstance(); - final int errorCount = validation.checkINodeReference(new Configuration()); + final int errorCount = validation.checkINodeReference(conf); Assert.assertEquals("Error Count: " + errorCount, 0, errorCount); } catch (HadoopIllegalArgumentException e) { - FsImageValidation.Cli.printError("The environment variable " - + FsImageValidation.FS_IMAGE + " is not set.", e); + LOG.warn("The environment variable {} is not set: {}", + FsImageValidation.FS_IMAGE, e); + } + } + + @Test + public void testHaConf() { + final Configuration conf = new Configuration(); + final String nsId = "cluster0"; + FsImageValidation.setHaConf(nsId, conf); + Assert.assertTrue(HAUtil.isHAEnabled(conf, nsId)); + } + + @Test + public void testToCommaSeparatedNumber() { + for(long b = 1; b < Integer.MAX_VALUE; ) { + for (long n = b; n < Integer.MAX_VALUE; n *= 10) { + runTestToCommaSeparatedNumber(n); + } + b = b == 1? 11: 10*(b-1) + 1; + } + } + + static void runTestToCommaSeparatedNumber(long n) { + final String s = FsImageValidation.Util.toCommaSeparatedNumber(n); + LOG.info("{} ?= {}", n, s); + for(int i = s.length(); i > 0; ) { + for(int j = 0; j < 3 && i > 0; j++) { + Assert.assertTrue(Character.isDigit(s.charAt(--i))); + } + if (i > 0) { + Assert.assertEquals(',', s.charAt(--i)); + } } + + Assert.assertNotEquals(0, s.length()%4); + Assert.assertEquals(n, Long.parseLong(s.replaceAll(",", ""))); } } \ No newline at end of file From a8f246ff7c86960b423d1513c4984efa0f70e4ba Mon Sep 17 00:00:00 2001 From: Tsz Wo Nicholas Sze Date: Sun, 19 Jul 2020 11:01:06 -0700 Subject: [PATCH 8/8] Fix checkstyle warnings in TestFsImageValidation. --- .../hadoop/hdfs/server/namenode/TestFsImageValidation.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java index 882aff512a21e..97bb2b9a33f9f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsImageValidation.java @@ -67,7 +67,7 @@ public void testHaConf() { @Test public void testToCommaSeparatedNumber() { - for(long b = 1; b < Integer.MAX_VALUE; ) { + for(long b = 1; b < Integer.MAX_VALUE;) { for (long n = b; n < Integer.MAX_VALUE; n *= 10) { runTestToCommaSeparatedNumber(n); } @@ -78,7 +78,7 @@ public void testToCommaSeparatedNumber() { static void runTestToCommaSeparatedNumber(long n) { final String s = FsImageValidation.Util.toCommaSeparatedNumber(n); LOG.info("{} ?= {}", n, s); - for(int i = s.length(); i > 0; ) { + for(int i = s.length(); i > 0;) { for(int j = 0; j < 3 && i > 0; j++) { Assert.assertTrue(Character.isDigit(s.charAt(--i))); }