[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8196729: Add jstatd option to specify RMI connector port
From:       Daniil Titov <daniil.x.titov () oracle ! com>
Date:       2020-02-06 18:26:29
Message-ID: 51118795-16BB-49F9-BA0D-257BFC06223D () oracle ! com
[Download RAW message or body]

Thank you Chris and Serguei for reviewing this change!

Best regards,
Daniil

On 2/5/20, 11:15 AM, "Chris Plummer" <chris.plummer@oracle.com> wrote:

    +1
    
    Chris
    
    On 2/5/20 9:36 AM, serguei.spitsyn@oracle.com wrote:
    > Hi Daniil,
    >
    > Looks good.
    > Thank you for the update!
    >
    > Thanks,
    > Serguei
    >
    >
    > On 2/4/20 22:00, Daniil Titov wrote:
    >> Hi Serguei,
    >>
    >> Thank you for finding this! Please review the new version of webrev [1]
    >> that has it corrected. The new webrev also includes changes in the 
    >> test to
    >> make sure that all jstatd tests run for both styles of command line 
    >> options.
    >>   Testing: Mach5 jobs for sun/tools/jstatd  succeeded.  Tiers1, 
    >> tiers2, tiers3,
    >> and tiers5 job are in the progress.
    >>
    >> [1] Webrev: http://cr.openjdk.java.net/~dtitov/8196729/webrev.02/
    >> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8196729
    >> [3] CSR : https://bugs.openjdk.java.net/browse/JDK-8238357
    >>
    >> Thanks,
    >> Daniil
    >>
    >> From: "serguei.spitsyn@oracle.com" <serguei.spitsyn@oracle.com>
    >> Date: Tuesday, February 4, 2020 at 7:51 PM
    >> To: Daniil Titov <daniil.x.titov@oracle.com>, 
    >> "serviceability-dev@openjdk.java.net" 
    >> <serviceability-dev@openjdk.java.net>
    >> Subject: Re: RFR: 8196729: Add jstatd option to specify RMI connector 
    >> port
    >>
    >> Hi Daniil,
    >>
    >> It looks okay to me in general.
    >> But I'm puzzled with this part:
    >>
    >> http://cr.openjdk.java.net/~dtitov/8196729/webrev.01/src/jdk.jstatd/share/classes/sun/tools/jstatd/Jstatd.java.udiff.html \
  >>
    >> +            } else if (arg.startsWith("-r")) {
    >> +                if (arg.compareTo("-r") != 0) {
    >> +                    port = Integer.parseInt(arg.substring(2));
    >> +                } else {
    >> +                    argc++;
    >> +                    if (argc >= args.length) {
    >> +                        printUsage();
    >> +                        System.exit(1);
    >> +                    }
    >> +                    rmiPort = Integer.parseInt(args[argc]);
    >> +                }
    >>
    >> The option -r is for rmi connection port number.
    >> Why does this code set the RMI registry port? :
    >> +                if (arg.compareTo("-r") != 0) {
    >> +                    port = Integer.parseInt(arg.substring(2));
    >>
    >> Thanks,
    >> Serguei
    >>
    >>
    >> On 1/31/20 13:08, Daniil Titov wrote:
    >> Please review change [1] that adds a new command line option to 
    >> jstatd tool to specify a RMI connector port.
    >>
    >> Currently a random port is used that prevents this tool from being 
    >> used behind a firewall or in a container.
    >>
    >> New CSR [3] was created for this change and it needs to be reviewed 
    >> as well.
    >>
    >> Man pages for jstatd will be updated in a separate issue.
    >>
    >> Testing: Mach5 tier1-tier3 and sun/tools/jstatd/* tests succeeded. 
    >> Mach5 tier5 tests are in progress.
    >>   [1] Webrev: http://cr.openjdk.java.net/~dtitov/8196729/webrev.01/
    >> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8196729
    >> [3] CSR : https://bugs.openjdk.java.net/browse/JDK-8238357
    >>
    >> Thank you,
    >> Daniil
    >>
    >>
    >>
    >>
    >>
    >>
    >>
    >
    
    


[prev in list] [next in list] [prev in thread] [next in thread]