Author: daryn
Date: Thu Nov 15 20:37:05 2012
New Revision: 1409994
URL: http://svn.apache.org/viewvc?rev=1409994&view=rev
Log:
HDFS-4186. logSync() is called with the write lock held while releasing lease
Modified:
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/namenode/FSNamesystem.java
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java
Modified:
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1409994&r1=1409993&r2=1409994&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
(original)
+++
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
Thu Nov 15 20:37:05 2012
@@ -11,7 +11,7 @@ Release 0.23.6 - UNRELEASED
OPTIMIZATIONS
BUG FIXES
-/
+
Release 0.23.5 - UNRELEASED
INCOMPATIBLE CHANGES
@@ -68,6 +68,9 @@ Release 0.23.5 - UNRELEASED
HDFS-4182. SecondaryNameNode leaks NameCache entries (bobby)
+ HDFS-4186. logSync() is called with the write lock held while releasing
+ lease (Kihwal Lee via daryn)
+
Release 0.23.4
INCOMPATIBLE CHANGES
Modified:
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1409994&r1=1409993&r2=1409994&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
(original)
+++
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Thu Nov 15 20:37:05 2012
@@ -1129,8 +1129,10 @@ public class FSNamesystem implements Nam
createParent, replication, blockSize);
} finally {
writeUnlock();
+ // There might be transactions logged while trying to recover the lease.
+ // They need to be sync'ed even when an exception was thrown.
+ getEditLog().logSync();
}
- getEditLog().logSync();
if (auditLog.isInfoEnabled() && isExternalInvocation()) {
final HdfsFileStatus stat = dir.getFileInfo(src, false);
logAuditEvent(UserGroupInformation.getCurrentUser(),
@@ -1336,6 +1338,9 @@ public class FSNamesystem implements Nam
recoverLeaseInternal(inode, src, holder, clientMachine, true);
} finally {
writeUnlock();
+ // There might be transactions logged while trying to recover the lease.
+ // They need to be sync'ed even when an exception was thrown.
+ getEditLog().logSync();
}
return false;
}
@@ -1437,8 +1442,10 @@ public class FSNamesystem implements Nam
false, blockManager.maxReplication, (long)0);
} finally {
writeUnlock();
+ // There might be transactions logged while trying to recover the lease.
+ // They need to be sync'ed even when an exception was thrown.
+ getEditLog().logSync();
}
- getEditLog().logSync();
if (lb != null) {
if (NameNode.stateChangeLog.isDebugEnabled()) {
NameNode.stateChangeLog.debug("DIR* NameSystem.appendFile: file "
@@ -2171,7 +2178,8 @@ public class FSNamesystem implements Nam
* RecoveryInProgressException if lease recovery is in progress.<br>
* IOException in case of an error.
* @return true if file has been successfully finalized and closed or
- * false if block recovery has been initiated
+ * false if block recovery has been initiated. Since the lease owner
+ * has been changed and logged, caller should call logSync().
*/
boolean internalReleaseLease(Lease lease, String src,
String recoveryLeaseHolder) throws AlreadyBeingCreatedException,
@@ -2303,6 +2311,7 @@ public class FSNamesystem implements Nam
assert hasWriteLock();
if(newHolder == null)
return lease;
+ // The following transaction is not synced. Make sure it's sync'ed later.
logReassignLease(lease.getHolder(), src, newHolder);
return reassignLeaseInternal(lease, src, newHolder, pendingFile);
}
@@ -4225,13 +4234,8 @@ public class FSNamesystem implements Nam
private void logReassignLease(String leaseHolder, String src,
String newHolder) throws IOException {
- writeLock();
- try {
- getEditLog().logReassignLease(leaseHolder, src, newHolder);
- } finally {
- writeUnlock();
- }
- getEditLog().logSync();
+ assert hasWriteLock();
+ getEditLog().logReassignLease(leaseHolder, src, newHolder);
}
/**
Modified:
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java?rev=1409994&r1=1409993&r2=1409994&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
(original)
+++
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
Thu Nov 15 20:37:05 2012
@@ -370,16 +370,20 @@ public class LeaseManager {
/** Check leases periodically. */
public void run() {
for(; fsnamesystem.isRunning(); ) {
+ boolean needSync = false;
fsnamesystem.writeLock();
try {
if (!fsnamesystem.isInSafeMode()) {
- checkLeases();
+ needSync = checkLeases();
}
} finally {
fsnamesystem.writeUnlock();
+ // lease reassignments should to be sync'ed.
+ if (needSync) {
+ fsnamesystem.getEditLog().logSync();
+ }
}
-
try {
Thread.sleep(HdfsServerConstants.NAMENODE_LEASE_RECHECK_INTERVAL);
} catch(InterruptedException ie) {
@@ -391,13 +395,16 @@ public class LeaseManager {
}
}
- /** Check the leases beginning from the oldest. */
- private synchronized void checkLeases() {
+ /** Check the leases beginning from the oldest.
+ * @return true if sync is needed
+ */
+ private synchronized boolean checkLeases() {
+ boolean needSync = false;
assert fsnamesystem.hasWriteLock();
for(; sortedLeases.size() > 0; ) {
final Lease oldest = sortedLeases.first();
if (!oldest.expiredHardLimit()) {
- return;
+ return needSync;
}
LOG.info("Lease " + oldest + " has expired hard limit");
@@ -420,6 +427,10 @@ public class LeaseManager {
LOG.debug("Started block recovery " + p + " lease " + oldest);
}
}
+ // If a lease recovery happened, we need to sync later.
+ if (!needSync && !completed) {
+ needSync = true;
+ }
} catch (IOException e) {
LOG.error("Cannot release the path "+p+" in the lease "+oldest, e);
removing.add(p);
@@ -430,6 +441,7 @@ public class LeaseManager {
removeLease(oldest, p);
}
}
+ return needSync;
}
/** {@inheritDoc} */
Modified:
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java?rev=1409994&r1=1409993&r2=1409994&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java
(original)
+++
hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java
Thu Nov 15 20:37:05 2012
@@ -400,7 +400,7 @@ public class TestEditLog extends TestCas
// Now ask to sync edit from B, which should sync both edits.
doCallLogSync(threadB, editLog);
- assertEquals("logSync from second thread should bump txid up to 2",
+ assertEquals("logSync from second thread should bump txid up to 3",
3, editLog.getSyncTxId());
// Now ask to sync edit from A, which was already batched in - thus
