-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48972/#review141703
-----------------------------------------------------------




ambari-agent/src/main/python/ambari_agent/alerts/port_alert.py (line 66)
<https://reviews.apache.org/r/48972/#comment207064>

    I'm 50/50 on the fence about using parameters for this:
    - On one hand, they are constructs which should be consistent across all 
port alerts. Parameters by nature are not consistent - they are meant to extend 
alerts to provide coverage of an area which wasn't thought of originally. In 
this case, we can change the port alert behavior and just include a command and 
response as first-class citizens of the definition
    
    - On the other hand, parameters gives us a way to represent them in the UI 
and decorate them with descriptions. 
    
    I'm not going to flag this, but I'm curious to get the opinions of the 
other reviewers here. Originally, I was thinking of something like this:
    
    ```
    "source": {
      "type": "PORT",
      "uri": "{{zoo.cfg/clientPort}}",
      "default_port": 2181,
      "socket_command": "ruok",
      "socket_response": "imok",
      ...
    ```



ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5/alerts.json 
(line 61)
<https://reviews.apache.org/r/48972/#comment207057>

    "A socket command which queries ZooKeeper to respond with its state. The 
expected response is imok."



ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5/alerts.json 
(line 66)
<https://reviews.apache.org/r/48972/#comment207058>

    socket.command.response



ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5/alerts.json 
(lines 67 - 69)
<https://reviews.apache.org/r/48972/#comment207059>

    The expected response to the socket command.


- Jonathan Hurley


On July 8, 2016, 4:20 p.m., Masahiro Tanaka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48972/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 4:20 p.m.)
> 
> 
> Review request for Ambari, Florian Barca, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-17253
>     https://issues.apache.org/jira/browse/AMBARI-17253
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> There are too many WARNING in ZooKeeper log.
> ```
> 2016-06-15 21:02:15,405 - WARN  
> [NIOServerCxn.Factory:0.0.0.0/0.0.0.0:2181:NIOServerCnxn@357] - caught end of 
> stream exception
> EndOfStreamException: Unable to read additional data from client sessionid 
> 0x0, likely client has closed socket
>         at 
> org.apache.zookeeper.server.NIOServerCnxn.doIO(NIOServerCnxn.java:228)
>         at 
> org.apache.zookeeper.server.NIOServerCnxnFactory.run(NIOServerCnxnFactory.java:208)
>         at java.lang.Thread.run(Thread.java:745)
> ```
> 
> It may be because of Ambari Alert. Ambari Alert pings to the zookeeper port 
> to do monitoring.
> We should use 'ruok' to monitor zookeepers.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/alerts/port_alert.py 1918327 
>   ambari-agent/src/test/python/ambari_agent/TestPortAlert.py dffa56c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/PortSource.java
>  d7279de 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java
>  7ef12a7 
>   
> ambari-server/src/main/resources/common-services/ZOOKEEPER/3.4.5/alerts.json 
> 469036a 
> 
> Diff: https://reviews.apache.org/r/48972/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> ```
> +1 overall. Here are the results of testing the latest attachment 
> http://issues.apache.org/jira/secure/attachment/12811835/AMBARI-17253.2.patch
> against trunk revision .
> +1 @author. The patch does not contain any @author tags.
> +1 tests included. The patch appears to include 1 new or modified test files.
> +1 javac. The applied patch does not increase the total number of javac 
> compiler warnings.
> +1 release audit. The applied patch does not increase the total number of 
> release audit warnings.
> +1 core tests. The patch passed unit tests in .
> Test results: 
> https://builds.apache.org/job/Ambari-trunk-test-patch/7427//testReport/
> Console output: 
> https://builds.apache.org/job/Ambari-trunk-test-patch/7427//console
> This message is automatically generated.
> ```
> 
> 
> Thanks,
> 
> Masahiro Tanaka
> 
>

Reply via email to