River 3.0 API changes

[Peter <ji...@zeus.net.au>]

Before we release River 3.0, I think it's important to discuss changes 
to the public api.

Changes to api have been minimal, however we should come to concensus 
prior to release.  Note that changes made were for correctness reasons 
only, but the community should decide whether we should favour 
correctness or backward compatibility.

The changes:

net.jini.core.event.RemoteEvent - protected fields changed to final & 
private.
net.jini.core.lookup.ServiceEvent - protected fields changed to final 
protected.
net.jini.discovery.RemoteDiscoveryEvent - protected field discarded 
changed to final protected, protected fields; marshalledRegs, regs & 
groups changed to private final.

Note that net.jini.lookup.LookupLocator's serial from changed in River 
2.2, in a non backward compatible way, however the LookupLocator in 
River 3.0 doesn't include the change, but it's serial form is backward 
compatible with both versions (at some point I'd like to address the 
issue that change intended to address).

In all cases serial form has been preserved.

These changes were made at the same time the new Startable.start() 
interface was created, that didn't go down too well, I didn't get around 
to discussing the above changes after the fallout from that.

Prior to making a decision, I'd like to discuss why the changes were 
made, examples of impacts it will have on downstream code, including 
what's updates are required.  Keep in mind that people will need to 
recompile River 3.0 due to namespace changes, so this should be based on 
whether the changes required can be done so without impacting client 
code backward compatibiliy.

I'm not against reverting these changes, however we need to understand 
that doing so will result in dropped events in tests, caused by unsafe 
publication, causing some test failures.  River 3.0 has far less 
blocking than the 2.2 branch, this exposes race conditions.

It's worth noting I haven't fixed all race conditions in River 3.0, but 
I have fixed the majority.

Regards,

Peter.

What I would have liked to fix but didn't (I did manage to work around 
it), a fundamental design issue with lease identity is, an expired lease 
is equal to a current lease, if both leases have the same id, when 
really the renew method should return a new lease with the same lease 
id, that isn't equal,  in my mind LeaseMap should have also been 
immutable, with a new map containing renewed leases returned upon renewal:

Sun Bug: 4287125
         Norm: Client leases are being renewed after the set they are in 
should
                 have expired.

                 A given client lease should not be renewed after the 
set it is in
                 has expired.  An iron-clad guarantee can't be made here 
because
                 we can't hold onto locks while leases are being 
renewed.  However,
                 the problem with Norm was worse than could be explained 
by the
                 short window between committing to renew a client lease 
and
                 performing the actual renewal.  The problem arose because
                 there was no check in the renewal threads to make sure that
                 the set the client lease was in still current, and 
because the
                 thread that removed sets from the service often runs 
late.  Such
                 a check is now performed so the window where a client 
lease renewal
                 can occur, after the lease on the set the client lease 
is in has
                 expired, is very small.

                 Comment by P. Firmstone Apr 21st 2016: <br>
                 These issues would not occur with immutable leases and 
sets,
                 upon renewal a new set with successfully renewed leases
                 would be returned.

Re: River 3.0 API changes

[Peter <ji...@zeus.net.au>]

Correction:

net.jini.core.discovery.LookupLocator (incorrect package listed below), 
also has two protected fields that have been made final, host and port.

Regards,

Peter.

On 25/04/2016 8:58 PM, Peter wrote:
> Before we release River 3.0, I think it's important to discuss changes 
> to the public api.
>
> Changes to api have been minimal, however we should come to concensus 
> prior to release.  Note that changes made were for correctness reasons 
> only, but the community should decide whether we should favour 
> correctness or backward compatibility.
>
> The changes:
>
> net.jini.core.event.RemoteEvent - protected fields changed to final & 
> private.
> net.jini.core.lookup.ServiceEvent - protected fields changed to final 
> protected.
> net.jini.discovery.RemoteDiscoveryEvent - protected field discarded 
> changed to final protected, protected fields; marshalledRegs, regs & 
> groups changed to private final.
>
> Note that net.jini.lookup.LookupLocator's serial from changed in River 
> 2.2, in a non backward compatible way, however the LookupLocator in 
> River 3.0 doesn't include the change, but it's serial form is backward 
> compatible with both versions (at some point I'd like to address the 
> issue that change intended to address).
>
> In all cases serial form has been preserved.
>
> These changes were made at the same time the new Startable.start() 
> interface was created, that didn't go down too well, I didn't get 
> around to discussing the above changes after the fallout from that.
>
> Prior to making a decision, I'd like to discuss why the changes were 
> made, examples of impacts it will have on downstream code, including 
> what's updates are required.  Keep in mind that people will need to 
> recompile River 3.0 due to namespace changes, so this should be based 
> on whether the changes required can be done so without impacting 
> client code backward compatibiliy.
>
> I'm not against reverting these changes, however we need to understand 
> that doing so will result in dropped events in tests, caused by unsafe 
> publication, causing some test failures.  River 3.0 has far less 
> blocking than the 2.2 branch, this exposes race conditions.
>
> It's worth noting I haven't fixed all race conditions in River 3.0, 
> but I have fixed the majority.
>
> Regards,
>
> Peter.
>
> What I would have liked to fix but didn't (I did manage to work around 
> it), a fundamental design issue with lease identity is, an expired 
> lease is equal to a current lease, if both leases have the same id, 
> when really the renew method should return a new lease with the same 
> lease id, that isn't equal,  in my mind LeaseMap should have also been 
> immutable, with a new map containing renewed leases returned upon 
> renewal:
>
> Sun Bug: 4287125
>         Norm: Client leases are being renewed after the set they are 
> in should
>                 have expired.
>
>                 A given client lease should not be renewed after the 
> set it is in
>                 has expired.  An iron-clad guarantee can't be made 
> here because
>                 we can't hold onto locks while leases are being 
> renewed.  However,
>                 the problem with Norm was worse than could be 
> explained by the
>                 short window between committing to renew a client 
> lease and
>                 performing the actual renewal.  The problem arose because
>                 there was no check in the renewal threads to make sure 
> that
>                 the set the client lease was in still current, and 
> because the
>                 thread that removed sets from the service often runs 
> late.  Such
>                 a check is now performed so the window where a client 
> lease renewal
>                 can occur, after the lease on the set the client lease 
> is in has
>                 expired, is very small.
>
>                 Comment by P. Firmstone Apr 21st 2016: <br>
>                 These issues would not occur with immutable leases and 
> sets,
>                 upon renewal a new set with successfully renewed leases
>                 would be returned.
>
>