You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-commits@hadoop.apache.org by to...@apache.org on 2012/01/20 08:26:20 UTC
svn commit: r1233794 - in
/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs: ./
src/main/java/org/apache/hadoop/hdfs/ src/test/java/org/apache/hadoop/hdfs/
Author: todd
Date: Fri Jan 20 07:26:19 2012
New Revision: 1233794
URL: http://svn.apache.org/viewvc?rev=1233794&view=rev
Log:
HDFS-2810. Leases not getting renewed properly by clients. Contributed by Todd Lipcon.
Modified:
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/DFSClient.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1233794&r1=1233793&r2=1233794&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt Fri Jan 20 07:26:19 2012
@@ -338,6 +338,8 @@ Release 0.23.1 - UNRELEASED
HDFS-2790. FSNamesystem.setTimes throws exception with wrong
configuration name in the message. (Arpit Gupta via eli)
+ HDFS-2810. Leases not getting renewed properly by clients (todd)
+
Release 0.23.0 - 2011-11-01
INCOMPATIBLE CHANGES
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java?rev=1233794&r1=1233793&r2=1233794&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java Fri Jan 20 07:26:19 2012
@@ -373,11 +373,17 @@ public class DFSClient implements java.i
return clientRunning;
}
- /** Renew leases */
- void renewLease() throws IOException {
+ /**
+ * Renew leases.
+ * @return true if lease was renewed. May return false if this
+ * client has been closed or has no files open.
+ **/
+ boolean renewLease() throws IOException {
if (clientRunning && !isFilesBeingWrittenEmpty()) {
namenode.renewLease(clientName);
+ return true;
}
+ return false;
}
/**
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java?rev=1233794&r1=1233793&r2=1233794&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java Fri Jan 20 07:26:19 2012
@@ -67,7 +67,7 @@ import org.apache.hadoop.util.StringUtil
* </p>
*/
class LeaseRenewer {
- private static final Log LOG = LogFactory.getLog(LeaseRenewer.class);
+ static final Log LOG = LogFactory.getLog(LeaseRenewer.class);
static final long LEASE_RENEWER_GRACE_DEFAULT = 60*1000L;
static final long LEASE_RENEWER_SLEEP_DEFAULT = 1000L;
@@ -407,7 +407,13 @@ class LeaseRenewer {
final DFSClient c = copies.get(i);
//skip if current client name is the same as the previous name.
if (!c.getClientName().equals(previousName)) {
- c.renewLease();
+ if (!c.renewLease()) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Did not renew lease for client " +
+ c);
+ }
+ continue;
+ }
previousName = c.getClientName();
if (LOG.isDebugEnabled()) {
LOG.debug("Lease renewed for client " + previousName);
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java?rev=1233794&r1=1233793&r2=1233794&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java Fri Jan 20 07:26:19 2012
@@ -17,11 +17,14 @@
*/
package org.apache.hadoop.hdfs;
+import static org.junit.Assert.*;
+
import java.io.IOException;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.test.GenericTestUtils;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -29,6 +32,8 @@ import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
+import com.google.common.base.Supplier;
+
public class TestLeaseRenewer {
private String FAKE_AUTHORITY="hdfs://nn1/";
private UserGroupInformation FAKE_UGI_A =
@@ -46,19 +51,24 @@ public class TestLeaseRenewer {
@Before
public void setupMocksAndRenewer() throws IOException {
- MOCK_DFSCLIENT = Mockito.mock(DFSClient.class);
- Mockito.doReturn(true)
- .when(MOCK_DFSCLIENT).isClientRunning();
- Mockito.doReturn((int)FAST_GRACE_PERIOD)
- .when(MOCK_DFSCLIENT).getHdfsTimeout();
- Mockito.doReturn("myclient")
- .when(MOCK_DFSCLIENT).getClientName();
+ MOCK_DFSCLIENT = createMockClient();
renewer = LeaseRenewer.getInstance(
FAKE_AUTHORITY, FAKE_UGI_A, MOCK_DFSCLIENT);
renewer.setGraceSleepPeriod(FAST_GRACE_PERIOD);
}
+ private DFSClient createMockClient() {
+ DFSClient mock = Mockito.mock(DFSClient.class);
+ Mockito.doReturn(true)
+ .when(mock).isClientRunning();
+ Mockito.doReturn((int)FAST_GRACE_PERIOD)
+ .when(mock).getHdfsTimeout();
+ Mockito.doReturn("myclient")
+ .when(mock).getClientName();
+ return mock;
+ }
+
@Test
public void testInstanceSharing() throws IOException {
// Two lease renewers with the same UGI should return
@@ -93,11 +103,11 @@ public class TestLeaseRenewer {
public void testRenewal() throws Exception {
// Keep track of how many times the lease gets renewed
final AtomicInteger leaseRenewalCount = new AtomicInteger();
- Mockito.doAnswer(new Answer<Void>() {
+ Mockito.doAnswer(new Answer<Boolean>() {
@Override
- public Void answer(InvocationOnMock invocation) throws Throwable {
+ public Boolean answer(InvocationOnMock invocation) throws Throwable {
leaseRenewalCount.incrementAndGet();
- return null;
+ return true;
}
}).when(MOCK_DFSCLIENT).renewLease();
@@ -120,6 +130,57 @@ public class TestLeaseRenewer {
renewer.closeFile(filePath, MOCK_DFSCLIENT);
}
+ /**
+ * Regression test for HDFS-2810. In this bug, the LeaseRenewer has handles
+ * to several DFSClients with the same name, the first of which has no files
+ * open. Previously, this was causing the lease to not get renewed.
+ */
+ @Test
+ public void testManyDfsClientsWhereSomeNotOpen() throws Exception {
+ // First DFSClient has no files open so doesn't renew leases.
+ final DFSClient mockClient1 = createMockClient();
+ Mockito.doReturn(false).when(mockClient1).renewLease();
+ assertSame(renewer, LeaseRenewer.getInstance(
+ FAKE_AUTHORITY, FAKE_UGI_A, mockClient1));
+
+ // Set up a file so that we start renewing our lease.
+ DFSOutputStream mockStream1 = Mockito.mock(DFSOutputStream.class);
+ String filePath = "/foo";
+ renewer.put(filePath, mockStream1, mockClient1);
+
+ // Second DFSClient does renew lease
+ final DFSClient mockClient2 = createMockClient();
+ Mockito.doReturn(true).when(mockClient2).renewLease();
+ assertSame(renewer, LeaseRenewer.getInstance(
+ FAKE_AUTHORITY, FAKE_UGI_A, mockClient2));
+
+ // Set up a file so that we start renewing our lease.
+ DFSOutputStream mockStream2 = Mockito.mock(DFSOutputStream.class);
+ renewer.put(filePath, mockStream2, mockClient2);
+
+
+ // Wait for lease to get renewed
+ GenericTestUtils.waitFor(new Supplier<Boolean>() {
+ @Override
+ public Boolean get() {
+ try {
+ Mockito.verify(mockClient1, Mockito.atLeastOnce()).renewLease();
+ Mockito.verify(mockClient2, Mockito.atLeastOnce()).renewLease();
+ return true;
+ } catch (AssertionError err) {
+ LeaseRenewer.LOG.warn("Not yet satisfied", err);
+ return false;
+ } catch (IOException e) {
+ // should not throw!
+ throw new RuntimeException(e);
+ }
+ }
+ }, 100, 10000);
+
+ renewer.closeFile(filePath, mockClient1);
+ renewer.closeFile(filePath, mockClient2);
+ }
+
@Test
public void testThreadName() throws Exception {
DFSOutputStream mockStream = Mockito.mock(DFSOutputStream.class);