[infinispan-dev] Configuration changes proposal for 8.0

[Dan Berindei]

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.

On the other hand, why not adopt the JCache interface and stop implementing Map?

>
>> - 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
>>