On 02/22/2017 09:53 AM, Radim Vansa wrote: > On 02/22/2017 09:11 AM, Dan Berindei wrote: >> On Tue, Feb 21, 2017 at 8:28 PM, Radim Vansa <rva...@redhat.com> wrote: >>> On 02/21/2017 07:14 PM, Dan Berindei wrote: >>>> But doesn't the functional API's evalMany() provide something very >>>> close to the API you're suggesting? >>>> >>>> Dan >>>> >>>> >>>> On Tue, Feb 21, 2017 at 7:55 PM, Radim Vansa <rva...@redhat.com> wrote: >>>>> On 02/21/2017 05:16 PM, Tristan Tarrant wrote: >>>>>> On 21/02/17 16:29, Sanne Grinovero wrote: >>>>>>>> You haven't explained what "flush" means. Since you separate that from >>>>>>>> atomicity/consistency, I assume that batches on non-tx cache are just >>>>>>>> ordered putOrRemoveAll operations, immediately visible on flush without >>>>>>>> any atomicity. >>>>>> I assume that in Sanne's idea, ordering in a batch doesn't matter, aside >>>>>> from operations on the same key. Having ordering in there would for >>>>>> example not allow us to parallelize by segment. >>>>>> >>>>>>> So I want to write a first chunk, in our code that looks like: >>>>>>> >>>>>>> startBatch >>>>>>> put(chunk1/A, [some large value]) >>>>>>> put(chunk1/B, [some small metadata]) >>>>>>> put(chunk1/C, [some small metadata]) >>>>>>> endBatch >>>>>>> There is no reason to use a transaction, in fact we had to disable >>>>>>> transactions as some of these entries could be large. >>>>>>> There also is no reason for the batch, other than optimising the >>>>>>> latency. >>>>>> Let me summarize to see if we have the requirements for a useful >>>>>> batching system (which is sort of patterned on the JDBC statement >>>>>> batching): >>>>>> >>>>>> - a batch is not an atomic operation, i.e. it is not backed by a >>>>>> transaction >>>>>> - it can be wrapped in a transaction if needed >>>>>> - batches cannot be nested >>>>>> - batches only involve unconditional write operations (put, putAll, >>>>>> remove) >>>>>> - ordering of operations within a batch is unimportant aside from >>>>>> modifications to the same key where we apply "last one wins" >>>>>> - when a batch is "flushed" (i.e. endBatch is invoked) the ops are >>>>>> grouped by segment and sent to the appropriate owner for processing, >>>>>> potentially in parallel >>>>>> >>>>>> As Radim has called it, this is essentially a putOrRemoveAll op (with an >>>>>> async counterpart). >>>>> It is putOrRemoveAll when applied on a non-tx cache, and actually >>>>> implementing that shouldn't be complex. However, when transactions come >>>>> into play, it is different, because Sanne wants us to remove the >>>>> modifications in completed batch from the local transactional invocation >>>>> context and 'cache' them on the owners. Since reads have to be >>>>> transactionally consistent, we need to inspect the transaction on the >>>>> remote nodes (remote repeatable read). >>>>> >>>>> Sanne's request makes sense to me. However as the current implementation >>>>> is providing a false assumption that it could work as JDBC batches while >>>>> it's nothing but crippled JTA, and as I don't see anyone shouting "I'll >>>>> implement that, next week it's done!", I second deprecating/removing the >>>>> API for the time being. >>>>> >>>> Exactly my thoughts, it's definitely not an unreasonable request, but >>>> it would require a lot of work to implement correctly. >>>> >>>> >>>>> I don't find the current API ideal either, as it depends on thread >>>>> locals (JTA does as well, but...) while it does not seem useful enough >>>>> to me. I would prefer >>>>> >>>>> interface BatchingCache { >>>>> Batch start(); >>>>> } >>>>> >>>>> @NotThreadSafe >>>>> interface Batch { >>>>> void put(K k, V value); >>>>> ... >>>>> void execute(); >>>>> void drop(); // maybe not needed >>>>> } >>>>> >>>> I was hoping the functional API's evalMany() would be good enough, but >>>> I see now that it replicates the function argument (which holds all >>>> the values) everywhere. So putAll() is still more efficient, unless >>>> using groups to make sure all keys in the batch have the same owners. >>> Uh, doesn't putAll do the same? Is the modifications list isolate per >>> target segments when replicating? >>> >> I missed the evalMany(Map, BiFunction) overload yesterday, and I >> assume WriteOnlyMap.evalMany(Map, >> MarshallableFunctions.setValueConsumer()) would behave exactly the >> same as putAll(Map). Of course, it's not better either, as you still >> have to create the map beforehand... >> >> OTOH I'm pretty sure you're the one who wrote the code to split the >> input keys/values by target segments both for putAll() and for >> evalMany() in NonTxDistributionInterceptor :) > I was referring to "I see now that it replicates the function argument > (which holds all the values) everywhere. So putAll() is still more > efficient." And besides sending the setValue operation along, I don't > see what you mean. > > * In non-tx mode, evalMany() wraps the entries using > ReadOnlySegmentAwareMap which filters segments for given node during > marshalling. putAll does exactly the same. > * In tx mode, both commands are added to modifications list and this is > sent (in PrepareCommand) to all participating nodes, including entries > that are not owned by a given node. (that's a space for optimizations). > > Anyway, while evalMany would be sufficient for non-transactional mode > (and batching API would be only a convenience), it is not enough in tx > mode as it does not wipe out non-local modifications/reads on originator > during the flush. > When implementing tx functional commands, I was considering keeping > modifications on remote node during the transaction (before the prepare > phase) for repeatable functional reads, but in the end I went with > sending all modifications for a given entry along with the read.
Silly me :-/ When reviewing the code I've realized that non-functional reads don't send these modifications, which is a bug [1] R. [1] https://issues.jboss.org/browse/ISPN-7505 > > Radim > > > >> Dan >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > -- Radim Vansa <rva...@redhat.com> JBoss Performance Team _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev