You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ha...@apache.org on 2018/02/16 22:00:32 UTC
[10/21] hadoop git commit: HDFS-13112. Token expiration edits may
cause log corruption or deadlock. Contributed by Daryn Sharp.
HDFS-13112. Token expiration edits may cause log corruption or deadlock. Contributed by Daryn Sharp.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/47473952
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/47473952
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/47473952
Branch: refs/heads/HDFS-12996
Commit: 47473952e56b0380147d42f4110ad03c2276c961
Parents: a53d62a
Author: Kihwal Lee <ki...@apache.org>
Authored: Thu Feb 15 15:32:42 2018 -0600
Committer: Kihwal Lee <ki...@apache.org>
Committed: Thu Feb 15 15:32:42 2018 -0600
----------------------------------------------------------------------
.../DelegationTokenSecretManager.java | 53 ++++++++++++++------
.../hdfs/server/namenode/FSNamesystem.java | 17 ++++---
.../hdfs/server/namenode/FSNamesystemLock.java | 7 +++
.../org/apache/hadoop/hdfs/util/RwLock.java | 5 +-
.../namenode/TestSecurityTokenEditLog.java | 24 ++++++++-
5 files changed, 83 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/47473952/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenSecretManager.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenSecretManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenSecretManager.java
index b7f89a8..3547c96 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenSecretManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/delegation/DelegationTokenSecretManager.java
@@ -21,7 +21,6 @@ package org.apache.hadoop.hdfs.security.token.delegation;
import java.io.DataInput;
import java.io.DataOutputStream;
import java.io.IOException;
-import java.io.InterruptedIOException;
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Iterator;
@@ -366,34 +365,58 @@ public class DelegationTokenSecretManager
@Override //AbstractDelegationTokenManager
protected void logUpdateMasterKey(DelegationKey key)
throws IOException {
- synchronized (noInterruptsLock) {
+ try {
// The edit logging code will fail catastrophically if it
// is interrupted during a logSync, since the interrupt
// closes the edit log files. Doing this inside the
- // above lock and then checking interruption status
- // prevents this bug.
- if (Thread.interrupted()) {
- throw new InterruptedIOException(
- "Interrupted before updating master key");
+ // fsn lock will prevent being interrupted when stopping
+ // the secret manager.
+ namesystem.readLockInterruptibly();
+ try {
+ // this monitor isn't necessary if stopped while holding write lock
+ // but for safety, guard against a stop with read lock.
+ synchronized (noInterruptsLock) {
+ if (Thread.currentThread().isInterrupted()) {
+ return; // leave flag set so secret monitor exits.
+ }
+ namesystem.logUpdateMasterKey(key);
+ }
+ } finally {
+ namesystem.readUnlock();
}
- namesystem.logUpdateMasterKey(key);
+ } catch (InterruptedException ie) {
+ // AbstractDelegationTokenManager may crash if an exception is thrown.
+ // The interrupt flag will be detected when it attempts to sleep.
+ Thread.currentThread().interrupt();
}
}
@Override //AbstractDelegationTokenManager
protected void logExpireToken(final DelegationTokenIdentifier dtId)
throws IOException {
- synchronized (noInterruptsLock) {
+ try {
// The edit logging code will fail catastrophically if it
// is interrupted during a logSync, since the interrupt
// closes the edit log files. Doing this inside the
- // above lock and then checking interruption status
- // prevents this bug.
- if (Thread.interrupted()) {
- throw new InterruptedIOException(
- "Interrupted before expiring delegation token");
+ // fsn lock will prevent being interrupted when stopping
+ // the secret manager.
+ namesystem.readLockInterruptibly();
+ try {
+ // this monitor isn't necessary if stopped while holding write lock
+ // but for safety, guard against a stop with read lock.
+ synchronized (noInterruptsLock) {
+ if (Thread.currentThread().isInterrupted()) {
+ return; // leave flag set so secret monitor exits.
+ }
+ namesystem.logExpireDelegationToken(dtId);
+ }
+ } finally {
+ namesystem.readUnlock();
}
- namesystem.logExpireDelegationToken(dtId);
+ } catch (InterruptedException ie) {
+ // AbstractDelegationTokenManager may crash if an exception is thrown.
+ // The interrupt flag will be detected when it attempts to sleep.
+ Thread.currentThread().interrupt();
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/47473952/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
index 6c27d7e..b0973a9 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
@@ -1580,6 +1580,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
this.fsLock.readLock();
}
@Override
+ public void readLockInterruptibly() throws InterruptedException {
+ this.fsLock.readLockInterruptibly();
+ }
+ @Override
public void readUnlock() {
this.fsLock.readUnlock();
}
@@ -5675,9 +5679,9 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
assert !isInSafeMode() :
"this should never be called while in safemode, since we stop " +
"the DT manager before entering safemode!";
- // No need to hold FSN lock since we don't access any internal
- // structures, and this is stopped before the FSN shuts itself
- // down, etc.
+ // edit log rolling is not thread-safe and must be protected by the
+ // fsn lock. not updating namespace so read lock is sufficient.
+ assert hasReadLock();
getEditLog().logUpdateMasterKey(key);
getEditLog().logSync();
}
@@ -5691,9 +5695,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
assert !isInSafeMode() :
"this should never be called while in safemode, since we stop " +
"the DT manager before entering safemode!";
- // No need to hold FSN lock since we don't access any internal
- // structures, and this is stopped before the FSN shuts itself
- // down, etc.
+ // edit log rolling is not thread-safe and must be protected by the
+ // fsn lock. not updating namespace so read lock is sufficient.
+ assert hasReadLock();
+ // do not logSync so expiration edits are batched
getEditLog().logCancelDelegationToken(id);
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/47473952/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
index 32c7efa..900f8a2 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystemLock.java
@@ -145,6 +145,13 @@ class FSNamesystemLock {
}
}
+ public void readLockInterruptibly() throws InterruptedException {
+ coarseLock.readLock().lockInterruptibly();
+ if (coarseLock.getReadHoldCount() == 1) {
+ readLockHeldTimeStampNanos.set(timer.monotonicNowNanos());
+ }
+ }
+
public void readUnlock() {
readUnlock(OP_NAME_OTHER);
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/47473952/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/RwLock.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/RwLock.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/RwLock.java
index e36f0f7..deaeaa4 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/RwLock.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/RwLock.java
@@ -21,7 +21,10 @@ package org.apache.hadoop.hdfs.util;
public interface RwLock {
/** Acquire read lock. */
public void readLock();
-
+
+ /** Acquire read lock, unless interrupted while waiting */
+ void readLockInterruptibly() throws InterruptedException;
+
/** Release read lock. */
public void readUnlock();
http://git-wip-us.apache.org/repos/asf/hadoop/blob/47473952/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSecurityTokenEditLog.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSecurityTokenEditLog.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSecurityTokenEditLog.java
index 5aa19bb..c43c909 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSecurityTokenEditLog.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSecurityTokenEditLog.java
@@ -24,6 +24,7 @@ import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.util.Iterator;
+import java.util.concurrent.atomic.AtomicReference;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
@@ -37,7 +38,11 @@ import org.apache.hadoop.hdfs.server.namenode.NNStorage.NameNodeDirType;
import org.apache.hadoop.io.Text;
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.security.token.Token;
+import org.junit.Assert;
import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
import static org.mockito.Mockito.*;
/**
@@ -180,8 +185,25 @@ public class TestSecurityTokenEditLog {
Text renewer = new Text(UserGroupInformation.getCurrentUser().getUserName());
FSImage fsImage = mock(FSImage.class);
FSEditLog log = mock(FSEditLog.class);
- doReturn(log).when(fsImage).getEditLog();
+ doReturn(log).when(fsImage).getEditLog();
+ // verify that the namesystem read lock is held while logging token
+ // expirations. the namesystem is not updated, so write lock is not
+ // necessary, but the lock is required because edit log rolling is not
+ // thread-safe.
+ final AtomicReference<FSNamesystem> fsnRef = new AtomicReference<>();
+ doAnswer(
+ new Answer<Void>() {
+ @Override
+ public Void answer(InvocationOnMock invocation) throws Throwable {
+ // fsn claims read lock if either read or write locked.
+ Assert.assertTrue(fsnRef.get().hasReadLock());
+ Assert.assertFalse(fsnRef.get().hasWriteLock());
+ return null;
+ }
+ }
+ ).when(log).logCancelDelegationToken(any(DelegationTokenIdentifier.class));
FSNamesystem fsn = new FSNamesystem(conf, fsImage);
+ fsnRef.set(fsn);
DelegationTokenSecretManager dtsm = fsn.getDelegationTokenSecretManager();
try {
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org