From 9863cb19059e93f06f0566961e99d58d246239f8 Mon Sep 17 00:00:00 2001 From: Colin McCabe Date: Thu, 21 Nov 2013 20:15:07 +0000 Subject: [PATCH] HDFS-5543. Fix narrow race condition in TestPathBasedCacheRequests (cmccabe) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1544310 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 + .../server/namenode/TestCacheDirectives.java | 106 ++++++++++-------- 2 files changed, 64 insertions(+), 45 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 5c3d2a3cf9..0c571e4887 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -397,6 +397,9 @@ Trunk (Unreleased) HDFS-5513. CacheAdmin commands fail when using . as the path. (wang) + HDFS-5543. Fix narrow race condition in TestPathBasedCacheRequests + (cmccabe) + Release 2.3.0 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCacheDirectives.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCacheDirectives.java index 325c225127..ffd8d18993 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCacheDirectives.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCacheDirectives.java @@ -614,6 +614,47 @@ public Boolean get() { }, 500, 60000); } + private static void waitForCachedStats(final DistributedFileSystem dfs, + final long targetFilesAffected, final long targetBytesNeeded, + final long targetBytesCached, + final CacheDirectiveInfo filter, final String infoString) + throws Exception { + LOG.info("Polling listDirectives{" + + ((filter == null) ? "ALL" : filter.toString()) + + " for " + targetFilesAffected + " targetFilesAffected, " + + targetBytesNeeded + " targetBytesNeeded, " + + targetBytesCached + " targetBytesCached"); + GenericTestUtils.waitFor(new Supplier() { + @Override + public Boolean get() { + RemoteIterator iter = null; + CacheDirectiveEntry entry = null; + try { + iter = dfs.listCacheDirectives(filter); + entry = iter.next(); + } catch (IOException e) { + fail("got IOException while calling " + + "listCacheDirectives: " + e.getMessage()); + } + Assert.assertNotNull(entry); + CacheDirectiveStats stats = entry.getStats(); + if ((targetFilesAffected == stats.getFilesAffected()) && + (targetBytesNeeded == stats.getBytesNeeded()) && + (targetBytesCached == stats.getBytesCached())) { + return true; + } else { + LOG.info(infoString + ": filesAffected: " + + stats.getFilesAffected() + "/" + targetFilesAffected + + ", bytesNeeded: " + + stats.getBytesNeeded() + "/" + targetBytesNeeded + + ", bytesCached: " + + stats.getBytesCached() + "/" + targetBytesCached); + return false; + } + } + }, 500, 60000); + } + private static void checkNumCachedReplicas(final DistributedFileSystem dfs, final List paths, final int expectedBlocks, final int expectedReplicas) @@ -804,21 +845,12 @@ public void testWaitForCachedReplicasInDirectory() throws Exception { waitForCachedBlocks(namenode, 4, 8, "testWaitForCachedReplicasInDirectory:1"); // Verify that listDirectives gives the stats we want. - RemoteIterator iter = - dfs.listCacheDirectives(new CacheDirectiveInfo.Builder(). - setPath(new Path("/foo")). - build()); - CacheDirectiveEntry entry = iter.next(); - CacheDirectiveStats stats = entry.getStats(); - Assert.assertEquals(Long.valueOf(2), - stats.getFilesAffected()); - Assert.assertEquals(Long.valueOf( - 2 * numBlocksPerFile * BLOCK_SIZE * 2), - stats.getBytesNeeded()); - Assert.assertEquals(Long.valueOf( - 2 * numBlocksPerFile * BLOCK_SIZE * 2), - stats.getBytesCached()); - + waitForCachedStats(dfs, 2, + 8 * BLOCK_SIZE, 8 * BLOCK_SIZE, + new CacheDirectiveInfo.Builder(). + setPath(new Path("/foo")). + build(), + "testWaitForCachedReplicasInDirectory:2"); long id2 = dfs.addCacheDirective( new CacheDirectiveInfo.Builder(). setPath(new Path("/foo/bar")). @@ -827,44 +859,28 @@ public void testWaitForCachedReplicasInDirectory() throws Exception { build()); // wait for an additional 2 cached replicas to come up waitForCachedBlocks(namenode, 4, 10, - "testWaitForCachedReplicasInDirectory:2"); + "testWaitForCachedReplicasInDirectory:3"); // the directory directive's stats are unchanged - iter = dfs.listCacheDirectives( + waitForCachedStats(dfs, 2, + 8 * BLOCK_SIZE, 8 * BLOCK_SIZE, new CacheDirectiveInfo.Builder(). - setPath(new Path("/foo")). - build()); - entry = iter.next(); - stats = entry.getStats(); - Assert.assertEquals(Long.valueOf(2), - stats.getFilesAffected()); - Assert.assertEquals(Long.valueOf( - 2 * numBlocksPerFile * BLOCK_SIZE * 2), - stats.getBytesNeeded()); - Assert.assertEquals(Long.valueOf( - 2 * numBlocksPerFile * BLOCK_SIZE * 2), - stats.getBytesCached()); + setPath(new Path("/foo")). + build(), + "testWaitForCachedReplicasInDirectory:4"); // verify /foo/bar's stats - iter = dfs.listCacheDirectives( + waitForCachedStats(dfs, 1, + 4 * numBlocksPerFile * BLOCK_SIZE, + // only 3 because the file only has 3 replicas, not 4 as requested. + 3 * numBlocksPerFile * BLOCK_SIZE, new CacheDirectiveInfo.Builder(). - setPath(new Path("/foo/bar")). - build()); - entry = iter.next(); - stats = entry.getStats(); - Assert.assertEquals(Long.valueOf(1), - stats.getFilesAffected()); - Assert.assertEquals(Long.valueOf( - 4 * numBlocksPerFile * BLOCK_SIZE), - stats.getBytesNeeded()); - // only 3 because the file only has 3 replicas, not 4 as requested. - Assert.assertEquals(Long.valueOf( - 3 * numBlocksPerFile * BLOCK_SIZE), - stats.getBytesCached()); - + setPath(new Path("/foo/bar")). + build(), + "testWaitForCachedReplicasInDirectory:5"); // remove and watch numCached go to 0 dfs.removeCacheDirective(id); dfs.removeCacheDirective(id2); waitForCachedBlocks(namenode, 0, 0, - "testWaitForCachedReplicasInDirectory:3"); + "testWaitForCachedReplicasInDirectory:6"); } finally { cluster.shutdown(); }