Re: [infinispan-dev] ISPN-2463: Hopefully one final question
Yeah you're right that the tests will have to be moved about as well. There are still some instances that I'm finding where the old-school configs are being used but you don't see them in imports because of the packages that they exist in. For example, the tests in the org.infinispan.config.* module won't have any import for org.infinispan.config.Configuration etc. Which means it can be nasty to find at times. Navin Surtani Software Engineer JBoss SET JBoss EAP Twitter: @navssurtani - Original Message - From: "Manik Surtani" To: "infinispan -Dev List" Sent: Friday, November 23, 2012 1:02:31 AM Subject: Re: [infinispan-dev] ISPN-2463: Hopefully one final question And this will also determine where the tests live. I think we'll end up with a situation where the existing test suite is broken up and scattered across the different modules. On 22 Nov 2012, at 09:56, Tristan Tarrant < ttarr...@redhat.com > wrote: Navin, 6.0 will split core into several jars (names are currently fictional): * infinispan-api (which will only contain interfaces common to all types of caches) * infinispan-commons (common classes) * infinispan-local (local cache functionality) * infinispan-clustered (clustering functionality) This will make the hotrod client not pull in the entire embedded infinispan library as well. One thing this split will cause is that these jars should have different package roots to avoid the OSGi/JBoss modules "package split". Tristan On 11/22/2012 04:35 AM, Navin Surtani wrote: Could you elaborate on what other refactoring has to be done please? I mean, specifically on the configuration side, if there's a 1-for-1 replacement in terms of the configuration that's already available then shouldn't we just be directly swapping in the other API? I get that there are some changes to the way that things would be configured, and some defaults have been changed - but there are still ways to do the same thing correct? Navin Surtani Software Engineer JBoss SET JBoss EAP Twitter: @navssurtani - Original Message - From: "Tristan Tarrant" To: "infinispan -Dev List" Cc: "Navin Surtani" Sent: Wednesday, November 21, 2012 8:56:20 PM Subject: Re: [infinispan-dev] ISPN-2463: Hopefully one final question Before applying the chainsaw, we must first decide if we're going to have a 5.3. Also 6.0 will have much more refactoring than just dropping org.infinispan.config.* so I think that should be handled first. Tristan ___ 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 -- Manik Surtani ma...@jboss.org twitter.com/maniksurtani Platform Architect, JBoss Data Grid http://red.ht/data-grid ___ 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
Re: [infinispan-dev] Failover and state of Callable in dist.exec
On 22 Nov 2012, at 16:30, Vladimir Blagojevic wrote: > As Anna and I were working on failover tests we discovered one > peculiarity - state of Callable is not preserved as we send it to > various nodes for execution. I am not 100% this is important but it > might be for some clients. As things stand right now we always failover > to different nodes from master node. Every time Callables fails the call > unwinds immediately back to originating node and originating node sends > callable to another node. Fairly straight forward and expected. However, > for repeated executions, we send fresh "originating" copy of Callable, > in virgin state as it was submitted to dist.exec to begin with. In order > to preserve state we would have to serialize back state from failed node > and use that Callable state instead of virgin copy. > > What do you think? Is this state preservation a deal breaker? Or should > we postpone it for next release? Users cannot rely on the partial execution state, as the failure might be caused by a node crash, situation in which no intermediate state is obtained. For now I think we should only document this behaviour. Cheers, -- Mircea Markus Infinispan lead (www.infinispan.org) ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Failover and state of Callable in dist.exec
I'd keep it as it is, by making promises about preserving the callable state you're opening a lot of edge cases that you need to document. You have to also specify when the state is not preserved: if the target leaves the cluster, if there is a replication timeout, if there is a problem with serialization of the intermediate state, and so on... Besides, I don't think this feature would interact with transactions very well, because the Callable would be reading values from the cache in one transaction (on the original target) and using the same values in a different transaction (on the failover target) - with no consistency guarantees. Cheers Dan On Thu, Nov 22, 2012 at 6:30 PM, Vladimir Blagojevic wrote: > Hey guys, > > As Anna and I were working on failover tests we discovered one > peculiarity - state of Callable is not preserved as we send it to > various nodes for execution. I am not 100% this is important but it > might be for some clients. As things stand right now we always failover > to different nodes from master node. Every time Callables fails the call > unwinds immediately back to originating node and originating node sends > callable to another node. Fairly straight forward and expected. However, > for repeated executions, we send fresh "originating" copy of Callable, > in virgin state as it was submitted to dist.exec to begin with. In order > to preserve state we would have to serialize back state from failed node > and use that Callable state instead of virgin copy. > > What do you think? Is this state preservation a deal breaker? Or should > we postpone it for next release? > > Regards, > Vladimir > ___ > 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
Re: [infinispan-dev] Magic at JdbcStringBasedCacheStoreTest2
I don't like the idea very much, as we'd need one of those tests in each module. On Thu, Nov 22, 2012 at 7:00 PM, Manik Surtani wrote: > Maybe we should have a test that detects tests not ending in *Test, and > fails accordingly? ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
[infinispan-dev] Failover and state of Callable in dist.exec
Hey guys, As Anna and I were working on failover tests we discovered one peculiarity - state of Callable is not preserved as we send it to various nodes for execution. I am not 100% this is important but it might be for some clients. As things stand right now we always failover to different nodes from master node. Every time Callables fails the call unwinds immediately back to originating node and originating node sends callable to another node. Fairly straight forward and expected. However, for repeated executions, we send fresh "originating" copy of Callable, in virgin state as it was submitted to dist.exec to begin with. In order to preserve state we would have to serialize back state from failed node and use that Callable state instead of virgin copy. What do you think? Is this state preservation a deal breaker? Or should we postpone it for next release? Regards, Vladimir ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Magic at JdbcStringBasedCacheStoreTest2
Maybe we should have a test that detects tests not ending in *Test, and fails accordingly? On 22 Nov 2012, at 09:52, Dan Berindei wrote: > There are a few more tests with this issue, I've created > https://issues.jboss.org/browse/ISPN-2534 to change their names. > > > On Thu, Nov 22, 2012 at 11:34 AM, Thomas Fromm wrote: > On 22.11.2012 09:45, Thomas Fromm wrote: > > > > So why this test class does not fail in CI? > > > > > > Dan pointed me to the magic thing: Its not executed from CI because > naming is not *Test. ;-) > ___ > 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 -- Manik Surtani ma...@jboss.org twitter.com/maniksurtani Platform Architect, JBoss Data Grid http://red.ht/data-grid ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] public API/what should be preserve between minor releases?
On 22 Nov 2012, at 17:06, Manik Surtani wrote: > > On 21 Nov 2012, at 17:02, Mircea Markus wrote: > >> Hi, >> >> Cache, CacheManager and all the API that's in the org.infinispan package >> needs to be backward compatible. >> >> What about more obscure stuff, e.g. >> RpcManager.invokeRemotelyInFuture(Collection recipients, >> ReplicableCommand rpc, boolean usePriorityQueue, >> NotifyingNotifiableFuture future); >> >> This is still accessible through cache.getAdvancedCache().getRpcManager(), >> so it still counts as public API. I doubt that any user is using that method >> directly, but OTOH it's public so who knows. >> What do people think? Shall we be strict with regard to such 'obscure' >> methods between minor releases? > > More than something exposed via AdvancedCache, it's more of a concern when it > comes to people writing custom interceptors and may have direct access to the > RpcManager. > > What do you intend to change in the RpcManager? remove the future argument from all the methods once my async refactoring is in place. Cheers, -- Mircea Markus Infinispan lead (www.infinispan.org) ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] public API/what should be preserve between minor releases?
On 21 Nov 2012, at 17:02, Mircea Markus wrote: > Hi, > > Cache, CacheManager and all the API that's in the org.infinispan package > needs to be backward compatible. > > What about more obscure stuff, e.g. > RpcManager.invokeRemotelyInFuture(Collection recipients, > ReplicableCommand rpc, boolean usePriorityQueue, > NotifyingNotifiableFuture future); > > This is still accessible through cache.getAdvancedCache().getRpcManager(), so > it still counts as public API. I doubt that any user is using that method > directly, but OTOH it's public so who knows. > What do people think? Shall we be strict with regard to such 'obscure' > methods between minor releases? More than something exposed via AdvancedCache, it's more of a concern when it comes to people writing custom interceptors and may have direct access to the RpcManager. What do you intend to change in the RpcManager? - M -- Manik Surtani ma...@jboss.org twitter.com/maniksurtani Platform Architect, JBoss Data Grid http://red.ht/data-grid ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] ISPN-2463: Hopefully one final question
And this will also determine where the tests live. I think we'll end up with a situation where the existing test suite is broken up and scattered across the different modules. On 22 Nov 2012, at 09:56, Tristan Tarrant wrote: > Navin, > > 6.0 will split core into several jars (names are currently fictional): > infinispan-api (which will only contain interfaces common to all types of > caches) > infinispan-commons (common classes) > infinispan-local (local cache functionality) > infinispan-clustered (clustering functionality) > This will make the hotrod client not pull in the entire embedded infinispan > library as well. > One thing this split will cause is that these jars should have different > package roots to avoid the OSGi/JBoss modules "package split". > Tristan > > On 11/22/2012 04:35 AM, Navin Surtani wrote: >> Could you elaborate on what other refactoring has to be done please? I mean, >> specifically on the configuration side, if there's a 1-for-1 replacement in >> terms of the configuration that's already available then shouldn't we just >> be directly swapping in the other API? I get that there are some changes to >> the way that things would be configured, and some defaults have been changed >> - but there are still ways to do the same thing correct? >> >> >> >> Navin Surtani >> >> >> Software Engineer >> JBoss SET >> JBoss EAP >> >> >> Twitter: @navssurtani >> >> - Original Message - >> From: "Tristan Tarrant" >> To: "infinispan -Dev List" >> Cc: "Navin Surtani" >> Sent: Wednesday, November 21, 2012 8:56:20 PM >> Subject: Re: [infinispan-dev] ISPN-2463: Hopefully one final question >> >> Before applying the chainsaw, we must first decide if we're going to >> have a 5.3. >> Also 6.0 will have much more refactoring than just dropping >> org.infinispan.config.* so I think that should be handled first. >> >> Tristan >> ___ >> 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 -- Manik Surtani ma...@jboss.org twitter.com/maniksurtani Platform Architect, JBoss Data Grid http://red.ht/data-grid ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Refactoring async operations
On Thu, Nov 22, 2012 at 3:31 PM, Mircea Markus wrote:
>
> On 22 Nov 2012, at 10:16, Dan Berindei wrote:
>
>
> On Thu, Nov 22, 2012 at 11:53 AM, Galder Zamarreño wrote:
>
>>
>> On Nov 21, 2012, at 4:49 PM, Mircea Markus wrote:
>>
>> > Hi,
>> >
>> > Part of fixing ISPN-2435, I need to significantly change
>> DistributionInterceptor which at the moment is a very complex pice of code.
>> Building the fix on top of it is extremely difficult and error prone, so I
>> need to refactor it a bit before moving forward.
>> > One such refactoring is about changing the way the async operations are
>> handled (e.g. putAsync()). At the moment all the interceptor calls happen
>> in user's thread, but two remote calls which are invoked with futures and
>> aggregated:
>> > the L1 invalidation and the actual distribution call. The code for
>> handling this future aggregation is rather complicated and spreads over
>> multiple classes (RpcManager, L1Manager, ReplicationInterceptor,
>> DistributionInterceptor), so the simple alternative solution I have in mind
>> is to build an asycPut on top of a syncPut and wrap it in a future:
>> >
>> > CacheImpl:putAsync(k,v) {
>> > final InvocationContext ic =
>> createInvocatinonContextInCallerThread(); //this is for class loading
>> purpose
>> > return asyncPoolExecutor.submit(new Callable() {
>> > public Object call() {
>> > return put(k,v, ic); //this is the actual sync put
>> > }
>> > }
>> > }
>> >
>> > This would significantly simplify several components ( no references to
>> network/aggregated futures in RpcManager, L1Manager,
>> ReplicationInterceptor, DistributionInterceptor).
>>
>> ^ At first glance, that's how I'd have implemented this feature, but
>> Manik went down the route of wrapping in futures only those operations that
>> went remote.
>>
>> Maybe he was worried about ctx switch cost? Or maybe about ownership of
>> locks when these are acquired in a separate thread from the actual caller
>> thread?
>>
>
> Speaking of locks, does putAsync make sense in a transactional context?
>
> Good point, I don't think async operation should work in the context of
> transaction: that would mean having two threads(the async operation thread
> and the 'main' thread) working on the same javax.transaction.Transaction
> object concurrently which is something not supported by most TM afaik, and
> something we don't support internally.
>
>
I'm not sure, but I think it is supported now - the only things happening
on a different thread only care about the cache's transaction, and not
about the TM transaction.
> There may be another backwards compatibility issue here, with listeners
> that expect to be called on the caller's thread (e.g. to use the TM
> transaction that's stored in a thread-local).
>
>
>> > Possible issues:
>> > - caller's class loader - the class loader is aggregated in the
>> InvocationContext, so as long as we build the class loader in caller's
>> thread we should be fine
>>
>> ^ To be precise, we don't build a class loaders. I guess you're refering
>> at building the invocation context.
>>
>> These days we're more tight wrt the classloader used, avoiding the
>> reliance on the TCCL, so I think we're in a safer position.
>>
>> > - IsMarshallableInterceptor is used with async marshalling, in order to
>> notify the user when objects added to the cache are not serializable. With
>> the approach I suggested, for async calls only (e.g. putAsync) this
>> notification would not happen in caller's thread, but async on
>> future.get(). I really don't expect users to rely on this functionality,
>> but something that would change never the less.
>>
>> ^ I don't think this is crucial. You need to call future.get() to find
>> out if things worked correctly or not, regardless of cause.
>>
>> > - anything else you can think of?
>> >
>> > I know this is a significant change at this stage in the project, so I
>> really tried to go without it - but that resulted in spaghetti code taking
>> a lot of time to patch. So instead of spending that time to code a complex
>> hack I'd rather go for the simple and nice solution and add more unit tests
>> to prove it works.
>>
>> ^ Have you done some experimenting already?
>>
>> Cheers,
>>
>> >
>> > Cheers,
>> > --
>> > Mircea Markus
>> > Infinispan lead (www.infinispan.org)
>> >
>> >
>>
>
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] public API/what should be preserve between minor releases?
Hi Tomas, On 22 Nov 2012, at 08:46, Tomas Sykora wrote: > Hi guys, > > is this topic related? > https://community.jboss.org/wiki/RollingUpgradesInInfinispan Indirectly: in the case of rolling upgrades existing clients still need to be able to operate with new versions, which require an API compatibility. > > I'd assume that *whole* cluster will be upgraded to some higher version. > Running cluster with different versions seems risky to me too. This ^^ > process should be smooth but I don't know how about changes in API. > > I'm very interested in this topic - that's the reason why I jumped in to your > discussion. Very interesting topic indeed. 5.2 brings support for rolling upgrades, but the way we do it is not by allowing 5.1 and 5.2 nodes to coexist in the same cluster, but by creating a new 5.2 cluster and migrating both state and clients from the 5.1 clients to it. > Thank you! > > - Original Message - > From: "Thomas Fromm" > To: "infinispan -Dev List" > Sent: Thursday, November 22, 2012 8:23:22 AM > Subject: Re: [infinispan-dev] public API/what should be preserve between > minor releases? > > On 22.11.2012 04:11, Navin Surtani wrote: >>> As you see, IMHO such API changes do not have much effect in normal >>> situations. I can imagine only problems, when the changed methods are >>> used within dist exec calls and there are (during update or whatever) >>> different versions of infinispan inside the cluster. >> Does this really happen? I'm asking purely out of ignorance but on instinct >> I think that running different versions on the cluster is probably a bit >> risky? > > Thats the plan for our own software. Customer can update the nodes one > by one, of course only for patch releases. > I'd expect (or at least hope) that this will be also possible with > infinispan in case of critical bug was fixed or smth. > ___ > 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 Cheers, -- Mircea Markus Infinispan lead (www.infinispan.org) ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Refactoring async operations
On 22 Nov 2012, at 10:16, Dan Berindei wrote:
>
> On Thu, Nov 22, 2012 at 11:53 AM, Galder Zamarreño wrote:
>
> On Nov 21, 2012, at 4:49 PM, Mircea Markus wrote:
>
> > Hi,
> >
> > Part of fixing ISPN-2435, I need to significantly change
> > DistributionInterceptor which at the moment is a very complex pice of code.
> > Building the fix on top of it is extremely difficult and error prone, so I
> > need to refactor it a bit before moving forward.
> > One such refactoring is about changing the way the async operations are
> > handled (e.g. putAsync()). At the moment all the interceptor calls happen
> > in user's thread, but two remote calls which are invoked with futures and
> > aggregated:
> > the L1 invalidation and the actual distribution call. The code for handling
> > this future aggregation is rather complicated and spreads over multiple
> > classes (RpcManager, L1Manager, ReplicationInterceptor,
> > DistributionInterceptor), so the simple alternative solution I have in mind
> > is to build an asycPut on top of a syncPut and wrap it in a future:
> >
> > CacheImpl:putAsync(k,v) {
> > final InvocationContext ic = createInvocatinonContextInCallerThread();
> > //this is for class loading purpose
> > return asyncPoolExecutor.submit(new Callable() {
> > public Object call() {
> > return put(k,v, ic); //this is the actual sync put
> > }
> > }
> > }
> >
> > This would significantly simplify several components ( no references to
> > network/aggregated futures in RpcManager, L1Manager,
> > ReplicationInterceptor, DistributionInterceptor).
>
> ^ At first glance, that's how I'd have implemented this feature, but Manik
> went down the route of wrapping in futures only those operations that went
> remote.
>
> Maybe he was worried about ctx switch cost? Or maybe about ownership of locks
> when these are acquired in a separate thread from the actual caller thread?
>
> Speaking of locks, does putAsync make sense in a transactional context?
Good point, I don't think async operation should work in the context of
transaction: that would mean having two threads(the async operation thread and
the 'main' thread) working on the same javax.transaction.Transaction object
concurrently which is something not supported by most TM afaik, and something
we don't support internally.
>
> There may be another backwards compatibility issue here, with listeners that
> expect to be called on the caller's thread (e.g. to use the TM transaction
> that's stored in a thread-local).
>
>
> > Possible issues:
> > - caller's class loader - the class loader is aggregated in the
> > InvocationContext, so as long as we build the class loader in caller's
> > thread we should be fine
>
> ^ To be precise, we don't build a class loaders. I guess you're refering at
> building the invocation context.
>
> These days we're more tight wrt the classloader used, avoiding the reliance
> on the TCCL, so I think we're in a safer position.
>
> > - IsMarshallableInterceptor is used with async marshalling, in order to
> > notify the user when objects added to the cache are not serializable. With
> > the approach I suggested, for async calls only (e.g. putAsync) this
> > notification would not happen in caller's thread, but async on
> > future.get(). I really don't expect users to rely on this functionality,
> > but something that would change never the less.
>
> ^ I don't think this is crucial. You need to call future.get() to find out if
> things worked correctly or not, regardless of cause.
>
> > - anything else you can think of?
> >
> > I know this is a significant change at this stage in the project, so I
> > really tried to go without it - but that resulted in spaghetti code taking
> > a lot of time to patch. So instead of spending that time to code a complex
> > hack I'd rather go for the simple and nice solution and add more unit tests
> > to prove it works.
>
> ^ Have you done some experimenting already?
>
> Cheers,
>
> >
> > Cheers,
> > --
> > Mircea Markus
> > Infinispan lead (www.infinispan.org)
> >
> >
> >
> >
> > ___
> > infinispan-dev mailing list
> > infinispan-dev@lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Galder Zamarreño
> gal...@redhat.com
> twitter.com/galderz
>
> Project Lead, Escalante
> http://escalante.io
>
> Engineer, Infinispan
> http://infinispan.org
>
>
> ___
> 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
Cheers,
--
Mircea Markus
Infinispan lead (www.infinispan.org)
___
infinispan-dev mailing list
infinispan-dev@
Re: [infinispan-dev] Refactoring async operations
Thanks for the feedback Galder!
On 22 Nov 2012, at 09:53, Galder Zamarreño wrote:
>
> On Nov 21, 2012, at 4:49 PM, Mircea Markus wrote:
>
>> Hi,
>>
>> Part of fixing ISPN-2435, I need to significantly change
>> DistributionInterceptor which at the moment is a very complex pice of code.
>> Building the fix on top of it is extremely difficult and error prone, so I
>> need to refactor it a bit before moving forward.
>> One such refactoring is about changing the way the async operations are
>> handled (e.g. putAsync()). At the moment all the interceptor calls happen in
>> user's thread, but two remote calls which are invoked with futures and
>> aggregated:
>> the L1 invalidation and the actual distribution call. The code for handling
>> this future aggregation is rather complicated and spreads over multiple
>> classes (RpcManager, L1Manager, ReplicationInterceptor,
>> DistributionInterceptor), so the simple alternative solution I have in mind
>> is to build an asycPut on top of a syncPut and wrap it in a future:
>>
>> CacheImpl:putAsync(k,v) {
>>final InvocationContext ic = createInvocatinonContextInCallerThread();
>> //this is for class loading purpose
>>return asyncPoolExecutor.submit(new Callable() {
>> public Object call() {
>> return put(k,v, ic); //this is the actual sync put
>> }
>>}
>> }
>>
>> This would significantly simplify several components ( no references to
>> network/aggregated futures in RpcManager, L1Manager, ReplicationInterceptor,
>> DistributionInterceptor).
>
> ^ At first glance, that's how I'd have implemented this feature, but Manik
> went down the route of wrapping in futures only those operations that went
> remote.
>
> Maybe he was worried about ctx switch cost? Or maybe about ownership of locks
> when these are acquired in a separate thread from the actual caller thread?
>
>> Possible issues:
>> - caller's class loader - the class loader is aggregated in the
>> InvocationContext, so as long as we build the class loader in caller's
>> thread we should be fine
>
> ^ To be precise, we don't build a class loaders. I guess you're refering at
> building the invocation context.
yes, I'm referring to IC which aggregates the ClassLoader
>
> These days we're more tight wrt the classloader used, avoiding the reliance
> on the TCCL, so I think we're in a safer position.
>
>> - IsMarshallableInterceptor is used with async marshalling, in order to
>> notify the user when objects added to the cache are not serializable. With
>> the approach I suggested, for async calls only (e.g. putAsync) this
>> notification would not happen in caller's thread, but async on future.get().
>> I really don't expect users to rely on this functionality, but something
>> that would change never the less.
>
> ^ I don't think this is crucial. You need to call future.get() to find out if
> things worked correctly or not, regardless of cause.
+1
>
>> - anything else you can think of?
>>
>> I know this is a significant change at this stage in the project, so I
>> really tried to go without it - but that resulted in spaghetti code taking a
>> lot of time to patch. So instead of spending that time to code a complex
>> hack I'd rather go for the simple and nice solution and add more unit tests
>> to prove it works.
>
> ^ Have you done some experimenting already?
Yes, I've pretty much implemented the code without this refactoring. But then
ended up in a loop of fix regressions -> introduce new regressions, which was
very hard to break because of the complexity of the code.
>
> Cheers,
>
>>
>> Cheers,
>> --
>> Mircea Markus
>> Infinispan lead (www.infinispan.org)
>>
>>
>>
>>
>> ___
>> infinispan-dev mailing list
>> infinispan-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Galder Zamarreño
> gal...@redhat.com
> twitter.com/galderz
>
> Project Lead, Escalante
> http://escalante.io
>
> Engineer, Infinispan
> http://infinispan.org
>
>
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
Cheers,
--
Mircea Markus
Infinispan lead (www.infinispan.org)
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] no eviction on indexing
> Right you should never enable eviction on a cache used to store
> permanent data - like the Lucene index segment.
But couldn't you still have eviction if you used store to persist overflown
data.
> How would you phrase the error message?
Well, at least put in the *right* cache name somewhere in the msg. ;-)
I though I was going blind, as requesting cache - the indexing one - clearly
had NONE set as eviction strategy.
> On 22 November 2012 12:21, Ales Justin wrote:
>> Ah, OK, it should really be disabled on Lucene caches:
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> Horrible err msg ...
>>
>> On Nov 22, 2012, at 12:53 PM, Ales Justin wrote:
>>
>> I was changing cache config a bit, and got this:
>> * https://gist.github.com/4130728
>>
>> private static void verifyCacheHasNoEviction(AdvancedCache cache) {
>> if (cache.getConfiguration().getEvictionStrategy().isEnabled())
>> throw new IllegalArgumentException("DistributedSegmentReadLocker is
>> not reliable when using a cache with eviction enabled, disable eviction on
>> this cache instance");
>> }
>>
>>
>> How do you then handle memory overflow on no-eviction caches?
>>
>>
>> -Ales
>>
>>
>>
>> ___
>> 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
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] no eviction on indexing
Right you should never enable eviction on a cache used to store
permanent data - like the Lucene index segment.
How would you phrase the error message?
On 22 November 2012 12:21, Ales Justin wrote:
> Ah, OK, it should really be disabled on Lucene caches:
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> Horrible err msg ...
>
> On Nov 22, 2012, at 12:53 PM, Ales Justin wrote:
>
> I was changing cache config a bit, and got this:
> * https://gist.github.com/4130728
>
>private static void verifyCacheHasNoEviction(AdvancedCache cache) {
> if (cache.getConfiguration().getEvictionStrategy().isEnabled())
> throw new IllegalArgumentException("DistributedSegmentReadLocker is
> not reliable when using a cache with eviction enabled, disable eviction on
> this cache instance");
>}
>
>
> How do you then handle memory overflow on no-eviction caches?
>
>
> -Ales
>
>
>
> ___
> 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
Re: [infinispan-dev] no eviction on indexing
Ah, OK, it should really be disabled on Lucene caches:
Horrible err msg ...
On Nov 22, 2012, at 12:53 PM, Ales Justin wrote:
> I was changing cache config a bit, and got this:
> * https://gist.github.com/4130728
>
>private static void verifyCacheHasNoEviction(AdvancedCache cache) {
> if (cache.getConfiguration().getEvictionStrategy().isEnabled())
> throw new IllegalArgumentException("DistributedSegmentReadLocker is
> not reliable when using a cache with eviction enabled, disable eviction on
> this cache instance");
>}
>
>
> How do you then handle memory overflow on no-eviction caches?
>
>
> -Ales
>
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
[infinispan-dev] no eviction on indexing
I was changing cache config a bit, and got this:
* https://gist.github.com/4130728
private static void verifyCacheHasNoEviction(AdvancedCache cache) {
if (cache.getConfiguration().getEvictionStrategy().isEnabled())
throw new IllegalArgumentException("DistributedSegmentReadLocker is
not reliable when using a cache with eviction enabled, disable eviction on this
cache instance");
}
How do you then handle memory overflow on no-eviction caches?
-Ales
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Refactoring async operations
On Thu, Nov 22, 2012 at 11:53 AM, Galder Zamarreño wrote:
>
> On Nov 21, 2012, at 4:49 PM, Mircea Markus wrote:
>
> > Hi,
> >
> > Part of fixing ISPN-2435, I need to significantly change
> DistributionInterceptor which at the moment is a very complex pice of code.
> Building the fix on top of it is extremely difficult and error prone, so I
> need to refactor it a bit before moving forward.
> > One such refactoring is about changing the way the async operations are
> handled (e.g. putAsync()). At the moment all the interceptor calls happen
> in user's thread, but two remote calls which are invoked with futures and
> aggregated:
> > the L1 invalidation and the actual distribution call. The code for
> handling this future aggregation is rather complicated and spreads over
> multiple classes (RpcManager, L1Manager, ReplicationInterceptor,
> DistributionInterceptor), so the simple alternative solution I have in mind
> is to build an asycPut on top of a syncPut and wrap it in a future:
> >
> > CacheImpl:putAsync(k,v) {
> > final InvocationContext ic =
> createInvocatinonContextInCallerThread(); //this is for class loading
> purpose
> > return asyncPoolExecutor.submit(new Callable() {
> > public Object call() {
> > return put(k,v, ic); //this is the actual sync put
> > }
> > }
> > }
> >
> > This would significantly simplify several components ( no references to
> network/aggregated futures in RpcManager, L1Manager,
> ReplicationInterceptor, DistributionInterceptor).
>
> ^ At first glance, that's how I'd have implemented this feature, but Manik
> went down the route of wrapping in futures only those operations that went
> remote.
>
> Maybe he was worried about ctx switch cost? Or maybe about ownership of
> locks when these are acquired in a separate thread from the actual caller
> thread?
>
Speaking of locks, does putAsync make sense in a transactional context?
There may be another backwards compatibility issue here, with listeners
that expect to be called on the caller's thread (e.g. to use the TM
transaction that's stored in a thread-local).
> > Possible issues:
> > - caller's class loader - the class loader is aggregated in the
> InvocationContext, so as long as we build the class loader in caller's
> thread we should be fine
>
> ^ To be precise, we don't build a class loaders. I guess you're refering
> at building the invocation context.
>
> These days we're more tight wrt the classloader used, avoiding the
> reliance on the TCCL, so I think we're in a safer position.
>
> > - IsMarshallableInterceptor is used with async marshalling, in order to
> notify the user when objects added to the cache are not serializable. With
> the approach I suggested, for async calls only (e.g. putAsync) this
> notification would not happen in caller's thread, but async on
> future.get(). I really don't expect users to rely on this functionality,
> but something that would change never the less.
>
> ^ I don't think this is crucial. You need to call future.get() to find out
> if things worked correctly or not, regardless of cause.
>
> > - anything else you can think of?
> >
> > I know this is a significant change at this stage in the project, so I
> really tried to go without it - but that resulted in spaghetti code taking
> a lot of time to patch. So instead of spending that time to code a complex
> hack I'd rather go for the simple and nice solution and add more unit tests
> to prove it works.
>
> ^ Have you done some experimenting already?
>
> Cheers,
>
> >
> > Cheers,
> > --
> > Mircea Markus
> > Infinispan lead (www.infinispan.org)
> >
> >
> >
> >
> > ___
> > infinispan-dev mailing list
> > infinispan-dev@lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Galder Zamarreño
> gal...@redhat.com
> twitter.com/galderz
>
> Project Lead, Escalante
> http://escalante.io
>
> Engineer, Infinispan
> http://infinispan.org
>
>
> ___
> 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
Re: [infinispan-dev] ISPN-2463: Hopefully one final question
Navin, 6.0 will split core into several jars (names are currently fictional): * infinispan-api (which will only contain interfaces common to all types of caches) * infinispan-commons (common classes) * infinispan-local (local cache functionality) * infinispan-clustered (clustering functionality) This will make the hotrod client not pull in the entire embedded infinispan library as well. One thing this split will cause is that these jars should have different package roots to avoid the OSGi/JBoss modules "package split". Tristan On 11/22/2012 04:35 AM, Navin Surtani wrote: Could you elaborate on what other refactoring has to be done please? I mean, specifically on the configuration side, if there's a 1-for-1 replacement in terms of the configuration that's already available then shouldn't we just be directly swapping in the other API? I get that there are some changes to the way that things would be configured, and some defaults have been changed - but there are still ways to do the same thing correct? Navin Surtani Software Engineer JBoss SET JBoss EAP Twitter: @navssurtani - Original Message - From: "Tristan Tarrant" To: "infinispan -Dev List" Cc: "Navin Surtani" Sent: Wednesday, November 21, 2012 8:56:20 PM Subject: Re: [infinispan-dev] ISPN-2463: Hopefully one final question Before applying the chainsaw, we must first decide if we're going to have a 5.3. Also 6.0 will have much more refactoring than just dropping org.infinispan.config.* so I think that should be handled first. Tristan ___ 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
Re: [infinispan-dev] Refactoring async operations
On Nov 21, 2012, at 4:49 PM, Mircea Markus wrote:
> Hi,
>
> Part of fixing ISPN-2435, I need to significantly change
> DistributionInterceptor which at the moment is a very complex pice of code.
> Building the fix on top of it is extremely difficult and error prone, so I
> need to refactor it a bit before moving forward.
> One such refactoring is about changing the way the async operations are
> handled (e.g. putAsync()). At the moment all the interceptor calls happen in
> user's thread, but two remote calls which are invoked with futures and
> aggregated:
> the L1 invalidation and the actual distribution call. The code for handling
> this future aggregation is rather complicated and spreads over multiple
> classes (RpcManager, L1Manager, ReplicationInterceptor,
> DistributionInterceptor), so the simple alternative solution I have in mind
> is to build an asycPut on top of a syncPut and wrap it in a future:
>
> CacheImpl:putAsync(k,v) {
> final InvocationContext ic = createInvocatinonContextInCallerThread();
> //this is for class loading purpose
> return asyncPoolExecutor.submit(new Callable() {
> public Object call() {
> return put(k,v, ic); //this is the actual sync put
> }
> }
> }
>
> This would significantly simplify several components ( no references to
> network/aggregated futures in RpcManager, L1Manager, ReplicationInterceptor,
> DistributionInterceptor).
^ At first glance, that's how I'd have implemented this feature, but Manik went
down the route of wrapping in futures only those operations that went remote.
Maybe he was worried about ctx switch cost? Or maybe about ownership of locks
when these are acquired in a separate thread from the actual caller thread?
> Possible issues:
> - caller's class loader - the class loader is aggregated in the
> InvocationContext, so as long as we build the class loader in caller's thread
> we should be fine
^ To be precise, we don't build a class loaders. I guess you're refering at
building the invocation context.
These days we're more tight wrt the classloader used, avoiding the reliance on
the TCCL, so I think we're in a safer position.
> - IsMarshallableInterceptor is used with async marshalling, in order to
> notify the user when objects added to the cache are not serializable. With
> the approach I suggested, for async calls only (e.g. putAsync) this
> notification would not happen in caller's thread, but async on future.get().
> I really don't expect users to rely on this functionality, but something that
> would change never the less.
^ I don't think this is crucial. You need to call future.get() to find out if
things worked correctly or not, regardless of cause.
> - anything else you can think of?
>
> I know this is a significant change at this stage in the project, so I really
> tried to go without it - but that resulted in spaghetti code taking a lot of
> time to patch. So instead of spending that time to code a complex hack I'd
> rather go for the simple and nice solution and add more unit tests to prove
> it works.
^ Have you done some experimenting already?
Cheers,
>
> Cheers,
> --
> Mircea Markus
> Infinispan lead (www.infinispan.org)
>
>
>
>
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
gal...@redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] Magic at JdbcStringBasedCacheStoreTest2
There are a few more tests with this issue, I've created https://issues.jboss.org/browse/ISPN-2534 to change their names. On Thu, Nov 22, 2012 at 11:34 AM, Thomas Fromm wrote: > On 22.11.2012 09:45, Thomas Fromm wrote: > > > > So why this test class does not fail in CI? > > > > > > Dan pointed me to the magic thing: Its not executed from CI because > naming is not *Test. ;-) > ___ > 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
Re: [infinispan-dev] Magic at JdbcStringBasedCacheStoreTest2
On 22.11.2012 09:45, Thomas Fromm wrote: > > So why this test class does not fail in CI? > > Dan pointed me to the magic thing: Its not executed from CI because naming is not *Test. ;-) ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
Re: [infinispan-dev] public API/what should be preserve between minor releases?
Hi guys, is this topic related? https://community.jboss.org/wiki/RollingUpgradesInInfinispan I'd assume that *whole* cluster will be upgraded to some higher version. Running cluster with different versions seems risky to me too. This ^^ process should be smooth but I don't know how about changes in API. I'm very interested in this topic - that's the reason why I jumped in to your discussion. Thank you! - Original Message - From: "Thomas Fromm" To: "infinispan -Dev List" Sent: Thursday, November 22, 2012 8:23:22 AM Subject: Re: [infinispan-dev] public API/what should be preserve between minor releases? On 22.11.2012 04:11, Navin Surtani wrote: >> As you see, IMHO such API changes do not have much effect in normal >> situations. I can imagine only problems, when the changed methods are >> used within dist exec calls and there are (during update or whatever) >> different versions of infinispan inside the cluster. > Does this really happen? I'm asking purely out of ignorance but on instinct I > think that running different versions on the cluster is probably a bit risky? Thats the plan for our own software. Customer can update the nodes one by one, of course only for patch releases. I'd expect (or at least hope) that this will be also possible with infinispan in case of critical bug was fixed or smth. ___ 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
[infinispan-dev] Magic at JdbcStringBasedCacheStoreTest2
Hey, I recognized today when executing test cases above at cmd line: infinispan/cachestore/jdbc ~> mvn -Dsurefire.useFile=false -Dtest=org/infinispan/loaders/jdbc/stringbased/JdbcStringBasedCacheStorest2 test ends up in: Caused by: java.lang.InstantiationException: org.infinispan.loaders.keymappers.TwoWayKey2StringMapper Problematic row is config.setKey2StringMapperClass(TwoWayKey2StringMapper.class.getName()); I looked around to find some magic mocking, which explains why there can be only an interface used, but I failed. Using e.g. TwoWayPersonKey2StringMapper at this point works. So why this test class does not fail in CI? --tf ___ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
