On July 1, 2016, 10:31 a.m., Robert Nettleton wrote: > > 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? > > Robert Nettleton wrote: > Thanks for catching this. > > The calls to sendLogLevelQueryRequest are removed, since the Ambari UI > will not use these calls in Ambari 2.4. I left the underlying code in the > LoggingRequestHelper's implementation, since I'd like to re-introduce this > call (in a more performant way) in Ambari 2.5. Since that code is also > unit-tested, it seemed to make sense to leave it in to be used in the next > release.
Makes sense; thanks for the explanation. I just wanted to make sure it wasn't something that was by accident. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49474/#review140353 ----------------------------------------------------------- On July 1, 2016, 12:25 p.m., Robert Nettleton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49474/ > ----------------------------------------------------------- > > (Updated July 1, 2016, 12:25 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 > >