Return-Path: X-Original-To: apmail-hadoop-hdfs-commits-archive@minotaur.apache.org Delivered-To: apmail-hadoop-hdfs-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 6D86BDBF8 for ; Thu, 15 Nov 2012 20:37:29 +0000 (UTC) Received: (qmail 28285 invoked by uid 500); 15 Nov 2012 20:37:29 -0000 Delivered-To: apmail-hadoop-hdfs-commits-archive@hadoop.apache.org Received: (qmail 28254 invoked by uid 500); 15 Nov 2012 20:37:29 -0000 Mailing-List: contact hdfs-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: hdfs-dev@hadoop.apache.org Delivered-To: mailing list hdfs-commits@hadoop.apache.org Received: (qmail 28246 invoked by uid 99); 15 Nov 2012 20:37:29 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 15 Nov 2012 20:37:29 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 15 Nov 2012 20:37:27 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 9F66C238896F; Thu, 15 Nov 2012 20:37:07 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1409994 - in /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs: ./ src/main/java/org/apache/hadoop/hdfs/server/namenode/ src/test/java/org/apache/hadoop/hdfs/server/namenode/ Date: Thu, 15 Nov 2012 20:37:06 -0000 To: hdfs-commits@hadoop.apache.org From: daryn@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20121115203707.9F66C238896F@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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.
* 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