jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
Please, review the following change

Issue : https://bugs.openjdk.java.net/browse/JDK-8043937
Webrev: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.00

For Java SE 8, JSR 160 was updated to downgrade the support for the JXM
RMI-IIOP transport from required to optional. This was the first step in
a multi-release effort to remove support for the IIOP transport from the
JMX Remote API.

Early in JDK 9, the build was changed to not include the JMX RMI-IIOP
connector by default.

This is the last step in removing the JMX RMI-IIOP connector from the
JMX specification. This changeset will remove the implementation classes
as well as related tests.

All jdk_jmx and jdk_management tests are passing after this change. JCK
tests will need to be updated (tracked in a related issue).

Thanks,

-JB-
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Alan Bateman
On 18/08/2015 15:35, Jaroslav Bachorik wrote:

> Please, review the following change
>
> Issue : https://bugs.openjdk.java.net/browse/JDK-8043937
> Webrev: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.00
>
> For Java SE 8, JSR 160 was updated to downgrade the support for the
> JXM RMI-IIOP transport from required to optional. This was the first
> step in a multi-release effort to remove support for the IIOP
> transport from the JMX Remote API.
>
> Early in JDK 9, the build was changed to not include the JMX RMI-IIOP
> connector by default.
>
> This is the last step in removing the JMX RMI-IIOP connector from the
> JMX specification. This changeset will remove the implementation
> classes as well as related tests.
>
> All jdk_jmx and jdk_management tests are passing after this change.
> JCK tests will need to be updated (tracked in a related issue).
Do you have a webrev for the changes to the top-level repo? I ask
because part of the removal has to be the removal of the
--enable-rmiconnector-iiop configure option, essentially removing the
changes introduced by:

http://hg.openjdk.java.net/jdk9/dev/rev/e9e5bd372a4e

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
On 19.8.2015 08:19, Alan Bateman wrote:

> On 18/08/2015 15:35, Jaroslav Bachorik wrote:
>> Please, review the following change
>>
>> Issue : https://bugs.openjdk.java.net/browse/JDK-8043937
>> Webrev: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.00
>>
>> For Java SE 8, JSR 160 was updated to downgrade the support for the
>> JXM RMI-IIOP transport from required to optional. This was the first
>> step in a multi-release effort to remove support for the IIOP
>> transport from the JMX Remote API.
>>
>> Early in JDK 9, the build was changed to not include the JMX RMI-IIOP
>> connector by default.
>>
>> This is the last step in removing the JMX RMI-IIOP connector from the
>> JMX specification. This changeset will remove the implementation
>> classes as well as related tests.
>>
>> All jdk_jmx and jdk_management tests are passing after this change.
>> JCK tests will need to be updated (tracked in a related issue).
> Do you have a webrev for the changes to the top-level repo? I ask
> because part of the removal has to be the removal of the
> --enable-rmiconnector-iiop configure option, essentially removing the
> changes introduced by:
>
> http://hg.openjdk.java.net/jdk9/dev/rev/e9e5bd372a4e

I missed this. However, I have my doubts about
'common/autoconf/generated-configure.sh' - the comment says this file
should not be edited but it was changed in the aforementioned commit.
Should I change it back?

-JB-

>
> -Alan

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Alan Bateman


On 19/08/2015 09:59, Jaroslav Bachorik wrote:
>
> I missed this. However, I have my doubts about
> 'common/autoconf/generated-configure.sh' - the comment says this file
> should not be edited but it was changed in the aforementioned commit.
> Should I change it back?
You will need to edit spec.gmk.in and jdk-options.m4, then re-run
autogen to regenerate generated-configure.sh.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
On 19.8.2015 11:02, Alan Bateman wrote:

>
>
> On 19/08/2015 09:59, Jaroslav Bachorik wrote:
>>
>> I missed this. However, I have my doubts about
>> 'common/autoconf/generated-configure.sh' - the comment says this file
>> should not be edited but it was changed in the aforementioned commit.
>> Should I change it back?
> You will need to edit spec.gmk.in and jdk-options.m4, then re-run
> autogen to regenerate generated-configure.sh.

Top level changes: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/
JDK changes: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/jdk

-JB-

>
> -Alan

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Alan Bateman
On 19/08/2015 11:47, Jaroslav Bachorik wrote:
> Top level changes:
> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/
> JDK changes: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/jdk
I think you also need to look at jake/make/rmic/Rmic-java.management.gmk
which is where RMICONNECTOR_IIOP is used. This is the original change to
the jdk repo but the make files have since been refactored:

    http://hg.openjdk.java.net/jdk9/dev/jdk/rev/c952b81ad038

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

