----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49474/#review140353 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java (line 36) <https://reviews.apache.org/r/49474/#comment205738> JavaDoc. ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java (lines 66 - 67) <https://reviews.apache.org/r/49474/#comment205739> Remove? ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java (line 79) <https://reviews.apache.org/r/49474/#comment205741> Based on what we've seen, the requests to the LOGSEARCH endpoint don't return until the connection times out (which could be never depending on what's configured). This will then cause this Executor to fill it's queue with a backlog of requests. Instead, maybe it would be better to: - Only enqueue if the request isn't already enqueued - Use a bounded executor and rejection policy ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java (line 115) <https://reviews.apache.org/r/49474/#comment205740> Use {} to prevent concatenation on DEBUG statements. ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java (lines 149 - 151) <https://reviews.apache.org/r/49474/#comment205742> Maybe only enqueue requests if the same request isn't on the queue. ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java (line 163) <https://reviews.apache.org/r/49474/#comment205744> JavaDoc. ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java (lines 78 - 79) <https://reviews.apache.org/r/49474/#comment205743> This is interesting; how would this service not be available if it's injected? I'm a bit confused by the changes made here, namely that `sendLogLevelQueryRequest` was replaced with `sendGetLogFileNamesRequest`. Is `sendLogLevelQueryRequest` no longer used? If so, it can be removed, right? - Jonathan Hurley On June 30, 2016, 5:11 p.m., Robert Nettleton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49474/ > ----------------------------------------------------------- > > (Updated June 30, 2016, 5:11 p.m.) > > > Review request for Ambari, Jonathan Hurley, Mahadev Konar, Oliver Szabo, and > Sumit Mohanty. > > > Bugs: AMBARI-17510 > https://issues.apache.org/jira/browse/AMBARI-17510 > > > Repository: ambari > > > Description > ------- > > This patch implements some updates to the Ambari LogSearch integration layer > in order to resolve AMBARI-17510. > > The initial implementation of the LoggingSearchPropertyProvider directly made > calls to the LogSearch Server's REST endpoint, in order to obtain LogSearch > metadata, and to attach this information to the Ambari HostComponent REST > resource. Recent testing has shown that there are some deployment scenarios > (larger clusters, rolling upgrade) that demonstrate a performance problem in > the LogSearch integration code. > > This patch addresses this problem by implementing the following: > > 1. A new, injectable service > (org.apache.ambari.server.controller.logging.LogSearchDataRetrievalService) > has been introduced into the LoggingSearchPropertyProvider. The property > provider now consults this service to obtain the required LogSearch data. > This new service is based on Jonathan Hurley's work in > org.apache.ambari.server.state.services.MetricsRetrievalService. > 2. The LogSearchDataRetrievalService utilizes an internal cache to maintain a > map of the LogSearch data, which typically will not change. If the cache > does not contain this information for a given HostComponent, the retrieval > service will make the required REST call to the LogSearch service to obtain > this data, and then add this to the cache. The remote REST call now happens > on a separate thread from the Ambari REST request handler, which removes the > possibility of causing degradation on the Ambari REST endpoing. > 3. The LoggingRequestHelperImpl class has been slightly updated, in order to > add 5 second connect/read timeouts on the LogSearch requests. This should > prevent any threads from hanging infinitely if the LogSearch server is slow > to respond. In a future patch, we might want to make these timeouts > configurable. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java > 947a9f4 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java > fe7e757 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java > 5ac66d8 > > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java > d8c71e2 > > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java > ff7e7f5 > > ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProviderTest.java > 593f660 > > Diff: https://reviews.apache.org/r/49474/diff/ > > > Testing > ------- > > 1. Ran the ambari-server "mvn clean test" suite, and all Java and Python unit > tests passed with this change applied. > 2. Deployed a 3-node cluster with HDFS, Yarn, and LogSearch selected with > this patch applied. Verified that the cluster deployed successfully, and that > the "host_components" REST resource (the resource associated with this > performance problem) returns properly. Ran 6-7 separate REST clients making > concurrent requests on the "host_componets" resource in Ambari for over 40 > minutes, and verified that there is no noticeable slowdown. Verified that > ambari-server remains responsive after this test. Also verified via > DEBUG-level logging that the LogSearch remote REST requests are indeed > happening on a separate thread now. > 3. Deployed a 3-node cluster with HDFS and Yarn selected (LogSearch not > selected) with this patch applied. Verified that the cluster deploys > properly, and that the Ambari REST "host_components" resource returns > properly when LogSearch is not included in the cluster. > > > Thanks, > > Robert Nettleton > >
