The attached test may generate the scenario where a pendingReplication record is left in BlockManager#pendingReplications until timeout.

And a simple patch uploaded.

Patch looks good. Can you combine the test and the fix?

In the test please remove unnecessary cast to DistributedFileSystem in setup() method.

Unrelated to this patch, since you are in that part of the code:

1. Since you are in that part of the code can you remove unnecessary exception declared as thrown from BlockManager#createBlockTokenSecretManager.
2. BlockManager#chooseTarget javadoc has incorrect @see to BlockPlacementPolicy method
3. BlockManager#queueReportedBlock has unnecessary { } around @see block
4. BlockManager#addStorageBlockImmediately() @link should have # preceding addStorageBlock(). Similar change for method countLiveNodes().

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12549572/HDFS-4072.trunk.001.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

org.apache.hadoop.hdfs.TestHFlush

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3357//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3357//console

This message is automatically generated.

Thanks for the comments Suresh. The modified patch uploaded.

Adding pendingReplications.remove(block) to removeBlock means we're just decrementing the replica count by 1 right? Seems like we should add this call to removeStoredBlock instead so we decrement each time a DN reports that the block has been deleted, and we eventually drop the replica count to 0 and removed the entry from PendingReplications. I think updating the test to corrupt two replicas would catch this.

Thanks for the comment Eli! I think you're right: the PendingReplicationBlocks#remove only decrements the pending replication number by 1, it's not removing the whole record. So I guess we only need to remove the whole record about the block from PendingReplicationBlocks here, and we can still do this operation in BlockManager#removeBlock().

Adding pendingReplications.remove(block) to removeBlock means we're just decrementing the replica count by 1 right?

Good catch.

Seems like we should add this call to removeStoredBlock instead

I do not think it works that way. Number of replicas reported as deleted and the number of pending replications are different. I think we should introduce a method to remove the pending replication given a block, instead of decrementing a count.

I think the existing names are poorly named as well. Methods should be renamed:

1. add() to increment()
2. remove() to decrement()
3. New method remove that removes the block from the map

Updated patch.

I think the existing names are poorly named as well. Methods should be renamed:

1.add() to increment()
2.remove() to decrement()
3.New method remove that removes the block from the map

We have discussed the same in HDFS-4022. That JIRA also planning to add the same APIs, as per the comments. So, will just comment there to wait till this JIRA go in then, as patch already updated here.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12549639/HDFS-4072.trunk.003.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

org.apache.hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3361//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3361//console

This message is automatically generated.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12549639/HDFS-4072.trunk.003.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3362//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3362//console

This message is automatically generated.

Cool, I am surprised to see similar discussion in another jira and similar conclusions there too. Thanks Uma for pointing out.

If there is no further comment on this, I will commit this by evening to unblock HDFS-4022.

Looks good to me, perhaps in another change modify the test so it would have failed on the previous version of the patch.

Eli, thanks for the advice. To address your comments, I made two replicas corrupt and checked if the pending replica size is 2 in the new testcase.

+1 pending jenkins. Nice change Jing.

Nit: "mark a block as corrupt" should be "mark a couple blocks as corrupt", we can fix this on commit.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12549721/HDFS-4072.trunk.004.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3364//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3364//console

This message is automatically generated.

Canceling the patch to upload minor nit taken care of before committing the code.

Uploading new patch with a small nit change.

Committed the patch to trunk and branch-2. Thank you Jing.

Integrated in Hadoop-trunk-Commit #2892 (See https://builds.apache.org/job/Hadoop-trunk-Commit/2892/)
HDFS-4072. On file deletion remove corresponding blocks pending replications. Contributed by Jing Zhao. (Revision 1399965)

Result = FAILURE
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1399965
Files :

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReplicationBlocks.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestPendingReplication.java

Jing, would this change be needed for branch-1 as well?

Integrated in Hadoop-Yarn-trunk #8 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/8/)
HDFS-4072. On file deletion remove corresponding blocks pending replications. Contributed by Jing Zhao. (Revision 1399965)

Result = FAILURE
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1399965
Files :

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReplicationBlocks.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestPendingReplication.java

Integrated in Hadoop-Hdfs-trunk #1200 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1200/)
HDFS-4072. On file deletion remove corresponding blocks pending replications. Contributed by Jing Zhao. (Revision 1399965)

Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1399965
Files :

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReplicationBlocks.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestPendingReplication.java

Integrated in Hadoop-Mapreduce-trunk #1230 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1230/)
HDFS-4072. On file deletion remove corresponding blocks pending replications. Contributed by Jing Zhao. (Revision 1399965)

Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1399965
Files :

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReplicationBlocks.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestPendingReplication.java

Branch-1 patch. Will run test-patch for it.

test-patch result for branch-1 patch:
-1 overall.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 3 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
-1 findbugs. The patch appears to introduce 222 new Findbugs (version 2.0.1) warnings.

I committed the patch to branch-1.

Merged to branch-0.23.

Integrated in Hadoop-Hdfs-0.23-Build #546 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/546/)
svn merge -c 1399965 Merging from trunk to branch-0.23 to fix HDFS-4072. (Revision 1453626)

Result = UNSTABLE
kihwal : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1453626
Files :

• /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
• /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/PendingReplicationBlocks.java
• /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestPendingReplication.java