Author: daryn
Date: Thu Nov 15 20:33:30 2012
New Revision: 1409988
URL: http://svn.apache.org/viewvc?rev=1409988&view=rev
Log:
HDFS-4186. logSync() is called with the write lock held while releasing lease (Kihwal Lee
via daryn)
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/server/namenode/FSNamesystem.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.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=1409988&r1=1409987&r2=1409988&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt Thu Nov 15 20:33:30 2012
@@ -2026,6 +2026,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/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1409988&r1=1409987&r2=1409988&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
(original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Thu Nov 15 20:33:30 2012
@@ -1720,16 +1720,25 @@ public class FSNamesystem implements Nam
short replication, long blockSize) throws AccessControlException,
SafeModeException, FileAlreadyExistsException, UnresolvedLinkException,
FileNotFoundException, ParentNotDirectoryException, IOException {
+ boolean skipSync = false;
writeLock();
try {
checkOperation(OperationCategory.WRITE);
startFileInternal(src, permissions, holder, clientMachine, flag,
createParent, replication, blockSize);
+ } catch (StandbyException se) {
+ skipSync = true;
+ throw se;
} finally {
writeUnlock();
- }
- getEditLog().logSync();
+ // There might be transactions logged while trying to recover the lease.
+ // They need to be sync'ed even when an exception was thrown.
+ if (!skipSync) {
+ getEditLog().logSync();
+ }
+ }
+
if (auditLog.isInfoEnabled() && isExternalInvocation()) {
final HdfsFileStatus stat = dir.getFileInfo(src, false);
logAuditEvent(UserGroupInformation.getCurrentUser(),
@@ -1910,6 +1919,7 @@ public class FSNamesystem implements Nam
*/
boolean recoverLease(String src, String holder, String clientMachine)
throws IOException {
+ boolean skipSync = false;
writeLock();
try {
checkOperation(OperationCategory.WRITE);
@@ -1931,8 +1941,16 @@ public class FSNamesystem implements Nam
}
recoverLeaseInternal(inode, src, holder, clientMachine, true);
+ } catch (StandbyException se) {
+ skipSync = true;
+ throw se;
} 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.
+ if (!skipSync) {
+ getEditLog().logSync();
+ }
}
return false;
}
@@ -2035,6 +2053,7 @@ public class FSNamesystem implements Nam
throws AccessControlException, SafeModeException,
FileAlreadyExistsException, FileNotFoundException,
ParentNotDirectoryException, IOException {
+ boolean skipSync = false;
if (!supportAppends) {
throw new UnsupportedOperationException(
"Append is not enabled on this NameNode. Use the " +
@@ -2048,10 +2067,17 @@ public class FSNamesystem implements Nam
lb = startFileInternal(src, null, holder, clientMachine,
EnumSet.of(CreateFlag.APPEND),
false, blockManager.maxReplication, 0);
+ } catch (StandbyException se) {
+ skipSync = true;
+ throw se;
} 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.
+ if (!skipSync) {
+ getEditLog().logSync();
+ }
}
- getEditLog().logSync();
if (lb != null) {
if (NameNode.stateChangeLog.isDebugEnabled()) {
NameNode.stateChangeLog.debug("DIR* NameSystem.appendFile: file "
@@ -2983,7 +3009,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,
@@ -3104,6 +3131,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);
}
@@ -5203,13 +5231,8 @@ public class FSNamesystem implements Nam
private void logReassignLease(String leaseHolder, String src,
String newHolder) {
- writeLock();
- try {
- getEditLog().logReassignLease(leaseHolder, src, newHolder);
- } finally {
- writeUnlock();
- }
- getEditLog().logSync();
+ assert hasWriteLock();
+ getEditLog().logReassignLease(leaseHolder, src, newHolder);
}
/**
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java?rev=1409988&r1=1409987&r2=1409988&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
(original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java
Thu Nov 15 20:33:30 2012
@@ -401,17 +401,21 @@ public class LeaseManager {
@Override
public void run() {
for(; shouldRunMonitor && fsnamesystem.isRunning(); ) {
+ boolean needSync = false;
try {
fsnamesystem.writeLockInterruptibly();
try {
if (!fsnamesystem.isInSafeMode()) {
- checkLeases();
+ needSync = checkLeases();
}
} finally {
fsnamesystem.writeUnlock();
+ // lease reassignments should to be sync'ed.
+ if (needSync) {
+ fsnamesystem.getEditLog().logSync();
+ }
}
-
Thread.sleep(HdfsServerConstants.NAMENODE_LEASE_RECHECK_INTERVAL);
} catch(InterruptedException ie) {
if (LOG.isDebugEnabled()) {
@@ -422,13 +426,16 @@ public class LeaseManager {
}
}
- /** Check the leases beginning from the oldest. */
- private synchronized void checkLeases() {
+ /** Check the leases beginning from the oldest.
+ * @return true is 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(oldest + " has expired hard limit");
@@ -451,6 +458,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);
@@ -462,6 +473,7 @@ public class LeaseManager {
removeLease(oldest, p);
}
}
+ return needSync;
}
@Override
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java?rev=1409988&r1=1409987&r2=1409988&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java
(original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEditLog.java
Thu Nov 15 20:33:30 2012
@@ -447,7 +447,7 @@ public class TestEditLog {
// 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
|