TheCloudlessSky / NHibernate.Caches.Redis

An NHibernate caching provider for Redis.
MIT License
59 stars 39 forks source link

Switch Redis library to to StackExchange.Redis #8

Closed TheCloudlessSky closed 9 years ago

TheCloudlessSky commented 10 years ago
TheCloudlessSky commented 10 years ago

@MattiasJakobsson @Tazer, I've pushed 2.0.0-alpha5 to Nuget with these changes. Can you guys give it a try (don't forget the -IncludePrerelease switch)? I've updated one of my projects to use the pre-release version and everything is running great.

Please checkout the readme to see if the changes make sense too.

Tazer commented 10 years ago

Great we will be testing this soon, We are a bit busy at the moment, but will test it out next week or so (as we running our version at the moment)

Tazer commented 10 years ago

We are now using it an staging environment, will get back to you if we notice any problems

Tazer commented 10 years ago

@TheCloudlessSky we are getting exceptions.

"Timeout performing EXEC, inst: 0, queue: 4, qu=1, qs=3, qc=0, wr=1/1, in=0/0" Not really sure why, we are using redis for other things, but it's just in the Nhibernate Caches provider this error occur. And when working via a vpn to the redis server it's occurring even ofter.

Not sure if the timeout's should be raised ?

TheCloudlessSky commented 10 years ago

What's the stack trace for that exception? That'll help track down the timeout. Depending on which one, the timeout can be increased.

bunceg commented 10 years ago

Hi, I know Tazer hasn't replied but do you have any thoughts about making this live and when? Do you intend to replace the existing package with this version or keep it running alongside as a different package?

Thanks

TheCloudlessSky commented 10 years ago

Hey @bunceg, right now you can use the 2.0.0 pre-release on nuget and test it out. It'd be much appreciated if you tried this out with your projects and reported any issues before making the official release.

I intend to fully replace the existing package with this version because of ServiceStack.Redis' licensing changes.

Tazer commented 10 years ago

Havnt seen the problem, only way I have reproduced it is when breakpointing. So we will get it in production om Tuesday and see how it works, so far it's been stable after the issue. On 12 Sep 2014 21:38, "Adrian Phinney" notifications@github.com wrote:

Hey @bunceg https://github.com/bunceg, right now you can use the 2.0.0-alpha3 pre-release on nuget and test it out. It'd be much appreciated if you tried this out with your projects and reported any issues before making the official release.

I intend to fully replace the existing package with this version because of ServiceStack.Redis' licensing changes.

— Reply to this email directly or view it on GitHub https://github.com/TheCloudlessSky/NHibernate.Caches.Redis/pull/8#issuecomment-55450987 .

TheCloudlessSky commented 10 years ago

@Tazer In that case then, yes, I saw it when break pointing too - but I figured it was because the connection was timing out because of the breakpoint...

TheCloudlessSky commented 10 years ago

StackExchange/StackExchange.Redis#83 is most likely related to this. What version of StackExchange.Redis are you running? Also, where are you putting the breakpoint to cause the issue? I'm running the sample app with 1.0.322 and I'm not getting any timeouts right now.

TheCloudlessSky commented 9 years ago

@Tazer I've made lots of changes that should help with performance. Would you mind giving it a test?

Tazer commented 9 years ago

Sure, I will give it a try tomorrow :)

On Tue, 16 Dec 2014 19:29 Adrian Phinney notifications@github.com wrote:

@Tazer https://github.com/Tazer I've made lots of changes that should help with performance. Would you mind giving it a test?

— Reply to this email directly or view it on GitHub https://github.com/TheCloudlessSky/NHibernate.Caches.Redis/pull/8#issuecomment-67207257 .

Tazer commented 9 years ago

Running the latest version on staging now. Will give it a few days so we dont get any unexcpeted problems.

TheCloudlessSky commented 9 years ago

Sweet! We're running it in production. FYI, I too ran into some timeout issues that seem to stem from Redis syching the RDB/AOF files (we're running on EC2). We increased the timeout to 2 seconds and haven't had any troubles:

var config = new ConfigurationOptions();
config.EndPoints.Add(redisHostAndPort);
// Increase the timeout so that AOF writes do not cause exceptions:
// https://github.com/StackExchange/StackExchange.Redis/issues/83
config.SyncTimeout = 2000;
Tazer commented 9 years ago

Have been running in production for a while now. Seems to work as it should,

But we also needed to make the syncTimeout change.

TheCloudlessSky commented 9 years ago

Great! Is there anything in the API that you'd like to see before I make the 2.0 release?

Tazer commented 9 years ago

Well , only one thing that comes up in mind is region configuration.

So you can add a region/prefix for the cache, So you can have multiple sites use nhibernate cache without conflicts. So alla keys get prefixed. Cause we found problem with that when running staging and production against same redis server instance

TheCloudlessSky commented 9 years ago

This can be accomplished with NHibernate by default. Set the cache.region_prefix property in your configuration:

<hibernate-configuration xmlns="urn:nhibernate-configuration-2.2">
  <session-factory>
    <property name="show_sql">true</property>
    <property name="cache.region_prefix">Staging</property>
  </session-factory>
</hibernate-configuration>

You can also do it if you're using FluentNHibernate (you can do this also by setting the property on the Configuration object in normal NHibernate too). For example:

fluentConfig.Cache(c =>
{
    c.UseSecondLevelCache().UseQueryCache()
    .ProviderClass<RedisCacheProvider>()
    .RegionPrefix("Staging");
})

This will make keys like:

NHibernate-Cache:Staging.App.Users.Users:v1:App.Models.Users#2

You can also set the region per-class in NHibernate. We use the region prefix globally and per-class when we make a schema change and we want to bust through the cache. Check out this blog post I wrote for an automated solution to this.

