RFR: 7150256: Add back Diagnostic Command JMX API

[frederic parain]

Mandy,

Thanks for the review

New webrevs are available here:

7150256:
http://cr.openjdk.java.net/~fparain/7150256/webrev.05/

8004095:
http://cr.openjdk.java.net/~fparain/8004095/webrev.05/

My answers to your questions are in-lined below.

On 21/12/2012 00:40, Mandy Chung wrote:
> Hi Frederic,
>
> It's good to see the management interface for diagnostic commands is back.
>
> On 12/17/12 6:58 AM, Frederic Parain wrote:
>>
>> http://cr.openjdk.java.net/~fparain/7150256/webrev.01/
>>
> Please send your makefiles change to build-dev and build-infra for
> review to ensure necessary change is made in the new build-infra work.

makefiles have been reviewed and approved by the build team.

> I didn't have time to a detailed review and I think it looks okay. Some
> comments:
>
> DiagnosticCommandImpl.Wrapper class
>     If there is any issue initializing the diagnostic command, it
> ignores the exception.  That makes it very hard to diagnose when things
> go wrong.  This exports diagnostic commands supported by the running JVM
> and so I would think any error would be a bug.

An exception when creating the Wrapper doesn't necessarily mean a bug.
The call to get the list of diagnostic commands and the call to get
the descriptions of these commands cannot be performed in an atomic
way. Because the diagnostic command framework allows dynamic
addition and removal of commands, a command might "disappear" between
the two calls. In this case, the creation of the wrapper for this
command will fail, but this shouldn't be considered as a bug.

> DiagnosticCommandArgumentInfo is non-public class but its constructor
> and methods are still public.

Fixed.

> The new tests hardcode the port number that is unreliable and may fail
> in nightly/jprt testing (e.g. the port is occupied by another test
> run).   Some management tests have the same reliability issue and I'm
> not sure what the state is right now.  It'd be good if the new tests can
> avoid using hardcode port number.

I don't know how to avoid the hardcoding of the port without wrapping
the test in a shell scripts. Is there a template available to do
dynamic port allocation?

Thanks,

Fred

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at oracle.com