You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by kiwiflyer <gi...@git.apache.org> on 2016/03/07 23:25:53 UTC

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

GitHub user kiwiflyer opened a pull request:

    https://github.com/apache/cloudstack/pull/1430

    CLOUDSTACK-9285 for 4.7.x

    Per Daan's request, here is a pull request for the 4.7.x release.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/myENA/cloudstack 4.7_cloudstack-9285

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1430.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1430
    
----
commit 5b6fbe6aebbe9cd24dfebe9271ecb5dc40877c94
Author: Simon Weller <sw...@ena.com>
Date:   2016-03-07T22:23:35Z

    Cloudstack 9285 for 4.7.x

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1430#discussion_r55286730
  
    --- Diff: agent/src/com/cloud/agent/Agent.java ---
    @@ -412,7 +412,8 @@ protected void reconnect(final Link link) {
                 try {
                     _connection.start();
                 } catch (final NioConnectionException e) {
    -                throw new CloudRuntimeException("Unable to start the connection!", e);
    +               s_logger.info("Attempted to connect to the server, but received an unexpected exception, trying again...");
    --- End diff --
    
    1. I think you should fix line 230/238, that's the root cause of the issue. line 415 is not.
    2. line 416 can be removed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200931266
  
    You are welcome.
    I try to go over as many PRs as I can, but I always end up overlooking some. You can always ping me on slack or mail to check a PR before a merge.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200883526
  
    output was posted for test run against master and 4.7.  maybe I jumped the gun?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200914880
  
    Ok,
    So, this agent class is executed as a service in the operating system (OS) the OS will call a service that calls the main method that instantiates and executes that agent right? 
    
    If an exception is thrown and it is not handled properly the agent dies. If the agent dies, ACS does not deal with it, right? 
    
    In that case, I would be inclined to accept the infinite loop when starting a connection. I would only suggest not hiding the exception there, and logging it together with messages at lines 239, 238 and 415.
    
    I just wonder why we need to maintain a connection like this one always open. For me, it seems that if we only opened it when sending a command, then we could send the command, and after that we could close the connection. Therefore, if it is not possible to open a connection we could just lose a command. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/1430


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-198239218
  
    stop the management server, and restart cloudstack-agent during the stopping.
    
    1. without the commit 3683dff :
    ```
    2016-03-17 21:25:26,359 ERROR [utils.nio.NioConnection] (main:null) Unable to initialize the threads.
    java.io.IOException: Connection closed with -1 on reading size.
            at com.cloud.utils.nio.Link.doHandshake(Link.java:513)
            at com.cloud.utils.nio.NioClient.init(NioClient.java:80)
            at com.cloud.utils.nio.NioConnection.start(NioConnection.java:88)
            at com.cloud.agent.Agent.start(Agent.java:228)
            at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:401)
            at com.cloud.agent.AgentShell.launchAgentFromClassInfo(AgentShell.java:369)
            at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:353)
            at com.cloud.agent.AgentShell.start(AgentShell.java:463)
            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
            at java.lang.reflect.Method.invoke(Method.java:606)
            at org.apache.commons.daemon.support.DaemonLoader.start(DaemonLoader.java:243)
    2016-03-17 21:25:26,362 INFO  [utils.exception.CSExceptionErrorCode] (main:null) Could not find exception: com.cloud.utils.exception.NioConnectionException in error code list for exceptions
    2016-03-17 21:25:26,362 ERROR [cloud.agent.AgentShell] (main:null) Unable to start agent:
    com.cloud.utils.exception.CloudRuntimeException: Unable to start the connection!
            at com.cloud.agent.Agent.start(Agent.java:230)
            at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:401)
            at com.cloud.agent.AgentShell.launchAgentFromClassInfo(AgentShell.java:369)
            at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:353)
            at com.cloud.agent.AgentShell.start(AgentShell.java:463)
            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
            at java.lang.reflect.Method.invoke(Method.java:606)
            at org.apache.commons.daemon.support.DaemonLoader.start(DaemonLoader.java:243)
    Caused by: com.cloud.utils.exception.NioConnectionException: Connection closed with -1 on reading size.
            at com.cloud.utils.nio.NioConnection.start(NioConnection.java:94)
            at com.cloud.agent.Agent.start(Agent.java:228)
            ... 9 more
    Caused by: java.io.IOException: Connection closed with -1 on reading size.
            at com.cloud.utils.nio.Link.doHandshake(Link.java:513)
            at com.cloud.utils.nio.NioClient.init(NioClient.java:80)
            at com.cloud.utils.nio.NioConnection.start(NioConnection.java:88)
            ... 10 more
    2016-03-17 21:25:26,363 INFO  [cloud.agent.Agent] (AgentShutdownThread:null) Stopping the agent: Reason = sig.kill
    2016-03-17 21:25:26,364 DEBUG [cloud.agent.Agent] (AgentShutdownThread:null) Sending shutdown to management server
    ```
    
    2. with the commit 3683dff , 
    agent.log show the following log every 5 seconds, until agent connects to management server:
    ```
    2016-03-18 08:10:45,518 INFO  [utils.nio.NioClient] (main:null) Connecting to 172.16.15.10:8250
    2016-03-18 08:10:45,529 ERROR [utils.nio.NioConnection] (main:null) Unable to initialize the threads.
    java.io.IOException: Connection closed with -1 on reading size.
            at com.cloud.utils.nio.Link.doHandshake(Link.java:513)
            at com.cloud.utils.nio.NioClient.init(NioClient.java:80)
            at com.cloud.utils.nio.NioConnection.start(NioConnection.java:88)
            at com.cloud.agent.Agent.start(Agent.java:236)
            at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:401)
            at com.cloud.agent.AgentShell.launchAgentFromClassInfo(AgentShell.java:369)
            at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:353)
            at com.cloud.agent.AgentShell.start(AgentShell.java:463)
            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
            at java.lang.reflect.Method.invoke(Method.java:606)
            at org.apache.commons.daemon.support.DaemonLoader.start(DaemonLoader.java:243)
    2016-03-18 08:10:45,530 INFO  [utils.exception.CSExceptionErrorCode] (main:null) Could not find exception: com.cloud.utils.exception.NioConnectionException in error code list for exceptions
    2016-03-18 08:10:45,530 INFO  [cloud.agent.Agent] (main:null) Attempted to connect to the server, but received an unexpected exception, trying again...
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200908383
  
    @swill  This exception is thrown when the NIO operations used to establish the connection to the management server on port 8250 fail.When this exception gets thrown, the agent is dead and has to be manually restarted. The tomcat container, however, is still up.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200956335
  
    @swill Yes, I'll work on this either today or tomorrow and get a new PR submitted.
    
    Thanks guys.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200888647
  
    The test output you mean was the one posted by @kiwiflyer ?
    I had not reviewed this PR (sorry for that), but there are two (2) things here that called my attention.
    
    At class “Agent.java” lines, 230, 238 and 451, (despite being the very same piece of code) I worry about the exception that is not being logged. I mean, we will see that message, but the exception stack that might be useful for debugging will not be logged.
    
    Additionally, the code between lines 232 and 239, it seems that it might occur an infinite loop there (before the commit the throw new CloudRuntimeException would break it). I know that when we deal with a connection to a resource, there are hundreds of things that can go wrong and sometimes if we try once or twice it might solve the problem. However, a code that may enter into an infinite loop with a hidden exception does not sound a neat solution for me. 
    
    Would not it be better to retry a few times and then, if nothing changes, let an exception happens to break that flow of execution?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-199787118
  
    Test log for reconnect scenerio:
    
    Agent throws an exception that can never be recovered from when the agent attempts to reconnect and is sent a RST. This case occurs when a load balancer (haproxy) is proxying the traffic and there is no management server active to serve the request on the backend.
    
    Original issue logs from agent.log:
    
    2016-03-03 17:15:36,527 INFO utils.nio.NioClient (logid:) NioClient connection closed
    2016-03-03 17:15:36,527 INFO cloud.agent.Agent (logid:) Reconnecting...
    2016-03-03 17:15:36,527 INFO utils.nio.NioClient (logid:) Connecting to 10.103.0.154:8250
    2016-03-03 17:15:36,540 ERROR utils.nio.NioConnection (logid:) Unable to initialize the threads.
    java.io.IOException: Connection closed with -1 on reading size.
    at com.cloud.utils.nio.Link.doHandshake(Link.java:513)
    at com.cloud.utils.nio.NioClient.init(NioClient.java:80)
    at com.cloud.utils.nio.NioConnection.start(NioConnection.java:88)
    at com.cloud.agent.Agent.reconnect(Agent.java:413)
    at com.cloud.agent.Agent$ServerHandler.doTask(Agent.java:868)
    at com.cloud.utils.nio.Task.call(Task.java:83)
    at com.cloud.utils.nio.Task.call(Task.java:29)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:745)
    2016-03-03 17:15:36,545 INFO utils.exception.CSExceptionErrorCode (logid:) Could not find exception: com.cloud.utils.exception.NioConnectionException in error code list for exceptions
    After the patch is applied:
    
    2016-03-03 17:50:05,190 INFO utils.nio.NioClient (logid:) NioClient connection closed
    2016-03-03 17:50:05,190 INFO cloud.agent.Agent (logid:) Reconnecting...
    2016-03-03 17:50:05,190 INFO utils.nio.NioClient (logid:) Connecting to 10.103.0.154:8250
    2016-03-03 17:50:05,206 ERROR utils.nio.NioConnection (logid:) Unable to initialize the threads.
    java.io.IOException: Connection closed with -1 on reading size.
    at com.cloud.utils.nio.Link.doHandshake(Link.java:513)
    at com.cloud.utils.nio.NioClient.init(NioClient.java:80)
    at com.cloud.utils.nio.NioConnection.start(NioConnection.java:88)
    at com.cloud.agent.Agent.reconnect(Agent.java:413)
    at com.cloud.agent.Agent$ServerHandler.doTask(Agent.java:869)
    at com.cloud.utils.nio.Task.call(Task.java:83)
    at com.cloud.utils.nio.Task.call(Task.java:29)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:745)
    2016-03-03 17:50:05,210 INFO utils.exception.CSExceptionErrorCode (logid:) Could not find exception: com.cloud.utils.exception.NioConnectionException in error code list for exceptions
    2016-03-03 17:50:05,211 INFO cloud.agent.Agent (logid:) Attempted to connect to the server, but received an unexpected exception, trying again...
    2016-03-03 17:50:10,211 INFO cloud.agent.Agent (logid:) Reconnecting...
    2016-03-03 17:50:10,211 INFO utils.nio.NioClient (logid:) Connecting to 10.103.0.154:8250
    
    This has been tested on 4.7.1 and master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-193648204
  
    @kiwiflyer this is the log in jira:
    
    ```
    2016-02-15 10:46:21,611 INFO  [utils.exception.CSExceptionErrorCode] (main:null) (logid:) Could not find exception: com.cloud.utils.exception.NioConnectionException in error code list for exceptions
    2016-02-15 10:46:21,612 ERROR [cloud.agent.AgentShell] (main:null) (logid:) Unable to start agent:
    com.cloud.utils.exception.CloudRuntimeException: Unable to start the connection!
            at com.cloud.agent.Agent.start(Agent.java:230)
            at com.cloud.agent.AgentShell.launchAgent(AgentShell.java:399)
            at com.cloud.agent.AgentShell.launchAgentFromClassInfo(AgentShell.java:367)
    ```
    
    @wilderrodrigues this issue was introduced by your commit 79a3f8c5774c50dc128968c29da5096dc3dde39e, can you please have a look?
    
    ```
    commit 79a3f8c5774c50dc128968c29da5096dc3dde39e
    Refs: 4.5.1-2982-g79a3f8c
    Author:     wilderrodrigues <wr...@schubergphilis.com>
    AuthorDate: Tue Sep 8 12:12:55 2015 +0200
    Commit:     wilderrodrigues <wr...@schubergphilis.com>
    CommitDate: Fri Sep 11 11:28:40 2015 +0200
    
        CLOUDSTACK-8822 - Replacing Runnable by Callable in the Taks and NioConnection classes
    
           - All the sub-classes were also updated according to the changes in the super-classes
           - There were also code formatting changes
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200905706
  
    That is it; 
    Well, giving that the “_shell.getBackoffAlgorithm().waitBeforeRetry()” uses a parameter that indicates the amount of time between retries, I would try for 10-20 times before throwing an exception.
    If after “n” times the error keeps happening, I see no reason to keep trying. The administrator should be able to see that error and act to solve it.
    
    @ kiwiflyer, I do not know that class. Is that the agent we use in system VMs? Or the ones we install in some hypervisors?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-193796033
  
    @ustcweizhou 
    So after looking at our issue and the one reported originally in 9285, I think they're two different issues. One related to initial connection on agent startup and our one, related to a re-connection event where the connection is terminated unexpectedly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200919876
  
    @kiwiflyer, it is nice that we achieved a consensus. 
    What do you think @swill?
    
    How do we proceed now? This PR has already been merged and forwarded.
    I believe the easiest approach would be to open a new one just adding the exceptions to the log.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200903313
  
    so you are basically saying that if the connection can not be established, it is not functional anyway, so there is no point in this timing out?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200901622
  
    I want to point out that this is taking care of the case where you have a load balancer between the agents and the management server (see original issue notes). I'm personally not convinced that letting an exception be thrown that causes the agent to fail, that is most likely caused by an underlying network connectivity issue is a great solution either. Tomcat doesn't get taken down, so the agent container is functional, but the application is dead.
    
    If a set number of retries is added, there has to be a clean termination of the agent so some other health checking application can restart the agent, without requiring manual intervention (very painful if you have lots of hosts).
    
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200906752
  
    @rafaelweingartner This is used in both the system VM agent and the host (hypervisor) agent.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-193813489
  
    @kiwiflyer Simon, I agree with you. Besides your change,  we also need to fix similar issue in line 230/238.
    Actually line 230 caused the issue described in the ticket.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-193851127
  
    @ustcweizhou  Does 3683dff work?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200936609
  
    Thank you sir.  I really appreciate it.  Once I get my CI online I will be able to start getting through this backlog in a much more efficient manner.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-199787555
  
    @swill dear RM, can we merge this in 4.7 and merge forward


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200918379
  
    @rafaelweingartner Yes, you are correct. I like your suggestion. I'll log the exception with the failure.
    
    In terms of the persistent connections, I believe the management server pushes to the agents. It also relies on the agent connection to determine health information for the agent and the VMs located on each host.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200897012
  
    I see your point and I agree with you.  What do you think is a reasonable amount of time to check before we throw the error?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-199787161
  
    LGTM based on field experience and code inspection


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200882489
  
    this was merged without functional tests (integration tests)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-193820061
  
    @ustcweizhou I've cleaned up the other 2 exceptions and also removed the newline you pointed out eariler.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200884247
  
    i will make sure that integration test are run against everything going forward.  this judgment call may have been premature.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-197556331
  
    @kiwiflyer 3683dff LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-193827846
  
    @kiwiflyer code LGTM. 
    can you 
    (1) squash the commits,
    (2) add a space before s_logger.info ? We use 4 space indentation in java.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-200926830
  
    @rafaelweingartner I agree.  I think it makes the most sense to open a new PR to add the exception log.  @kiwiflyer would you mind doing that for us?  Would you mind giving the output as you did before to show what is displayed in the logs when this code is hit? 
    
    Thanks for the quick and quality discussion on this guys.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9285 for 4.7.x

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the pull request:

    https://github.com/apache/cloudstack/pull/1430#issuecomment-193508523
  
    @ustcweizhou Thanks for the advice. Maybe I can catch you on the slack channel tomorrow to discuss a little, so I better understand the logic between start and reconnect.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---