David Holmes
In reply to this post by Jaroslav Bachorik
<offlist>

Hi Jaroslav,

Note you will also have to push the closed generated-configure.sh file.

David

On 19/08/2015 8:47 PM, Jaroslav Bachorik wrote:

> On 19.8.2015 11:02, Alan Bateman wrote:
>>
>>
>> On 19/08/2015 09:59, Jaroslav Bachorik wrote:
>>>
>>> I missed this. However, I have my doubts about
>>> 'common/autoconf/generated-configure.sh' - the comment says this file
>>> should not be edited but it was changed in the aforementioned commit.
>>> Should I change it back?
>> You will need to edit spec.gmk.in and jdk-options.m4, then re-run
>> autogen to regenerate generated-configure.sh.
>
> Top level changes: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/
> JDK changes: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/jdk
>
> -JB-
>
>>
>> -Alan
>
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
In reply to this post by Alan Bateman
On 19.8.2015 13:20, Alan Bateman wrote:
> On 19/08/2015 11:47, Jaroslav Bachorik wrote:
>> Top level changes:
>> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/
>> JDK changes: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.01/jdk
> I think you also need to look at jake/make/rmic/Rmic-java.management.gmk
> which is where RMICONNECTOR_IIOP is used. This is the original change to
> the jdk repo but the make files have since been refactored:
>
>     http://hg.openjdk.java.net/jdk9/dev/jdk/rev/c952b81ad038

I hope I got it all this time. Used brute force search for any mention
of 'iiop' and removed anything that had anything to do with JMX.

Top level: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.02
JDK: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.02/jdk

-JB-

>
> -Alan

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Alan Bateman
On 19/08/2015 13:51, Jaroslav Bachorik wrote:
>
> I hope I got it all this time. Used brute force search for any mention
> of 'iiop' and removed anything that had anything to do with JMX.
>
> Top level: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.02
> JDK: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.02/jdk

I think you are close :-)

The @deprecated in class RMIIIOPServerImpl says "This method was ..."
when it should be "This class". It might be simpler to just drop that
sentence and say "The IIOP transport is no longer supported".

I see the methods in this class throw UOE but I assume that can't happen
because the constructor always throws an exception (except of course if
someone does something sneaky, say with finalizers). So I think what you
have is okay.

There are several tests in jdk/test/javax/management that exercise IIOP
if available, shouldn't they be updated too?

-Alan.


Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
On 19.8.2015 15:05, Alan Bateman wrote:

> On 19/08/2015 13:51, Jaroslav Bachorik wrote:
>>
>> I hope I got it all this time. Used brute force search for any mention
>> of 'iiop' and removed anything that had anything to do with JMX.
>>
>> Top level: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.02
>> JDK: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.02/jdk
>
> I think you are close :-)
>
> The @deprecated in class RMIIIOPServerImpl says "This method was ..."
> when it should be "This class". It might be simpler to just drop that
> sentence and say "The IIOP transport is no longer supported".

Ok.

>
> I see the methods in this class throw UOE but I assume that can't happen
> because the constructor always throws an exception (except of course if
> someone does something sneaky, say with finalizers). So I think what you
> have is okay.

Yes, under normal circumstances you should never be able to obtain an
instance of this class.

>
> There are several tests in jdk/test/javax/management that exercise IIOP
> if available, shouldn't they be updated too?

Not sure about this. These tests are also exercising JMXMP if available
- but that has never been a part of JDK.

-JB-

>
> -Alan.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Alan Bateman


On 19/08/2015 15:01, Jaroslav Bachorik wrote:
> :
>
>>
>> There are several tests in jdk/test/javax/management that exercise IIOP
>> if available, shouldn't they be updated too?
>
> Not sure about this. These tests are also exercising JMXMP if
> available - but that has never been a part of JDK.
I think the tests date back to when JMX was a standalone JSR and RI. In
any case, with IIOP support removed then it seems like these tests
should be updated but it's not critical to do it now.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
On 19.8.2015 16:32, Alan Bateman wrote:

>
>
> On 19/08/2015 15:01, Jaroslav Bachorik wrote:
>> :
>>
>>>
>>> There are several tests in jdk/test/javax/management that exercise IIOP
>>> if available, shouldn't they be updated too?
>>
>> Not sure about this. These tests are also exercising JMXMP if
>> available - but that has never been a part of JDK.
> I think the tests date back to when JMX was a standalone JSR and RI. In
> any case, with IIOP support removed then it seems like these tests
> should be updated but it's not critical to do it now.