bunceg commented 9 years ago

One thing I noticed (with the OLD provider, and haven't got round to checking this one) is that if entities are cached, and then the app is updated and the domain model changes, the old entities are returned from the cache. i.e. the fact the model entities have changed doesn't seem to be tracked. I'm not sure if we're doing something wrong, it's expected behaviour so manually flush the cache after a release, an Nibernate caching bug, a service stack redis bug or an internal NHibernate bug.

Just thought I'd mention it though, just in case it's an NHibernate caching bug and it's still in this new version.

TheCloudlessSky commented 9 years ago

@bunceg If the entities change, everything should just work (as long as you have your configuration all in place and every ISession is hooked up to the correct cache). The previous version with ServiceStack.Redis worked as expected (we had been running it in production for ~1.5 years). So, I'm going to guess it might be an issue with your configuration/setup. Maybe you can open a separate issue and I can help you further debug the problem (also make sure you update to 2.0 of this library since it'll have much more stability/performance improvements when using StackExchange.Redis).

bunceg commented 9 years ago

Ok thanks I thought it must be us, but I'm not sure where the configuration error is :) I'll update to StackExchange shortly - I'd like to pull ServiceStack out of our project completely anyway..

Tazer commented 9 years ago

One thing that I noticed is that the depencies on StackExchange.Redis , Is conflicting with for example: http://blogs.msdn.com/b/webdev/archive/2014/05/12/announcing-asp-net-session-state-provider-for-redis-preview-release.aspx

That uses the StackExchange.Redis.StrongName , Maybe the dependency should be changed to that instead of the StackExchange.Redis ?

Tazer commented 9 years ago

Wrote it in my closed pull request, but guess this comment should be good to have in the Readme : https://github.com/TheCloudlessSky/NHibernate.Caches.Redis/pull/11#issuecomment-70099530

Tazer commented 9 years ago

Great :+1:

Tazer commented 9 years ago

We did run into some problems , when running Microsoft Sessionstate and this.

Getting timeout's when running both, guess it's something with multiple ConnectionMultiplexer .. Where he talks about here: http://stackoverflow.com/a/25946747/42466. And as the Microsoft SessionState Provider is web.config only.. I can't really inject in the current ConnectionMultiplexer.
Not a problem of this library maybe , but a good FYI :gem:

Any suggestions for a good solution ?

TheCloudlessSky commented 9 years ago

Hmm, I haven't seen that before with creating two separate ConnectionMultiplexers.

  1. If you just use one of them, do you experience timeouts?
  2. In the stack trace of the timeout exception, which library is throwing the exception (NHibernate.Caches.Redis or Microsoft SessionState)?
  3. Are you running on Azure (I don't have much experience with it, but I know other folks have had timeouts with Azure)?
  4. If you run one of the projects, do you experience timeouts?
  5. What's the syncTimeout for each library's config?
Tazer commented 9 years ago
  1. No
  2. Will try to reproduce, we got it in production and I didn't save the logs with stack
  3. Nope
  4. No
  5. 2000 for nhib,state: default and retry for 5000

Will try to make some tests on staging tomorrow and see if I can reproduce.

On Wed, 21 Jan 2015 21:43 Adrian Phinney notifications@github.com wrote:

  1. If you just use one of them, do you experience timeouts?
  2. In the stack trace of the timeout exception, which library is throwing the exception (NHibernate.Caches.Redis or Microsoft SessionState)?
  3. Are you running on Azure (I don't have much experience with it, but I know other folks have had timeouts with Azure)?
  4. If you run one of the projects, do you experience timeouts?
  5. What's the syncTimeout for each library's config?

— Reply to this email directly or view it on GitHub https://github.com/TheCloudlessSky/NHibernate.Caches.Redis/pull/8#issuecomment-70917546 .

TheCloudlessSky commented 9 years ago

Ok, I'm going to try running some tests just with multiple ConnectionMultiplexers and see if anything comes up.

TheCloudlessSky commented 9 years ago

I created a really small sample MVC 5 application with both projects and running Redis locally (on Windows) and I don't run into any issues. I'm guessing it's because the Redis instance is local so the latency is incredibly low. You can contact me through my email and I can help you guys debug the problem if necessary.

Tazer commented 9 years ago

Hello,

Sorry for late reply , I Have been on vacation without computer access :)

Think we found our problem. We hade a few REALLY large keys, so sometimes they caused timeouts, havnt seen timeouts after we redid the keys so they are smaller ( less data , more keys )

TheCloudlessSky commented 9 years ago

No worries - hope you had a great vacation!

That's really surprising about the large key size performance. I made a gist to test this locally and I didn't run into any performance issues (even with large key sizes). Were the large key sizes in this library or your own usage?

Tazer commented 9 years ago

We run into issues on reads , and our redis is hosted on another machine. So guess with large keys and network latency (even if it's 10gb between the servers), you will get into problems. But will see if I have some times for test later today.

TheCloudlessSky commented 9 years ago

Ok, I'm pushing the final 2.0 release now. If you run into any other problems, please create a separate issue :smile:.

Tazer commented 9 years ago

Sorry for not getting back, but have been working good since last time! Great! :)

On Thu, 16 Apr 2015 21:16 Adrian Phinney notifications@github.com wrote:

Ok, I'm pushing the final 2.0 release now. If you run into any other problems, please create a separate issue [image: :smile:].

— Reply to this email directly or view it on GitHub https://github.com/TheCloudlessSky/NHibernate.Caches.Redis/pull/8#issuecomment-93818880 .

TheCloudlessSky commented 9 years ago

@Tazer No problem. I'm glad it's working for you. Thanks for all the help you and @MattiasJakobsson did porting this and testing!