river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Patricia Shanahan <p...@acm.org>
Subject Re: River 3.0 API changes
Date Mon, 25 Apr 2016 22:50:08 GMT
My initial feeling is in favor of reverting - if we have test failures, 
we should investigate whether the bug is in River or in the test. If it 
is in the test, fix the test.

I have been trying to think whether there is a simple way of finding and 
reviewing suspect tests. Presumably a test would have to be affected by 
the changes to have a bug that is fixed by them?


On 4/25/2016 3:46 PM, Peter wrote:
> If I don't receive any responses, I'll revert these changes, prior to publishing the
River 3.0 release artifacts next week end.
> Regards,
> Peter.
> Sent from my Samsung device.
>   Include original message
> ---- Original message ----
> From: Peter <jini@zeus.net.au>
> Sent: 25/04/2016 09:23:16 pm
> To: dev@river.apache.org
> Subject: Re: River 3.0 API changes
> 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

View raw message