Re: jmx-dev RFR(S): 8234484: Add ability to configure third port for remote JMX

[Daniel Fuchs]

Hi Felix,

On 11/01/2020 07:37, Yangfei (Felix) wrote:

Hi Daniel,

     Thanks for the suggestions.
     I modified the patch making the third port also configurable via the 
management.properties file.
     New webrev: http://cr.openjdk.java.net/~fyang/8234484/webrev.01/

I think I would prefer the try catch NumberFormatException
to be just around the call to Integer.parseInt(), as it is
done elsewhere in this file (e.g. line 337).

     Previously, we test the effectiveness of the patch by checking the port 
usage with -Dsun.rmi.transport.tcp.logLevel=VERBOSE.
     Is it good to modify the following test case specifying the third port for 
the testing purpose?

Please don't do that. That test is not appropriate for this - as
it opens a remote connector, not a local connector.

Also please get someone from serviceability-dev to also look at
changes (added in cc:).

best regards,

-- daniel

diff -r d630c0a63222 
test/jdk/sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java
--- a/test/jdk/sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java  
Wed Jan 08 08:53:28 2020 +0900
+++ b/test/jdk/sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java  
Sat Jan 11 15:30:25 2020 +0000
@@ -31,6 +31,7 @@

import jdk.test.lib.process.OutputAnalyzer;

  import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.Utils;

/**

   * @test
@@ -203,6 +204,12 @@
              args.add("-Dcom.sun.management.jmxremote.host=" + address);
              args.add("-Dcom.sun.management.jmxremote.port=" + jmxPort);
              args.add("-Dcom.sun.management.jmxremote.rmi.port=" + rmiPort);
+            try {
+                int port = Utils.getFreePort();
+                args.add("-Dcom.sun.management.jmxlocal.port=" + port);
+            } catch (Exception e) {
+                System.out.println(e);
+            }
              args.add("-Dcom.sun.management.jmxremote.authenticate=false");
              args.add("-Dcom.sun.management.jmxremote.ssl=" + 
Boolean.toString(useSSL));
              // This is needed for testing on loopback

Felix

Hi Felix,

Shouldn't this be configurable via the management.properties file, like all 
other
JMX properties?

Also on the one hand - it would be good to have a test.
On the other hand - writing a stable test will be problematic as there is no way
to ensure that any particular port number will be available for the agent to 
bind
to.

best regards,

-- daniel

On 08/01/2020 01:09, Yangfei (Felix) wrote:

Hi,

Please review this patch adding a way to configure the third port for
remote JMX.

Currently, it is possible to configure two of the three ports for JMX
with com.sun.management.jmxremote.port and
com.sun.management.jmxremote.rmi.port.

However, there is no way to configure the third port. This can cause
problems if the randomly assigned port conflicts with another service
that has not yet acquired its port.

     Bug: https://bugs.openjdk.java.net/browse/JDK-8234484

    Webrev: http://cr.openjdk.java.net/~fyang/8234484/webrev.00/

    Passed tiered-1 test.

Thanks,

Felix