I can't think of a useful application for doing a put-if-absent without having it return a success / fail boolean.
Same for replacement and other atomic conditional operations: that boolean return is important to have. On 22 May 2015 11:29, "Galder Zamarreño" <gal...@redhat.com> wrote: > > > On 22 May 2015, at 09:54, Dan Berindei <dan.berin...@gmail.com> wrote: > > > > On Fri, May 22, 2015 at 9:41 AM, Galder Zamarreño <gal...@redhat.com> > wrote: > >> > >>> On 14 May 2015, at 16:40, Dan Berindei <dan.berin...@gmail.com> wrote: > >>> > >>> On Wed, May 13, 2015 at 8:07 PM, Pedro Ruivo <pe...@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. > > Well, but the primary owner would need to ship information to the > originator on whether the cache entry exists or not. Sure, there's a > difference between sending back a Boolean vs a V if the V is large, but you > still pay the penalty of network. > > > > >> > >>> 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@lists.jboss.org > >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev > >> > >> > >> -- > >> Galder Zamarreño > >> gal...@redhat.com > >> > >> > >> > >> > >> > >> _______________________________________________ > >> infinispan-dev mailing list > >> infinispan-dev@lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > > _______________________________________________ > > infinispan-dev mailing list > > infinispan-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > -- > Galder Zamarreño > gal...@redhat.com > > > > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev