----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27802/#review60602 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/resources/HostComponentLogResourceDefinition.java <https://reviews.apache.org/r/27802/#comment101950> You might want to make these names more unique since they are for host components. ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101951> These should not be on individual lines since they easily fit next to each other. ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101952> formatting. ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101953> formatting ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101954> formatting ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101955> downlaod (spelling) ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101957> Are these other types needed if you're just returning the stream to the HDFS log file? ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101956> Instead of trying to request "HostComponentLog:1" by key, why not just request only the Resource.Type.HostComponentLog instead of the other Resource.Types above and then use the first and only child? ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101958> That's an odd application type; is this the correct one to use for this web request? ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101961> Should be logged and handled correctly with respect to failure logic for retrieving logs. ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java <https://reviews.apache.org/r/27802/#comment101962> Hard coding NOT_FOUND seems wrong; there might be other errors that cause the exception. ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java <https://reviews.apache.org/r/27802/#comment101959> Should these HDFS property names belong here if they aren't actually retrieved via the Configuration instance? ambari-server/src/main/java/org/apache/ambari/server/controller/HostComponentLogResponse.java <https://reviews.apache.org/r/27802/#comment101963> I personally hate the use of "this." when assiging members in a constructor/method. However if the class already does it I try to be consistent. In this case, the constructor has a mixture of both. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProvider.java <https://reviews.apache.org/r/27802/#comment101964> These are not exposed in other places, are they? If not, no need to be public. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProvider.java <https://reviews.apache.org/r/27802/#comment101965> Perhaps make this a configuration property? ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProvider.java <https://reviews.apache.org/r/27802/#comment101968> This method will fail in HA mode for HDFS; in this mode, the http and https properties are scoped by two more parameters, such as hdfs-site/dfs.nameservices and hdfs-site/dfs.X.namenodes.Y where X and Y are parameterized namenode aliases. Example: dfs.namenode.http-address.hacluster.nn1 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProvider.java <https://reviews.apache.org/r/27802/#comment101967> The lack of http-address is not a problem when using SSL. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProvider.java <https://reviews.apache.org/r/27802/#comment101966> Why is SSL not supported for retrieving logs? This is how many installations run. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProvider.java <https://reviews.apache.org/r/27802/#comment101971> If I understand this code correctly, it retrieves the log file in chunks of length with an offset. However, if the LEVEL doesn't match and log entry in this chunk, no data is returned. Is this the behavior we want; offset=10000,length=5000,level=WARNING might return no data, but 15000/5000 suddenly does. ambari-server/src/main/resources/key_properties.json <https://reviews.apache.org/r/27802/#comment101969> Our direction has been to move away from this file and define these keys statically in the provider. Alerts and Views does this. ambari-server/src/main/resources/properties.json <https://reviews.apache.org/r/27802/#comment101970> Same for above, the direction has been to have the provider define these and not this file. - Jonathan Hurley On Nov. 10, 2014, 4:04 a.m., Cabir Zounaidou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27802/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2014, 4:04 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, John > Speidel, Mahadev Konar, Nate Cole, Sid Wagle, and Yusaku Sako. > > > Bugs: AMBARI-4083 > https://issues.apache.org/jira/browse/AMBARI-4083 > > > Repository: ambari > > > Description > ------- > > The host component log implements the following two rest api's. > > 1. To retrieve the log entries. > /clusters/{cluster}/hosts/{hostname}/host_components/{component}/logs > This api will retrieve the logs if available for the host component from > HDFS. It can fetch only maximum of 5120 bytes. The window can be adjusted > using the query parameters 'offset' and 'length'. It also provides simple > filtering using 'level' query parameter. > Sample response will look like below: > { > "href" : > "http://c6501.ambari.apache.org:8080/api/v1/clusters/cl1/hosts/c6503.ambari.apache.org/host_components/HBASE_CLIENT/logs";, > "items" : [ > { > "href" : > "http://c6501.ambari.apache.org:8080/api/v1/clusters/cl1/hosts/c6503.ambari.apache.org/host_components/HBASE_CLIENT/logs/HBASE_CLIENT";, > "length" : 5120, > "level" : null, > "offset" : 153920, > "size" : 159040, > "HostComponentLog" : { > "cluster_name" : "cl1", > "component_name" : "HBASE_CLIENT", > "entries" : [ > { > "timestamp" : "2014-11-01 17:46:38,456", > "level" : "DEBUG", > "thread" : "main-EventThread", > "message" : "master.SplitLogManager: task not yet acquired > /hbase-unsecure/splitWAL/WALs%2Fc6503.ambari.apache.org%2C60020%2C1414856039721-splitting%2Fc6503.ambari.apache.org%252C60020%252C1414856039721.1414856049592.meta > ver = 0" > }, > { > "timestamp" : "2014-11-01 17:46:38,459", > "level" : "DEBUG", > "thread" : "main-EventThread", > "message" : "master.SplitLogManager: put up splitlog task at > znode > /hbase-unsecure/splitWAL/WALs%2Fc6503.ambari.apache.org%2C60020%2C1414856039721-splitting%2Fc6503.ambari.apache.org%252C60020%252C1414856039721.1414856050193.meta" > }, > { > "timestamp" : "2014-11-01 17:46:38,460", > "level" : "DEBUG", > "thread" : "main-EventThread", > "message" : "master.SplitLogManager: put up splitlog task at > znode > /hbase-unsecure/splitWAL/WALs%2Fc6503.ambari.apache.org%2C60020%2C1414856039721-splitting%2Fc6503.ambari.apache.org%252C60020%252C1414856039721.1414856105560.meta" > }, > : > : > }] > } > ] > } > 2. To download the log file from HDFS use the following the api > > /clusters/{cluster}/hosts/{hostname}/host_components/{component}/logs?download=true > The response will automatically download the file with the HDFS file name. > > > Diffs > ----- > > ambari-agent/src/main/python/ambari_agent/Controller.py dc3a1cf > > ambari-server/src/main/java/org/apache/ambari/server/agent/AgentCommand.java > e2f013d > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java > e99e39f > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatResponse.java > 56b4f18 > > ambari-server/src/main/java/org/apache/ambari/server/agent/LogConfigCommand.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/agent/RegistrationResponse.java > 8a24560 > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/HostComponentLogResourceDefinition.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/HostComponentResourceDefinition.java > 6dc9e2d > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > 9ad37ec > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java > bb4c569 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/HostComponentService.java > 4990ad7 > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > 4f53544 > > ambari-server/src/main/java/org/apache/ambari/server/controller/HostComponentLogResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java > ae20f56 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProvider.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java > dbac906 > ambari-server/src/main/java/org/apache/ambari/server/state/LogConfig.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/LogConfigHandler.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/LogDefinition.java > PRE-CREATION > ambari-server/src/main/java/org/apache/ambari/server/state/LogEntry.java > PRE-CREATION > ambari-server/src/main/resources/key_properties.json c1a6636 > ambari-server/src/main/resources/log_handler_config.json PRE-CREATION > ambari-server/src/main/resources/properties.json 36cff96 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java > e7b946d > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentLogResourceProviderTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/27802/diff/ > > > Testing > ------- > > 1. Ran the unit tests. > 2. Manually ran the retrieve host component logs without any query parameters. > 3. Manually ran the retrieve host component logs with offset and length. > 4. Manually ran the retrieve host component logs with level filter. > 5. Manually ran the retrieve host component logs with webHDFS disabled. > 6. Manually ran the download host component log file with webHDFS enabled. > 7. Manually ran the download host component log file with webHDFS disabled. > > > Thanks, > > Cabir Zounaidou > >
