[infinispan-dev] Configuration changes proposal for 8.0

Fri May 22 03:54:37 EDT 2015

On Fri, May 22, 2015 at 9:41 AM, Galder Zamarreño <galder at redhat.com> wrote:
>
>> On 14 May 2015, at 16:40, Dan Berindei <dan.berindei at gmail.com> wrote:
>>
>> On Wed, May 13, 2015 at 8:07 PM, Pedro Ruivo <pedro at infinispan.org> wrote:
>>> Hi,
>>>
>>> comments inline
>>>
>>> Pedro
>>>
>>> On 05/13/2015 05:42 PM, Tristan Tarrant wrote:
>>>> Hi all,
>>>>
>>>> this is a list of proposed changes for 8.x that affect configuration and
>>>> default behaviour. We built this list last year, but never acted upon
>>>> it, but now I believe it's time to do these changes. Please
>>>> comment/add/remove as you think is appropriate.
>>>>
>>>> - Make IGNORE_RETURN_VALUES the default
>>>
>>> hmmm, not sure about it. it only makes sense for put(k,v) and
>>> remove(k,v) since all other write operations are conditional (i.e. need
>>> the previous value and no optimization can be made)
>>
>> For putIfAbsent(k, v) it also helps if you don't have to return the
>> previous value, and I would expect most write operations to be put(k,
>> v) anyway.
>
> AFAIK, the main objective of IGNORE_RETURN_VALUES has been to avoid having to check what the previous value was. Even if putIfAbsent() would not return the previous value, it needs to check if put happens for the first time, so there's an attempt to retrieve the previous value, if there's any, from the cluster.

In transactions, yes, you still have to retrieve the value from the
owners, because the originator makes the decision. But in a
non-transactional cache, the primary owner wouldn't have to ship the
previous value to the originator.

>
>> On the other hand, why not adopt the JCache interface and stop implementing Map?
>
> ^ The Java 8 API I've been working on tries to make this easier to achieve (and update will be available in the next week or so!), but I don't think we can abandon ConcurrentMap just yet...
>

Ok, I might have exaggerated with "stop implementing Map", but
IGNORE_RETURN_VALUES won't matter if we make JCache the
default/recommended way to access the cache in 8.0.

>>
>>>
>>>> - Drop asymmetric marshalling [1]
>>
>> +1, but it should be *asynchronous* marshalling.
>>
>>>> - Remove/change some transaction options [2]
>>>> - ReplicationQueue - one should not be allowed to specify it as an instance
>>>> - ReplicationQueue - enabled should be implicit ⇔ interval > 0 ||
>>>> queueSize > 0. Also flushing when queueSize has been reached should
>>>> happen async.
>>>
>>> Can we remove the ReplicationQueue? First, it does not have any benefit
>>> (JGroups already bundles the message and the Network can do it too) and
>>> second, it is not more efficient (when the message is delivered, we
>>> process the commands sequentially. So, if the first command blocks, the
>>> other commands are not processed until it finished). Third, it is
>>> complex if you have commands with multiple delivers orders (no order,
>>> FIFO, total)
>>
>> +1, the replication queue has the same purpose as the bundler in
>> JGroups. And while the JGroups bundler has multiple options that keep
>> evolving, our replication queue still has the same algorithm it had in
>> 4.0.
>>
>>>
>>>> - AsyncStoreConfiguration.shutdownTimeout -
>>>> force-immediate=”false[default]” (destructive)
>>>>                     .flushLockTimeout - remove as it is no longer used
>>>> - BackupConfigurationBuilder - remove getters and implicitly set
>>>> BackupFailurePolicy
>>>> - allow setting failurePolicy by class not by string
>>>> - Reconsider usefulness of ClusterLoader
>>>
>>> didn't get this one ^
>>
>> -1 from me to remove the ClusterLoader, unless we replace it with a
>> better integrated alternative for invalidation mode.
>>
>>>
>>>> - DataContainer.properties - to be removed
>>>
>>> so, we will not allow any custom implementation to be configured? or I
>>> miss something?
>>>
>>>> - ExpirationConfiguration.reaperEnabled should be removed and the
>>>> interval value used instead
>>>> - HashConfiguration - remove deprecated elements
>>
>> Here I would suggest also replacing the Hash with another interface:
>> https://issues.jboss.org/browse/ISPN-5465
>>
>>>> - L1Configuration to fail if the reaper frequency interval is less or
>>>> equal than 0
>>>> - LockingConfigurationBuilder.useLockStriping - remove it &
>>>> writeSkewCheck enabled by default and removed
>>>
>>
>> +1 to remove lock striping
>>
>>> write skew check should be moved to Transactions. It is the only place
>>> in which it makes sense
>>
>> +1 to enable writeSkewCheck by default
>> +1 to remove it from the locking configuration. But I think "write
>> skew disabled" should be another locking mode, perhaps called
>> LAST_WRITE_WINS.
>>
>>>
>>>> - SingletonStore - pushStateWhenCoordinator implicit by pushStateTimeout
>>>> -> flushTimeout
>>>> - StateTransfer.fetchInMemoryState -> renamed to enabled
>>
>> +1
>>
>>>> - StoreAsBinary - remove “enabled” and reply on storeKey/storeBinary +
>>>> remove deprecation
>>>> - remove deprecations from TransactionConfiguration and remove
>>>> use1PcForAutCommitTx
>>
>> +1
>>
>>>> - VersioningConfiguration.enabled should be computed based on the scheme
>>>> and removed
>>
>> +1
>>
>>>> - Version - remove the version fields and populate them through a mvn script
>>>> - SerializationConfigurationBuilder - consider removing the version field
>>
>> +1 to remove it, the javadoc there suggests we support
>> interoperability between different Infinispan versions and that's
>> still too onerous for us at this point.
>>
>>>> - ShutdownConfiguration - to be removed, confirm with an email
>>>> - TransportConfiguration.distributedSyncTimeout -> global RPC timeout
>>
>> +1
>>
>>>> - strictPeerToPeer should go as it is deprecated
>>
>> +1
>>
>>>>
>>>
>>> another suggestion:
>>>
>>> - Transaction, remove the transaction_protocol and add TOTAL as an
>>> option for locking_mode
>>>
>>> *or*
>>>
>>> - Transaction, remove the transaction_protocol
>>> - Locking, add locking mode with LOCK and TOTAL_ORDER options (since
>>> I'll probably implement total order based non transactional caches)
>>
>> I don't feel like TOTAL belongs to the locking element. Since total
>> order is only relevant in clustered caches, how about making it an
>> alternative clustering mode?
>>
>> <clustering mode="SYNC|ASYNC|TOTAL"/>
>>
>>
>>>
>>>>
>>>> [1] https://issues.jboss.org/browse/ISPN-2939
>>>> [2] https://issues.jboss.org/browse/ISPN-3927
>>>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Galder Zamarreño
> galder at redhat.com
>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev