You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by nc...@apache.org on 2016/11/07 20:52:17 UTC
[54/55] [abbrv] ambari git commit: AMBARI-17529. Ambari LogSearch
REST Layer should not queue up requests that have already been made.
(rnettleton)
AMBARI-17529. Ambari LogSearch REST Layer should not queue up requests that have already been made. (rnettleton)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/4066b7ef
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/4066b7ef
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/4066b7ef
Branch: refs/heads/branch-dev-patch-upgrade
Commit: 4066b7ef524e8dd7dad03b510e0b4792695c2598
Parents: 8f20af2
Author: Bob Nettleton <rn...@hortonworks.com>
Authored: Mon Nov 7 14:31:17 2016 -0500
Committer: Bob Nettleton <rn...@hortonworks.com>
Committed: Mon Nov 7 14:31:42 2016 -0500
----------------------------------------------------------------------
.../logging/LogSearchDataRetrievalService.java | 124 ++++++++---
.../LogSearchDataRetrievalServiceTest.java | 209 ++++++++++++++++++-
2 files changed, 307 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/4066b7ef/ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java
index 4929747..1c135b2 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java
@@ -19,6 +19,7 @@ package org.apache.ambari.server.controller.logging;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
+import com.google.common.collect.Sets;
import com.google.common.util.concurrent.AbstractService;
import com.google.inject.Inject;
import org.apache.ambari.server.AmbariService;
@@ -76,7 +77,7 @@ public class LogSearchDataRetrievalService extends AbstractService {
private Cache<String, Set<String>> logFileNameCache;
/**
- * a Cache of host+component names to a generated URI that
+ * A Cache of host+component names to a generated URI that
* can be used to access the "tail" of a given log file.
*
* This data is generated by ambari-server, but cached here to
@@ -86,6 +87,16 @@ public class LogSearchDataRetrievalService extends AbstractService {
private Cache<String, String> logFileTailURICache;
/**
+ * A set that maintains the current requests being made,
+ * but not yet completed. This Set will be used to
+ * keep multiple requests from being made for the same
+ * host/component combination.
+ *
+ */
+ private final Set<String> currentRequests = Sets.newConcurrentHashSet();
+
+
+ /**
* Executor instance to be used to run REST queries against
* the LogSearch service.
*/
@@ -130,7 +141,7 @@ public class LogSearchDataRetrievalService extends AbstractService {
* combination, or null if that object does not exist in the cache.
*/
public Set<String> getLogFileNames(String component, String host, String cluster) {
- String key = generateKey(component, host);
+ final String key = generateKey(component, host);
// check cache for data
Set<String> cacheResult =
@@ -140,9 +151,18 @@ public class LogSearchDataRetrievalService extends AbstractService {
LOG.debug("LogFileNames result for key = {} found in cache", key);
return cacheResult;
} else {
- // queue up a thread to make the LogSearch REST request to obtain this information
- LOG.debug("LogFileNames result for key = {} not in cache, queueing up remote request", key);
- startLogSearchFileNameRequest(host, component, cluster);
+ // queue up a thread to create the LogSearch REST request to obtain this information
+ if (currentRequests.contains(key)) {
+ LOG.debug("LogFileNames request has been made for key = {}, but not completed yet", key);
+ } else {
+ LOG.debug("LogFileNames result for key = {} not in cache, queueing up remote request", key);
+ // add request key to queue, to keep multiple copies of the same request from
+ // being submitted
+ currentRequests.add(key);
+ startLogSearchFileNameRequest(host, component, cluster);
+ }
+
+
}
return null;
@@ -182,8 +202,27 @@ public class LogSearchDataRetrievalService extends AbstractService {
this.loggingRequestHelperFactory = loggingRequestHelperFactory;
}
+ /**
+ * This protected method provides a way for unit-tests to insert a
+ * mock executor for simpler unit-testing.
+ *
+ * @param executor an Executor instance
+ */
+ protected void setExecutor(Executor executor) {
+ this.executor = executor;
+ }
+
+ /**
+ * This protected method allows for simpler unit tests.
+ *
+ * @return the Set of current Requests that are not yet completed
+ */
+ protected Set<String> getCurrentRequests() {
+ return currentRequests;
+ }
+
private void startLogSearchFileNameRequest(String host, String component, String cluster) {
- executor.execute(new LogSearchFileNameRequestRunnable(host, component, cluster));
+ executor.execute(new LogSearchFileNameRequestRunnable(host, component, cluster, logFileNameCache, currentRequests));
}
private AmbariManagementController getController() {
@@ -205,7 +244,7 @@ public class LogSearchDataRetrievalService extends AbstractService {
* which can then be used for subsequent requests for the same data.
*
*/
- private class LogSearchFileNameRequestRunnable implements Runnable {
+ static class LogSearchFileNameRequestRunnable implements Runnable {
private final String host;
@@ -213,33 +252,70 @@ public class LogSearchDataRetrievalService extends AbstractService {
private final String cluster;
- private LogSearchFileNameRequestRunnable(String host, String component, String cluster) {
- this.host = host;
+ private final Set<String> currentRequests;
+
+ private final Cache<String, Set<String>> logFileNameCache;
+
+ private LoggingRequestHelperFactory loggingRequestHelperFactory;
+
+ private AmbariManagementController controller;
+
+ LogSearchFileNameRequestRunnable(String host, String component, String cluster, Cache<String, Set<String>> logFileNameCache, Set<String> currentRequests) {
+ this(host, component, cluster, logFileNameCache, currentRequests, new LoggingRequestHelperFactoryImpl(), AmbariServer.getController());
+ }
+
+ LogSearchFileNameRequestRunnable(String host, String component, String cluster, Cache<String, Set<String>> logFileNameCache, Set<String> currentRequests,
+ LoggingRequestHelperFactory loggingRequestHelperFactory, AmbariManagementController controller) {
+ this.host = host;
this.component = component;
this.cluster = cluster;
+ this.logFileNameCache = logFileNameCache;
+ this.currentRequests = currentRequests;
+ this.loggingRequestHelperFactory = loggingRequestHelperFactory;
+ this.controller = controller;
}
@Override
public void run() {
LOG.debug("LogSearchFileNameRequestRunnable: starting...");
- LoggingRequestHelper helper =
- new LoggingRequestHelperFactoryImpl().getHelper(getController(), cluster);
-
- if (helper != null) {
- // make request to LogSearch service
- Set<String> logFileNamesResult =
- helper.sendGetLogFileNamesRequest(component, host);
-
- // update the cache if result is available
- if (logFileNamesResult != null) {
- LOG.debug("LogSearchFileNameRequestRunnable: request was successful, updating cache");
- logFileNameCache.put(generateKey(component, host), logFileNamesResult);
+ try {
+ LoggingRequestHelper helper =
+ loggingRequestHelperFactory.getHelper(controller, cluster);
+
+ if (helper != null) {
+ // make request to LogSearch service
+ Set<String> logFileNamesResult =
+ helper.sendGetLogFileNamesRequest(component, host);
+
+ // update the cache if result is available
+ if (logFileNamesResult != null) {
+ LOG.debug("LogSearchFileNameRequestRunnable: request was successful, updating cache");
+ final String key = generateKey(component, host);
+ // update cache with returned result
+ logFileNameCache.put(key, logFileNamesResult);
+ } else {
+ LOG.debug("LogSearchFileNameRequestRunnable: remote request was not successful");
+ }
} else {
- LOG.debug("LogSearchFileNameRequestRunnable: remote request was not successful");
+ LOG.debug("LogSearchFileNameRequestRunnable: request helper was null. This may mean that LogSearch is not available, or could be a potential connection problem.");
}
- } else {
- LOG.debug("LogSearchFileNameRequestRunnable: request helper was null. This may mean that LogSearch is not available, or could be a potential connection problem.");
+ } finally {
+ // since request has completed (either successfully or not),
+ // remove this host/component key from the current requests
+ currentRequests.remove(generateKey(component, host));
}
}
+
+ protected void setLoggingRequestHelperFactory(LoggingRequestHelperFactory loggingRequestHelperFactory) {
+ this.loggingRequestHelperFactory = loggingRequestHelperFactory;
+ }
+
+ protected void setAmbariManagementController(AmbariManagementController controller) {
+ this.controller = controller;
+ }
+
+
}
+
+
}
http://git-wip-us.apache.org/repos/asf/ambari/blob/4066b7ef/ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalServiceTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalServiceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalServiceTest.java
index 97c59f8..b58350b 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalServiceTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalServiceTest.java
@@ -17,12 +17,20 @@
*/
package org.apache.ambari.server.controller.logging;
+import com.google.common.cache.Cache;
+import org.apache.ambari.server.controller.AmbariManagementController;
import org.easymock.EasyMockSupport;
import org.junit.Test;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.Executor;
+
import static org.easymock.EasyMock.expect;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertEquals;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.isA;
+import static org.junit.Assert.*;
/**
* This test verifies the basic behavior of the
@@ -102,4 +110,201 @@ public class LogSearchDataRetrievalServiceTest {
}
+ @Test
+ public void testGetLogFileNamesDefault() throws Exception {
+ final String expectedHostName = "c6401.ambari.apache.org";
+ final String expectedComponentName = "DATANODE";
+ final String expectedClusterName = "clusterone";
+
+ EasyMockSupport mockSupport = new EasyMockSupport();
+
+ LoggingRequestHelperFactory helperFactoryMock =
+ mockSupport.createMock(LoggingRequestHelperFactory.class);
+
+ Executor executorMock =
+ mockSupport.createMock(Executor.class);
+
+ // expect the executor to be called to execute the LogSearch request
+ executorMock.execute(isA(LogSearchDataRetrievalService.LogSearchFileNameRequestRunnable.class));
+ // executor should only be called once
+ expectLastCall().once();
+
+ mockSupport.replayAll();
+
+ LogSearchDataRetrievalService retrievalService =
+ new LogSearchDataRetrievalService();
+ retrievalService.setLoggingRequestHelperFactory(helperFactoryMock);
+ // call the initialization routine called by the Google framework
+ retrievalService.doStart();
+ retrievalService.setExecutor(executorMock);
+
+
+ assertEquals("Default request set should be empty",
+ 0, retrievalService.getCurrentRequests().size());
+
+ Set<String> resultSet =
+ retrievalService.getLogFileNames(expectedComponentName, expectedHostName, expectedClusterName);
+
+ assertNull("Inital query on the retrieval service should be null, since cache is empty by default",
+ resultSet);
+ assertEquals("Incorrect number of entries in the current request set",
+ 1, retrievalService.getCurrentRequests().size());
+ assertTrue("Incorrect HostComponent set on request set",
+ retrievalService.getCurrentRequests().contains(expectedComponentName + "+" + expectedHostName));
+
+ mockSupport.verifyAll();
+ }
+
+ @Test
+ public void testGetLogFileNamesIgnoreMultipleRequestsForSameHostComponent() throws Exception {
+ final String expectedHostName = "c6401.ambari.apache.org";
+ final String expectedComponentName = "DATANODE";
+ final String expectedClusterName = "clusterone";
+
+ EasyMockSupport mockSupport = new EasyMockSupport();
+
+ LoggingRequestHelperFactory helperFactoryMock =
+ mockSupport.createMock(LoggingRequestHelperFactory.class);
+
+ Executor executorMock =
+ mockSupport.createMock(Executor.class);
+
+ mockSupport.replayAll();
+
+ LogSearchDataRetrievalService retrievalService =
+ new LogSearchDataRetrievalService();
+ retrievalService.setLoggingRequestHelperFactory(helperFactoryMock);
+ // call the initialization routine called by the Google framework
+ retrievalService.doStart();
+ // there should be no expectations set on this mock
+ retrievalService.setExecutor(executorMock);
+
+ // set the current requests to include this expected HostComponent
+ // this simulates the case where a request is ongoing for this HostComponent,
+ // but is not yet completed.
+ retrievalService.getCurrentRequests().add(expectedComponentName + "+" + expectedHostName);
+
+ Set<String> resultSet =
+ retrievalService.getLogFileNames(expectedComponentName, expectedHostName, expectedClusterName);
+
+ assertNull("Inital query on the retrieval service should be null, since cache is empty by default",
+ resultSet);
+
+ mockSupport.verifyAll();
+ }
+
+ @Test
+ public void testRunnableWithSuccessfulCall() throws Exception {
+ final String expectedHostName = "c6401.ambari.apache.org";
+ final String expectedComponentName = "DATANODE";
+ final String expectedClusterName = "clusterone";
+ final String expectedComponentAndHostName = expectedComponentName + "+" + expectedHostName;
+
+ EasyMockSupport mockSupport = new EasyMockSupport();
+
+ LoggingRequestHelperFactory helperFactoryMock =
+ mockSupport.createMock(LoggingRequestHelperFactory.class);
+ AmbariManagementController controllerMock =
+ mockSupport.createMock(AmbariManagementController.class);
+ LoggingRequestHelper helperMock =
+ mockSupport.createMock(LoggingRequestHelper.class);
+
+ Cache cacheMock =
+ mockSupport.createMock(Cache.class);
+ Set currentRequestsMock =
+ mockSupport.createMock(Set.class);
+
+ expect(helperFactoryMock.getHelper(controllerMock, expectedClusterName)).andReturn(helperMock);
+ expect(helperMock.sendGetLogFileNamesRequest(expectedComponentName, expectedHostName)).andReturn(Collections.singleton("/this/is/just/a/test/directory"));
+ // expect that the results will be placed in the cache
+ cacheMock.put(expectedComponentAndHostName, Collections.singleton("/this/is/just/a/test/directory"));
+ // expect that the completed request is removed from the current request set
+ expect(currentRequestsMock.remove(expectedComponentAndHostName)).andReturn(true).once();
+
+ mockSupport.replayAll();
+
+ LogSearchDataRetrievalService.LogSearchFileNameRequestRunnable loggingRunnable =
+ new LogSearchDataRetrievalService.LogSearchFileNameRequestRunnable(expectedHostName, expectedComponentName, expectedClusterName,
+ cacheMock, currentRequestsMock, helperFactoryMock, controllerMock);
+ loggingRunnable.run();
+
+ mockSupport.verifyAll();
+
+ }
+
+ @Test
+ public void testRunnableWithFailedCallNullHelper() throws Exception {
+ final String expectedHostName = "c6401.ambari.apache.org";
+ final String expectedComponentName = "DATANODE";
+ final String expectedClusterName = "clusterone";
+ final String expectedComponentAndHostName = expectedComponentName + "+" + expectedHostName;
+
+ EasyMockSupport mockSupport = new EasyMockSupport();
+
+ LoggingRequestHelperFactory helperFactoryMock =
+ mockSupport.createMock(LoggingRequestHelperFactory.class);
+ AmbariManagementController controllerMock =
+ mockSupport.createMock(AmbariManagementController.class);
+
+ Cache cacheMock =
+ mockSupport.createMock(Cache.class);
+ Set currentRequestsMock =
+ mockSupport.createMock(Set.class);
+
+ // return null to simulate an error during helper instance creation
+ expect(helperFactoryMock.getHelper(controllerMock, expectedClusterName)).andReturn(null);
+ // expect that the completed request is removed from the current request set,
+ // even in the event of a failure to obtain the LogSearch data
+ expect(currentRequestsMock.remove(expectedComponentAndHostName)).andReturn(true).once();
+
+ mockSupport.replayAll();
+
+ LogSearchDataRetrievalService.LogSearchFileNameRequestRunnable loggingRunnable =
+ new LogSearchDataRetrievalService.LogSearchFileNameRequestRunnable(expectedHostName, expectedComponentName, expectedClusterName,
+ cacheMock, currentRequestsMock, helperFactoryMock, controllerMock);
+ loggingRunnable.run();
+
+ mockSupport.verifyAll();
+
+ }
+
+ @Test
+ public void testRunnableWithFailedCallNullResult() throws Exception {
+ final String expectedHostName = "c6401.ambari.apache.org";
+ final String expectedComponentName = "DATANODE";
+ final String expectedClusterName = "clusterone";
+ final String expectedComponentAndHostName = expectedComponentName + "+" + expectedHostName;
+
+ EasyMockSupport mockSupport = new EasyMockSupport();
+
+ LoggingRequestHelperFactory helperFactoryMock =
+ mockSupport.createMock(LoggingRequestHelperFactory.class);
+ AmbariManagementController controllerMock =
+ mockSupport.createMock(AmbariManagementController.class);
+ LoggingRequestHelper helperMock =
+ mockSupport.createMock(LoggingRequestHelper.class);
+
+ Cache cacheMock =
+ mockSupport.createMock(Cache.class);
+ Set currentRequestsMock =
+ mockSupport.createMock(Set.class);
+
+ expect(helperFactoryMock.getHelper(controllerMock, expectedClusterName)).andReturn(helperMock);
+ // return null to simulate an error occurring during the LogSearch data request
+ expect(helperMock.sendGetLogFileNamesRequest(expectedComponentName, expectedHostName)).andReturn(null);
+ // expect that the completed request is removed from the current request set,
+ // even in the event of a failure to obtain the LogSearch data
+ expect(currentRequestsMock.remove(expectedComponentAndHostName)).andReturn(true).once();
+
+ mockSupport.replayAll();
+
+ LogSearchDataRetrievalService.LogSearchFileNameRequestRunnable loggingRunnable =
+ new LogSearchDataRetrievalService.LogSearchFileNameRequestRunnable(expectedHostName, expectedComponentName, expectedClusterName,
+ cacheMock, currentRequestsMock, helperFactoryMock, controllerMock);
+ loggingRunnable.run();
+
+ mockSupport.verifyAll();
+
+ }
+
}