-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Singleton Metrics Provider for Listener #285
Changes from all commits
b7e7995
62fca50
309526e
83a9554
47cdece
c0b2720
16e4bf5
349156c
fbb899a
89de7d9
5c161b8
d95d8ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,15 @@ | |
|
||
namespace DurableTask.SqlServer.AzureFunctions | ||
{ | ||
using System; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
public class SqlMetricsProvider | ||
{ | ||
readonly SqlOrchestrationService service; | ||
DateTime metricsTimeStamp = DateTime.MinValue; | ||
SqlScaleMetric? metrics; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added this cache. If the target scaler calls |
||
|
||
public SqlMetricsProvider(SqlOrchestrationService service) | ||
{ | ||
|
@@ -17,13 +20,20 @@ public SqlMetricsProvider(SqlOrchestrationService service) | |
|
||
public virtual async Task<SqlScaleMetric> GetMetricsAsync(int? previousWorkerCount = null) | ||
{ | ||
// GetRecommendedReplicaCountAsync will write a trace if the recommendation results | ||
// in a worker count that is different from the worker count we pass in as an argument. | ||
int recommendedReplicaCount = await this.service.GetRecommendedReplicaCountAsync( | ||
previousWorkerCount, | ||
CancellationToken.None); | ||
// We only want to query the metrics every 5 seconds. | ||
if (this.metrics == null || DateTime.UtcNow >= this.metricsTimeStamp.AddSeconds(5)) | ||
{ | ||
// GetRecommendedReplicaCountAsync will write a trace if the recommendation results | ||
// in a worker count that is different from the worker count we pass in as an argument. | ||
int recommendedReplicaCount = await this.service.GetRecommendedReplicaCountAsync( | ||
previousWorkerCount, | ||
CancellationToken.None); | ||
|
||
return new SqlScaleMetric { RecommendedReplicaCount = recommendedReplicaCount }; | ||
this.metricsTimeStamp = DateTime.UtcNow; | ||
this.metrics = new SqlScaleMetric { RecommendedReplicaCount = recommendedReplicaCount }; | ||
} | ||
|
||
return this.metrics; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,14 +12,12 @@ public class SqlTargetScaler : ITargetScaler | |
{ | ||
readonly SqlMetricsProvider sqlMetricsProvider; | ||
|
||
public SqlTargetScaler(string taskHubName, SqlMetricsProvider sqlMetricsProvider) | ||
public SqlTargetScaler(string functionId, SqlMetricsProvider sqlMetricsProvider) | ||
{ | ||
this.sqlMetricsProvider = sqlMetricsProvider; | ||
|
||
// Scalers in Durable Functions are shared for all functions in the same task hub. | ||
// So instead of using a function ID, we use the task hub name as the basis for the descriptor ID. | ||
string id = $"DurableTask-SqlServer:{taskHubName ?? "default"}"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
this.TargetScalerDescriptor = new TargetScalerDescriptor(id); | ||
// Scalers in Durable Functions is per function ids. And scalers share the same sqlMetricsProvider in the same taskhub. | ||
this.TargetScalerDescriptor = new TargetScalerDescriptor(functionId); | ||
} | ||
|
||
public TargetScalerDescriptor TargetScalerDescriptor { get; } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last thing. I recall the final fix that Alexey made to the Azure Storage metrics provider was to throttle it so that we only fetch metrics once every few seconds or so. This is how we ensured that we don't have too many storage transactions, even if the app has a lot of triggers. I think it will be important to do that for the
SqlMetricsProvider
too so that we don't overwhelm the customer's database with expensive SQL queries.