mk23 / jmxproxy

JMX to HTTP Proxy
https://mk23.github.io/jmxproxy
MIT License
42 stars 19 forks source link

JMXProxy goes into locked state for a non-existent host #142

Closed sanketplus closed 7 years ago

sanketplus commented 7 years ago

when you query data for a host that is not there (shutdown) in my case, JMXProxy will be blocked and it will not process new requests until the error below comes.I opened #141 before but I narrowed down to this.

ERROR [2017-01-13 14:10:09,320] com.github.mk23.jmxproxy.jmx.ConnectionWorker: communication failure with service:jmx:rmi:///jndi/rmi://<hostname>/jmxrmi
! java.net.ConnectException: Connection timed out (Connection timed out)
! at java.net.PlainSocketImpl.socketConnect(Native Method)
! at java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
! at java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
! at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
! at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
! at java.net.Socket.connect(Socket.java:589)
! at java.net.Socket.connect(Socket.java:538)
! at java.net.Socket.<init>(Socket.java:434)
! at java.net.Socket.<init>(Socket.java:211)
! at sun.rmi.transport.proxy.RMIDirectSocketFactory.createSocket(RMIDirectSocketFactory.java:40)
! at sun.rmi.transport.proxy.RMIMasterSocketFactory.createSocket(RMIMasterSocketFactory.java:148)
! at sun.rmi.transport.tcp.TCPEndpoint.newSocket(TCPEndpoint.java:613)
! ... 21 common frames omitted
mk23 commented 7 years ago

This is caused by a synchronization on the host map for every request. I'm testing a change that should avoid this condition and still be concurrency-safe.

https://github.com/mk23/jmxproxy/blob/master/src/main/java/com/github/mk23/jmxproxy/jmx/ConnectionManager.java#L163-L174

mk23 commented 7 years ago

There are actually two issues here. One is that we serialize too aggressively. The other problem is that there's no way to specify a timeout for a jmx connection. I don't see a simple way to fix the latter. We could make the connection asynchronously and timeout before the connection is established to return an error to the client sooner. This is a bit troublesome and has several hairy edge conditions. For the next version, I'll opt to just make sure we don't block operations if we hit a bad agent.

sanketplus commented 7 years ago

Making it asynchronous would not block the JMXProxy operations and it will handle other incoming requests; but if the client which is polling JMXProxy is serial in operation then it can get blocked until the connection times out. And that the time is considerably long.

I'm just pointing it out :)

mk23 commented 7 years ago

In the spirit of never backing down from a challenge, I went spelunking through JDK code and figured out what needs to be subclassed in order to provide a proper connection timeout. This is implemented and working in my lab environment (changes coming soon). The only caveat is that this approach may make adding support for SSL agents a bit trickier (but not impossible).