What we could do is to update the tests to accept the required protocol
as a test parameter. That way they still can serve as a test harness for
any potential new JMX connectors (eg. JMXMP etc.)

-JB-

>
> -Alan

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
In reply to this post by Alan Bateman
On 19.8.2015 08:19, Alan Bateman wrote:

> On 18/08/2015 15:35, Jaroslav Bachorik wrote:
>> Please, review the following change
>>
>> Issue : https://bugs.openjdk.java.net/browse/JDK-8043937
>> Webrev: http://cr.openjdk.java.net/~jbachorik/8043937/webrev.00
>>
>> For Java SE 8, JSR 160 was updated to downgrade the support for the
>> JXM RMI-IIOP transport from required to optional. This was the first
>> step in a multi-release effort to remove support for the IIOP
>> transport from the JMX Remote API.
>>
>> Early in JDK 9, the build was changed to not include the JMX RMI-IIOP
>> connector by default.
>>
>> This is the last step in removing the JMX RMI-IIOP connector from the
>> JMX specification. This changeset will remove the implementation
>> classes as well as related tests.
>>
>> All jdk_jmx and jdk_management tests are passing after this change.
>> JCK tests will need to be updated (tracked in a related issue).
> Do you have a webrev for the changes to the top-level repo? I ask
> because part of the removal has to be the removal of the
> --enable-rmiconnector-iiop configure option, essentially removing the
> changes introduced by:
>
> http://hg.openjdk.java.net/jdk9/dev/rev/e9e5bd372a4e

Updated webrev top-level:
http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03
Updated webrev jdk:
http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03/jdk

Contains changes to the top level repo as well as a correct version of
jdk/make/rmic/Rmic-java.management.gmk (thanks to Erik Joelsson)

-JB-

>
> -Alan

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Alan Bateman

On 21/08/2015 12:49, Jaroslav Bachorik wrote:
>
> Updated webrev top-level:
> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03
> Updated webrev jdk:
> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03/jdk
>
> Contains changes to the top level repo as well as a correct version of
> jdk/make/rmic/Rmic-java.management.gmk (thanks to Erik Joelsson)
The updated webrev and the updated @deprecated in RMIIIOPServerImpl
looks good. Can you make sure to test out boot cycle builds before
pushing this? That would help with the confidence that the updates to
the RMI stub generation are good.

-Alan.
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
On 24.8.2015 08:52, Alan Bateman wrote:

>
> On 21/08/2015 12:49, Jaroslav Bachorik wrote:
>>
>> Updated webrev top-level:
>> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03
>> Updated webrev jdk:
>> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03/jdk
>>
>> Contains changes to the top level repo as well as a correct version of
>> jdk/make/rmic/Rmic-java.management.gmk (thanks to Erik Joelsson)
> The updated webrev and the updated @deprecated in RMIIIOPServerImpl
> looks good. Can you make sure to test out boot cycle builds before
> pushing this? That would help with the confidence that the updates to
> the RMI stub generation are good.

Bootcycle build is ok.

-JB-

>
> -Alan.

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Erik Joelsson
In reply to this post by Alan Bateman
This version looks good to me.

/Erik

On 2015-08-24 08:52, Alan Bateman wrote:

>
> On 21/08/2015 12:49, Jaroslav Bachorik wrote:
>>
>> Updated webrev top-level:
>> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03
>> Updated webrev jdk:
>> http://cr.openjdk.java.net/~jbachorik/8043937/webrev.03/jdk
>>
>> Contains changes to the top level repo as well as a correct version
>> of jdk/make/rmic/Rmic-java.management.gmk (thanks to Erik Joelsson)
> The updated webrev and the updated @deprecated in RMIIIOPServerImpl
> looks good. Can you make sure to test out boot cycle builds before
> pushing this? That would help with the confidence that the updates to
> the RMI stub generation are good.
>
> -Alan.

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev RFR 8043937: Drop support for the IIOP transport from the JMX RMIConnector

Jaroslav Bachorik
In reply to this post by Alan Bateman
On 19.8.2015 16:32, Alan Bateman wrote:

>
>
> On 19/08/2015 15:01, Jaroslav Bachorik wrote:
>> :
>>
>>>
>>> There are several tests in jdk/test/javax/management that exercise IIOP
>>> if available, shouldn't they be updated too?
>>
>> Not sure about this. These tests are also exercising JMXMP if
>> available - but that has never been a part of JDK.
> I think the tests date back to when JMX was a standalone JSR and RI. In
> any case, with IIOP support removed then it seems like these tests
> should be updated but it's not critical to do it now.

The test clean up is tracked as a separate task now - JDK-8134421

-JB-

>
> -Alan