marcoCasamento / Hangfire.Redis.StackExchange

HangFire Redis storage based on original (and now unsupported) Hangfire.Redis but using lovely StackExchange.Redis client
Other
456 stars 109 forks source link

Idle performance #35

Closed keimpema closed 7 years ago

keimpema commented 7 years ago

I am seeing 20% cpu usage when Hangfire server is idle when I use Hangfire.Redis.StackExchange. It stays at 0% when using SqlServerStorage. There are no jobs running. Maybe there is a tight loop somewhere?

I am using MSOpenTech Redis for Windows 3.0.504 and nuget packages:

  <package id="Hangfire" version="1.6.7" targetFramework="net462" />
  <package id="Hangfire.Core" version="1.6.7" targetFramework="net462" />
  <package id="Hangfire.Redis.StackExchange" version="1.6.7.3" targetFramework="net462" />
  <package id="Hangfire.SimpleInjector" version="1.3.0.0" targetFramework="net462" />
  <package id="Hangfire.SqlServer" version="1.6.7" targetFramework="net462" />
  <package id="StackExchange.Redis" version="1.1.608" targetFramework="net462" />

Sql Server setup in startup.cs:

            Hangfire.GlobalConfiguration.Configuration.UseSqlServerStorage("Data Source=(localdb)\\ProjectsV13;Initial Catalog=Hangfire;Integrated Security=true;");
            Hangfire.GlobalConfiguration.Configuration.UseActivator(new SimpleInjectorJobActivator(container));
            app.UseHangfireServer();

Redis setup:

            var storage = new RedisStorage("localhost:6379, allowAdmin=true", new RedisStorageOptions { Db = 3 });
            Hangfire.GlobalConfiguration.Configuration.UseStorage(storage);
            Hangfire.GlobalConfiguration.Configuration.UseActivator(new SimpleInjectorJobActivator(container));
            app.UseHangfireServer();

VS performance monitor reports:

Redis profilingreportredis

SqlServer profilingreportsqlserver

marcoCasamento commented 7 years ago

Thanks for signaling in such details. I believe that the problem is inside https://github.com/marcoCasamento/Hangfire.Redis.StackExchange/blob/master/Hangfire.Redis.StackExchange/RedisSubscription.cs#L28.

If you're in a hurry you could try to temporarily add: cancellationToken.WaitHandle.WaitOne(TimeSpan.FromMinutes(3)); at the end of the Execute method, or you could alo try to rollback to 1.6.5. Hope to carefully look at it this evening

marcoCasamento commented 7 years ago

@keimpema any news on this ? did you try something of the above ? I'd also like if @WebApelsin could give his insight on this

WebApelsin commented 7 years ago

Yep, i think this could cause an idle load. The RedisSubscription as IServerComponent is wrapping in a loop inside the BackgroundProcessingServer. The proposed solution from @marcoCasamento should help. The only thought, I'd better add it to the beginning of the Execute method.

On the other hand, we are using this version on the production server under high load and I can't see such issue. Probably, it depends on the used hardware.

keimpema commented 7 years ago

I also tried nuget packages: 1.6.6.42 - same problem 1.6.5.24 - no problem at all

keimpema commented 7 years ago

The 3 minute wait at the beginning of the Execute method works. I was wondering why 3 minutes though? Seems a bit long.

marcoCasamento commented 7 years ago

3 minutes it's totally arbitrary, just a wait long enough to don't cause problems. Every IServerComponent run the execute method in an infinite loop driven by hangfire engine. It's responsibility of the IServerComponent to pause in the exec in order to don't spike loads. RedisSubscription Implements IServerComponent to get notified of the server shutdown and free resources on Redis server Instance.

Putting it at the beginning of the method, as suggested by WebApelSin is the correct place, and no timeout (wait for ever) should be better than the 3 minutes. I'm trying this solution right now, to make sure that the Unsubscribe is called correctly (don't know how to unit test this!) then I'll push the fix.

Finally, I'd like to find a better way to handle the ISubscriber.Unsubscribe, (that use a thread for himself) but didn't find a better way to do it than this one, suggested by WebApelsin last month.

keimpema commented 7 years ago

Thanks for the quick fix!

marcoCasamento commented 7 years ago

You welcome. I believe that the RedisStorage have to become IDisposable, but that's the topic for another issue, I'm going to close this one.

qq1206676756 commented 6 years ago

hello! I am currently experiencing this problem, how can I solve it in the end?