This is an automated email from the ASF dual-hosted git repository.
cliang pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
commit 3e868078020756de58ee714c48ef5a5023ca756b
Author: Chen Liang <cli...@apache.org>
AuthorDate: Tue Jan 28 15:19:47 2020 -0800
Revert "[SBN Read] Slow clients when Observer reads are enabled but there
are no Observers on the cluster. Contributed by Chen Liang."
This reverts commit ff8ff0f7e50cfc147fe383b18b344b46a23220df.
---
.../namenode/ha/ObserverReadProxyProvider.java | 50 +------------------
.../hdfs/server/namenode/ha/TestObserverNode.java | 56 ----------------------
.../namenode/ha/TestObserverReadProxyProvider.java | 6 ---
phantomjsdriver.log | 6 ---
4 files changed, 1 insertion(+), 117 deletions(-)
diff --git
a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ObserverReadProxyProvider.java
b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ObserverReadProxyProvider.java
index 1af0604..e3753c3 100644
---
a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ObserverReadProxyProvider.java
+++
b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ObserverReadProxyProvider.java
@@ -82,12 +82,6 @@ public class ObserverReadProxyProvider<T>
/** Client-side context for syncing with the NameNode server side. */
private final AlignmentContext alignmentContext;
- /** Configuration key for {@link #observerProbeRetryPeriodMs}. */
- static final String OBSERVER_PROBE_RETRY_PERIOD_KEY =
- "dfs.client.failover.observer.probe.retry.period";
- /** Observer probe retry period default to 10 min. */
- static final long OBSERVER_PROBE_RETRY_PERIOD_DEFAULT = 60 * 10 * 1000;
-
/** The inner proxy provider used for active/standby failover. */
private final AbstractNNFailoverProxyProvider<T> failoverProxy;
/** List of all NameNode proxies. */
@@ -147,21 +141,6 @@ public class ObserverReadProxyProvider<T>
private volatile ProxyInfo<T> lastProxy = null;
/**
- * In case there is no Observer node, for every read call, client will try
- * to loop through all Standby nodes and fail eventually. Since there is no
- * guarantee on when Observer node will be enabled. This can be very
- * inefficient.
- * The following value specify the period on how often to retry all Standby.
- */
- private long observerProbeRetryPeriodMs;
-
- /**
- * The previous time where zero observer were found. If there was observer,
- * or it is initialization, this is set to 0.
- */
- private long lastObserverProbeTime;
-
- /**
* By default ObserverReadProxyProvider uses
* {@link ConfiguredFailoverProxyProvider} for failover.
*/
@@ -179,7 +158,6 @@ public class ObserverReadProxyProvider<T>
this.failoverProxy = failoverProxy;
this.alignmentContext = new ClientGSIContext();
factory.setAlignmentContext(alignmentContext);
- this.lastObserverProbeTime = 0;
// Don't bother configuring the number of retries and such on the retry
// policy since it is mainly only used for determining whether or not an
@@ -210,9 +188,6 @@ public class ObserverReadProxyProvider<T>
// The host of the URI is the nameservice ID
AUTO_MSYNC_PERIOD_KEY_PREFIX + "." + uri.getHost(),
AUTO_MSYNC_PERIOD_DEFAULT, TimeUnit.MILLISECONDS);
- observerProbeRetryPeriodMs = conf.getTimeDuration(
- OBSERVER_PROBE_RETRY_PERIOD_KEY,
- OBSERVER_PROBE_RETRY_PERIOD_DEFAULT, TimeUnit.MILLISECONDS);
// TODO : make this configurable or remove this variable
if (wrappedProxy instanceof ClientProtocol) {
@@ -349,27 +324,6 @@ public class ObserverReadProxyProvider<T>
}
/**
- * Check if client need to find an Observer proxy.
- * If current proxy is Active then we should stick to it and postpone probing
- * for Observers for a period of time. When this time expires the client will
- * try to find an Observer again.
- * *
- * @return true if we did not reach the threshold
- * to start looking for Observer, or false otherwise.
- */
- private boolean shouldFindObserver() {
- // lastObserverProbeTime > 0 means we tried, but did not find any
- // Observers yet
- // If lastObserverProbeTime <= 0, previous check found observer, so
- // we should not skip observer read.
- if (lastObserverProbeTime > 0) {
- return Time.monotonicNow() - lastObserverProbeTime
- >= observerProbeRetryPeriodMs;
- }
- return true;
- }
-
- /**
* This will call {@link ClientProtocol#msync()} on the active NameNode
* (via the {@link #failoverProxy}) to update the state of this client, only
* if at least {@link #autoMsyncPeriodMs} ms has elapsed since the last time
@@ -416,7 +370,7 @@ public class ObserverReadProxyProvider<T>
lastProxy = null;
Object retVal;
- if (observerReadEnabled && shouldFindObserver() && isRead(method)) {
+ if (observerReadEnabled && isRead(method)) {
if (!msynced) {
// An msync() must first be performed to ensure that this client is
// up-to-date with the active's state. This will only be done once.
@@ -500,13 +454,11 @@ public class ObserverReadProxyProvider<T>
+ "also found {} standby, {} active, and {} unreachable. "
+ "Falling back to active.", failedObserverCount,
method.getName(), standbyCount, activeCount, unreachableCount);
- lastObserverProbeTime = 0;
} else {
if (LOG.isDebugEnabled()) {
LOG.debug("Read falling back to active without observer read "
+ "fail, is there no observer node running?");
}
- lastObserverProbeTime = Time.monotonicNow();
}
}
diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverNode.java
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverNode.java
index d11093f..2b0ddb5 100644
---
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverNode.java
+++
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverNode.java
@@ -19,7 +19,6 @@ package org.apache.hadoop.hdfs.server.namenode.ha;
import static
org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_STATE_CONTEXT_ENABLED_KEY;
import static
org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter.getServiceState;
-import static
org.apache.hadoop.hdfs.server.namenode.ha.ObserverReadProxyProvider.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -34,9 +33,7 @@ import java.net.URI;
import java.util.ArrayList;
import java.util.List;
-import java.util.concurrent.TimeUnit;
import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.FileUtil;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.FsPermission;
@@ -56,7 +53,6 @@ import org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter;
import org.apache.hadoop.hdfs.server.namenode.TestFsck;
import org.apache.hadoop.hdfs.tools.GetGroups;
import org.apache.hadoop.ipc.ObserverRetryOnActiveException;
-import org.apache.hadoop.util.Time;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
@@ -84,10 +80,6 @@ public class TestObserverNode {
public static void startUpCluster() throws Exception {
conf = new Configuration();
conf.setBoolean(DFS_NAMENODE_STATE_CONTEXT_ENABLED_KEY, true);
- // Set observer probe retry period to 0. Required by the tests that restart
- // Observer and immediately try to read from it.
- conf.setTimeDuration(
- OBSERVER_PROBE_RETRY_PERIOD_KEY, 0, TimeUnit.MILLISECONDS);
qjmhaCluster = HATestUtil.setUpObserverCluster(conf, 1, 0, true);
dfsCluster = qjmhaCluster.getDfsCluster();
}
@@ -421,54 +413,6 @@ public class TestObserverNode {
assertSentTo(0);
}
-
- /**
- * Test that if client connects to Active it does not try to find Observer
- * on next calls during some period of time.
- */
- @Test
- public void testStickyActive() throws Exception {
- Path testFile = new Path(testPath, "testStickyActive");
- Configuration newConf = new Configuration(conf);
- // Observer probe retry period set to 5 sec
- newConf.setLong(OBSERVER_PROBE_RETRY_PERIOD_KEY, 5000);
- // Disable cache, so that a new client actually gets created with new conf.
- newConf.setBoolean("fs.hdfs.impl.disable.cache", true);
- DistributedFileSystem newFs = (DistributedFileSystem)
FileSystem.get(newConf);
- newFs.create(testFile, (short)1).close();
- assertSentTo(0);
- dfsCluster.rollEditLogAndTail(0);
- // No Observers present, should still go to Active
- dfsCluster.transitionToStandby(2);
- assertEquals("NN[2] should be standby", HAServiceState.STANDBY,
- getServiceState(dfsCluster.getNameNode(2)));
- newFs.open(testFile).close();
- assertSentTo(0);
- // Restore Observer
- int newObserver = 1;
- dfsCluster.transitionToObserver(newObserver);
- assertEquals("NN[" + newObserver + "] should be observer",
- HAServiceState.OBSERVER,
- getServiceState(dfsCluster.getNameNode(newObserver)));
- long startTime = Time.monotonicNow();
- try {
- while(Time.monotonicNow() - startTime <= 5000) {
- newFs.open(testFile).close();
- // Client should still talk to Active
- assertSentTo(0);
- Thread.sleep(200);
- }
- } catch(AssertionError ae) {
- if(Time.monotonicNow() - startTime <= 5000) {
- throw ae;
- }
- assertSentTo(newObserver);
- } finally {
- dfsCluster.transitionToStandby(1);
- dfsCluster.transitionToObserver(2);
- }
- }
-
private void assertSentTo(int nnIdx) throws IOException {
assertTrue("Request was not sent to the expected namenode " + nnIdx,
HATestUtil.isSentToAnyOfNameNodes(dfs, dfsCluster, nnIdx));
diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverReadProxyProvider.java
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverReadProxyProvider.java
index e23bb24..13b5774 100644
---
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverReadProxyProvider.java
+++
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestObserverReadProxyProvider.java
@@ -24,7 +24,6 @@ import java.net.URI;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.permission.FsAction;
@@ -44,7 +43,6 @@ import org.mockito.stubbing.Answer;
import static org.apache.hadoop.ha.HAServiceProtocol.HAServiceState;
-import static
org.apache.hadoop.hdfs.server.namenode.ha.ObserverReadProxyProvider.*;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@@ -74,10 +72,6 @@ public class TestObserverReadProxyProvider {
nnURI = URI.create("hdfs://" + ns);
conf = new Configuration();
conf.set(HdfsClientConfigKeys.DFS_NAMESERVICES, ns);
- // Set observer probe retry period to 0. Required by the tests that
- // transition observer back and forth
- conf.setTimeDuration(
- OBSERVER_PROBE_RETRY_PERIOD_KEY, 0, TimeUnit.MILLISECONDS);
}
private void setupProxyProvider(int namenodeCount) throws Exception {
diff --git a/phantomjsdriver.log b/phantomjsdriver.log
deleted file mode 100644
index 6941f15..0000000
--- a/phantomjsdriver.log
+++ /dev/null
@@ -1,6 +0,0 @@
-[INFO - 2020-01-28T23:13:43.723Z] GhostDriver - Main - running on port 35401
-[INFO - 2020-01-28T23:13:44.162Z] Session
[d450b8d0-4223-11ea-891f-8fa5006f053d] - page.settings -
{"XSSAuditingEnabled":false,"javascriptCanCloseWindows":true,"javascriptCanOpenWindows":true,"javascriptEnabled":true,"loadImages":true,"localToRemoteUrlAccessEnabled":false,"userAgent":"Mozilla/5.0
(Macintosh; Intel Mac OS X) AppleWebKit/538.1 (KHTML, like Gecko)
PhantomJS/2.1.1 Safari/538.1","webSecurityEnabled":true}
-[INFO - 2020-01-28T23:13:44.162Z] Session
[d450b8d0-4223-11ea-891f-8fa5006f053d] - page.customHeaders: - {}
-[INFO - 2020-01-28T23:13:44.162Z] Session
[d450b8d0-4223-11ea-891f-8fa5006f053d] - Session.negotiatedCapabilities -
{"browserName":"phantomjs","version":"2.1.1","driverName":"ghostdriver","driverVersion":"1.2.0","platform":"mac-unknown-64bit","javascriptEnabled":true,"takesScreenshot":true,"handlesAlerts":false,"databaseEnabled":false,"locationContextEnabled":false,"applicationCacheEnabled":false,"browserConnectionEnabled":false,"cssSelectorsEnabled":true,"webStorageEnabled":false,"rota
[...]
-[INFO - 2020-01-28T23:13:44.162Z] SessionManagerReqHand -
_postNewSessionCommand - New Session Created:
d450b8d0-4223-11ea-891f-8fa5006f053d
-[INFO - 2020-01-28T23:13:44.990Z] ShutdownReqHand - _handle - About to
shutdown
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
