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

2016-03-24 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread swill 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread rafaelweingartner 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread swill 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread rafaelweingartner 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread rafaelweingartner 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread rafaelweingartner 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread swill 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread swill 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread rafaelweingartner 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread swill 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread asfgit 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread swill 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-24 Thread rafaelweingartner 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-22 Thread DaanHoogland 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-22 Thread DaanHoogland 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-22 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-19 Thread ustcweizhou 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-19 Thread ustcweizhou 
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:

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

2016-03-08 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-08 Thread ustcweizhou 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-08 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-08 Thread ustcweizhou 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-08 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-07 Thread ustcweizhou 
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 
AuthorDate: Tue Sep 8 12:12:55 2015 +0200
Commit: wilderrodrigues 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-07 Thread kiwiflyer 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-07 Thread ustcweizhou 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

2016-03-07 Thread kiwiflyer 
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 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---