On Tue, Feb 4, 2014 at 10:07 AM, Galder Zamarreño <galder@redhat.com> wrote:

On 28 Jan 2014, at 15:29, William Burns <mudokonman@gmail.com> wrote:

> Hello everyone,
>
> I wanted to discuss what I would say as dubious benefit of L1OnRehash
> especially compared to the benefits it provide.
>
> L1OnRehash is used to retain a value by moving a previously owned
> value into the L1 when a rehash occurs and this node no longer owns
> that value Also any current L1 values are removed when a rehash
> occurs. Therefore it can only save a single remote get for only a few
> keys when a rehash occurs.
>
> This by itself is fine however L1OnRehash has many edge cases to
> guarantee consistency as can be seen from
> https://issues.jboss.org/browse/ISPN-3838. This can get quite
> complicated for a feature that gives marginal performance increases
> (especially given that this value may never have been read recently -
> at least normal L1 usage guarantees this).
>
> My first suggestion is instead to deprecate the L1OnRehash
> configuration option and to remove this logic.

+1

+1 from me as well

> My second suggestion is a new implementation of L1OnRehash that is
> always enabled when L1 threshold is configured to 0. For those not
> familiar L1 threshold controls whether invalidations are broadcasted
> instead of individual messages. A value of 0 means to always
> broadcast. This would allow for some benefits that we can't currently
> do:
>
> 1. L1 values would never have to be invalidated on a rehash event
> (guarantee locality reads under rehash)
> 2. L1 requestors would not have to be tracked any longer
>
> However every write would be required to send an invalidation which
> could slow write performance in additional cases (since we currently
> only send invalidations when requestors are found). The difference
> would be lessened with udp, which is the transport I would assume
> someone would use when configuring L1 threshold to 0.

Sounds good to me, but I think you could go even beyond this and maybe get rid of threshold configuration option too?

If the transport is UDP and multicast is configured, invalidations are broadcasted (and apply the two benefits you mention).
If UDP w/ unicast or TCP used, track invalidations and send them as unicasts.

Do we really need to expose these configuration options to the user?

I think the idea was that even with UDP, sending 2 unicasts and waiting for only 2 responses may be faster than sending a multicast and waiting for 10 responses. However, I'm not sure that's the case if we send 1 unicast invalidation from each owner instead of a single multicast invalidation from the primary owner/originator [1]. Maybe if each owner would return a list of requestors and the originator would do the invalidation at the end...

One tangible benefit of having the setting is that we can run the test suite with TCP only, and still cover every path in L1Manager. If removed it completely, it would still be possible to change the toggle in L1ManagerImpl via reflection, but it would be a little hacky.

> What do you guys think? I am thinking that no one minds the removal
> of L1OnRehash that we have currently (if so let me know). I am quite
> curious what others think about the changes for L1 threshold value of
> 0, maybe this configuration value is never used?
>

Since we don't give any guidance as to what a good threshold value would be, I doubt many people use it.

My alternative proposal would be to replace the invalidationThreshold=-1|0|>0 setting with a traceRequestors=true|false setting.

1. If traceRequestors == false, don't keep track of requestors, only send the invalidation from the originator, and enable l1OnRehash.

This means we can keep the entries that are in L1 after a rehash as well.

2. If traceRequestors == true, track requestors, send unicast/multicast invalidations depending on the transport, and disable l1OnRehash.

[1] https://issues.jboss.org/browse/ISPN-186

Cheers

Dan