Skip to content

Commit

Permalink
Cache results of the IsBatchingAvaiable method per connection type
Browse files Browse the repository at this point in the history
  • Loading branch information
odinserj committed Feb 18, 2019
1 parent 4748d7e commit 6dc93e8
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 32 deletions.
39 changes: 23 additions & 16 deletions src/Hangfire.Core/Server/DelayedJobScheduler.cs
Expand Up @@ -15,6 +15,7 @@
// License along with Hangfire. If not, see <http://www.gnu.org/licenses/>.

using System;
using System.Collections.Concurrent;
using Hangfire.Annotations;
using Hangfire.Common;
using Hangfire.Logging;
Expand Down Expand Up @@ -72,6 +73,7 @@ public class DelayedJobScheduler : IBackgroundProcess
private static readonly int BatchSize = 1000;

private readonly ILog _logger = LogProvider.For<DelayedJobScheduler>();
private readonly ConcurrentDictionary<Type, bool> _isBatchingAvailableCache = new ConcurrentDictionary<Type, bool>();

private readonly IBackgroundJobStateChanger _stateChanger;
private readonly TimeSpan _pollingDelay;
Expand Down Expand Up @@ -201,25 +203,30 @@ private void EnqueueBackgroundJob(BackgroundProcessContext context, IStorageConn
}
}

private static bool IsBatchingAvailable(IStorageConnection connection)
private bool IsBatchingAvailable(IStorageConnection connection)
{
if (connection is JobStorageConnection storageConnection)
{
try
{
storageConnection.GetFirstByLowestScoreFromSet(null, 0, 0, 1);
}
catch (ArgumentNullException ex) when (ex.ParamName == "key")
{
return true;
}
catch (Exception)
return _isBatchingAvailableCache.GetOrAdd(
connection.GetType(),
type =>
{
//
}
}
if (connection is JobStorageConnection storageConnection)
{
try
{
storageConnection.GetFirstByLowestScoreFromSet(null, 0, 0, 1);
}
catch (ArgumentNullException ex) when (ex.ParamName == "key")
{
return true;
}
catch (Exception)
{
//
}
}
return false;
return false;
});
}

private T UseConnectionDistributedLock<T>(JobStorage storage, Func<IStorageConnection, T> action)
Expand Down
39 changes: 23 additions & 16 deletions src/Hangfire.Core/Server/RecurringJobScheduler.cs
Expand Up @@ -15,6 +15,7 @@
// License along with Hangfire. If not, see <http://www.gnu.org/licenses/>.

using System;
using System.Collections.Concurrent;
using Hangfire.Annotations;
using Hangfire.Client;
using Hangfire.Common;
Expand Down Expand Up @@ -66,6 +67,7 @@ public class RecurringJobScheduler : IBackgroundProcess
private static readonly int BatchSize = 1000;

private readonly ILog _logger = LogProvider.For<RecurringJobScheduler>();
private readonly ConcurrentDictionary<Type, bool> _isBatchingAvailableCache = new ConcurrentDictionary<Type, bool>();

private readonly IBackgroundJobFactory _factory;
private readonly IStateMachine _stateMachine;
Expand Down Expand Up @@ -362,25 +364,30 @@ private bool UseConnectionDistributedLock(JobStorage storage, Func<IStorageConne
return false;
}

private static bool IsBatchingAvailable(IStorageConnection connection)
private bool IsBatchingAvailable(IStorageConnection connection)
{
if (connection is JobStorageConnection storageConnection)
{
try
return _isBatchingAvailableCache.GetOrAdd(
connection.GetType(),
type =>
{
storageConnection.GetFirstByLowestScoreFromSet(null, 0, 0, 1);
}
catch (ArgumentNullException ex) when (ex.ParamName == "key")
{
return true;
}
catch (Exception)
{
//
}
}
if (connection is JobStorageConnection storageConnection)
{
try
{
storageConnection.GetFirstByLowestScoreFromSet(null, 0, 0, 1);
}
catch (ArgumentNullException ex) when (ex.ParamName == "key")
{
return true;
}
catch (Exception)
{
//
}
}
return false;
return false;
});
}
}
}

2 comments on commit 6dc93e8

@burningice2866
Copy link
Contributor

@burningice2866 burningice2866 commented on 6dc93e8 Feb 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about making it as a IsBatchingAvailable-extension method on IStorageConnection, still with a cache but without any duplicated logic.

@odinserj
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be hard to cache the results. If we made caching dictionary static, it will be hard to set up the mocks for unit testing. I don't believe there will be other usages of this method in the near future and I think it will be better to leave this as is and wait for the third duplication.

Please sign in to comment.