fix: Replace IReadOnlySet/IReadOnlyDictionary with HashSet/Dictionary for performance (CA1859) and use LoggerMessage delegates (CA1848)
Source: CLAUDE.md observability section - Serilog structured logging and performance are first-class concerns Slice: ModelOperations/Scheduling, Domain/ModelFeedbackCycle Policy: CA1859 (concrete types over interfaces), CA1848 (LoggerMessage delegates) Changes: - ScheduledModelOperationJob: LoggerMessage.Define for warning/info logs - ModelOperationsDispatcherJob: LoggerMessage.Define for error logs - ModelOperationExecution: Dictionary<State, HashSet<State>> state machine - ModelFeedbackCycle: Dictionary<State, HashSet<State>> state machine Verification: - dotnet build: 0 errors, 0 warnings - dotnet test: 41/41 tests passed (ArchitectureTests 5, ModelOperations 17, SignalEngine 18) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,7 +6,7 @@
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
|
||||
<AnalysisLevel>latest-recommended</AnalysisLevel>
|
||||
<NoWarn>$(NoWarn);CA1859;CA1822;CA1848;CA1873;CA1305;CA1707;CA1861;xUnit2031</NoWarn>
|
||||
<NoWarn>$(NoWarn);CA1822;CA1873;CA1305;CA1707;CA1861;xUnit2031</NoWarn>
|
||||
<Deterministic>true</Deterministic>
|
||||
<ContinuousIntegrationBuild Condition="'$(CI)' == 'true'">true</ContinuousIntegrationBuild>
|
||||
</PropertyGroup>
|
||||
|
||||
@@ -19,15 +19,15 @@ public sealed record ModelOperationExecutionTransition(
|
||||
|
||||
public sealed class ModelOperationExecution
|
||||
{
|
||||
private static readonly IReadOnlyDictionary<ModelOperationExecutionState, IReadOnlySet<ModelOperationExecutionState>> Allowed =
|
||||
new Dictionary<ModelOperationExecutionState, IReadOnlySet<ModelOperationExecutionState>>
|
||||
private static readonly Dictionary<ModelOperationExecutionState, HashSet<ModelOperationExecutionState>> Allowed =
|
||||
new()
|
||||
{
|
||||
[ModelOperationExecutionState.Requested] = Set(ModelOperationExecutionState.Running, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Quarantined),
|
||||
[ModelOperationExecutionState.Running] = Set(ModelOperationExecutionState.Succeeded, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Failed, ModelOperationExecutionState.Quarantined),
|
||||
[ModelOperationExecutionState.BusinessHold] = Set(ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined),
|
||||
[ModelOperationExecutionState.Failed] = Set(ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined),
|
||||
[ModelOperationExecutionState.Succeeded] = new HashSet<ModelOperationExecutionState>(),
|
||||
[ModelOperationExecutionState.Quarantined] = new HashSet<ModelOperationExecutionState>()
|
||||
[ModelOperationExecutionState.Requested] = new(new[] { ModelOperationExecutionState.Running, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Quarantined }),
|
||||
[ModelOperationExecutionState.Running] = new(new[] { ModelOperationExecutionState.Succeeded, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Failed, ModelOperationExecutionState.Quarantined }),
|
||||
[ModelOperationExecutionState.BusinessHold] = new(new[] { ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined }),
|
||||
[ModelOperationExecutionState.Failed] = new(new[] { ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined }),
|
||||
[ModelOperationExecutionState.Succeeded] = new(),
|
||||
[ModelOperationExecutionState.Quarantined] = new()
|
||||
};
|
||||
|
||||
private readonly List<ModelOperationExecutionTransition> transitions = [];
|
||||
@@ -63,5 +63,5 @@ public sealed class ModelOperationExecution
|
||||
return transition;
|
||||
}
|
||||
|
||||
private static IReadOnlySet<ModelOperationExecutionState> Set(params ModelOperationExecutionState[] states) => new HashSet<ModelOperationExecutionState>(states);
|
||||
private static HashSet<ModelOperationExecutionState> Set(params ModelOperationExecutionState[] states) => new(states);
|
||||
}
|
||||
|
||||
@@ -24,19 +24,19 @@ public sealed record ModelFeedbackTransition(
|
||||
|
||||
public sealed class ModelFeedbackCycle
|
||||
{
|
||||
private static readonly IReadOnlyDictionary<ModelFeedbackCycleState, IReadOnlySet<ModelFeedbackCycleState>> Allowed =
|
||||
new Dictionary<ModelFeedbackCycleState, IReadOnlySet<ModelFeedbackCycleState>>
|
||||
private static readonly Dictionary<ModelFeedbackCycleState, HashSet<ModelFeedbackCycleState>> Allowed =
|
||||
new()
|
||||
{
|
||||
[ModelFeedbackCycleState.Planned] = Set(ModelFeedbackCycleState.PredictionFrozen, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.PredictionFrozen] = Set(ModelFeedbackCycleState.OutcomesMaturing, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.OutcomesMaturing] = Set(ModelFeedbackCycleState.Evaluated, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.Evaluated] = Set(ModelFeedbackCycleState.ImprovementProposed, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.ImprovementProposed] = Set(ModelFeedbackCycleState.ChallengerPlanned, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.ChallengerPlanned] = Set(ModelFeedbackCycleState.IndependentlyValidated, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.IndependentlyValidated] = Set(ModelFeedbackCycleState.PromotionReviewPending, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.PromotionReviewPending] = Set(ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold),
|
||||
[ModelFeedbackCycleState.BusinessHold] = Set(ModelFeedbackCycleState.Planned, ModelFeedbackCycleState.Closed),
|
||||
[ModelFeedbackCycleState.Closed] = new HashSet<ModelFeedbackCycleState>()
|
||||
[ModelFeedbackCycleState.Planned] = new(new[] { ModelFeedbackCycleState.PredictionFrozen, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.PredictionFrozen] = new(new[] { ModelFeedbackCycleState.OutcomesMaturing, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.OutcomesMaturing] = new(new[] { ModelFeedbackCycleState.Evaluated, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.Evaluated] = new(new[] { ModelFeedbackCycleState.ImprovementProposed, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.ImprovementProposed] = new(new[] { ModelFeedbackCycleState.ChallengerPlanned, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.ChallengerPlanned] = new(new[] { ModelFeedbackCycleState.IndependentlyValidated, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.IndependentlyValidated] = new(new[] { ModelFeedbackCycleState.PromotionReviewPending, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.PromotionReviewPending] = new(new[] { ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }),
|
||||
[ModelFeedbackCycleState.BusinessHold] = new(new[] { ModelFeedbackCycleState.Planned, ModelFeedbackCycleState.Closed }),
|
||||
[ModelFeedbackCycleState.Closed] = new()
|
||||
};
|
||||
|
||||
private readonly List<ModelFeedbackTransition> transitions = [];
|
||||
@@ -89,6 +89,6 @@ public sealed class ModelFeedbackCycle
|
||||
return transition;
|
||||
}
|
||||
|
||||
private static IReadOnlySet<ModelFeedbackCycleState> Set(params ModelFeedbackCycleState[] states)
|
||||
=> new HashSet<ModelFeedbackCycleState>(states);
|
||||
private static HashSet<ModelFeedbackCycleState> Set(params ModelFeedbackCycleState[] states)
|
||||
=> new(states);
|
||||
}
|
||||
|
||||
@@ -15,6 +15,12 @@ public sealed class ModelOperationsDispatcherJob(
|
||||
ScheduleOccurrencePlanner occurrencePlanner,
|
||||
ILogger<ModelOperationsDispatcherJob> logger)
|
||||
{
|
||||
private static readonly Action<ILogger, string, string, Exception?> LogDispatchFailed =
|
||||
LoggerMessage.Define<string, string>(
|
||||
LogLevel.Error,
|
||||
new EventId(1, nameof(LogDispatchFailed)),
|
||||
"Failed to dispatch model operation {OperationCode} for {ScopeKey}.");
|
||||
|
||||
[Queue("q-control")]
|
||||
[DisableConcurrentExecution(timeoutInSeconds: 840)]
|
||||
[AutomaticRetry(Attempts = 0, OnAttemptsExceeded = AttemptsExceededAction.Delete)]
|
||||
@@ -47,7 +53,7 @@ public sealed class ModelOperationsDispatcherJob(
|
||||
}
|
||||
catch (Exception exception)
|
||||
{
|
||||
logger.LogError(exception, "Failed to dispatch model operation {OperationCode} for {ScopeKey}.", item.OperationCode, item.ScopeKey);
|
||||
LogDispatchFailed(logger, item.OperationCode, item.ScopeKey, exception);
|
||||
await schedules.ReleaseAsync(
|
||||
item.ScheduleId,
|
||||
leaseOwner,
|
||||
|
||||
@@ -8,6 +8,18 @@ public sealed class ScheduledModelOperationJob(
|
||||
IModelOperationRequestService service,
|
||||
ILogger<ScheduledModelOperationJob> logger)
|
||||
{
|
||||
private static readonly Action<ILogger, string, string, Exception?> LogModelOperationNotCreated =
|
||||
LoggerMessage.Define<string, string>(
|
||||
LogLevel.Warning,
|
||||
new EventId(1, nameof(LogModelOperationNotCreated)),
|
||||
"Model operation {OperationCode} for {ScopeKey} was not created because the approved frozen context is unavailable or the idempotency key already exists.");
|
||||
|
||||
private static readonly Action<ILogger, string, string, string, string, Exception?> LogModelOperationRequested =
|
||||
LoggerMessage.Define<string, string, string, string>(
|
||||
LogLevel.Information,
|
||||
new EventId(2, nameof(LogModelOperationRequested)),
|
||||
"Requested model operation {OperationCode} for {ScopeKey} with model {ModelVersion} and dataset {DatasetId}. No model mutation is performed by this job.");
|
||||
|
||||
[AutomaticRetry(Attempts = 3, OnAttemptsExceeded = AttemptsExceededAction.Fail)]
|
||||
public async Task ExecuteAsync(
|
||||
Guid scheduleId,
|
||||
@@ -28,18 +40,16 @@ public sealed class ScheduledModelOperationJob(
|
||||
|
||||
if (request is null)
|
||||
{
|
||||
logger.LogWarning(
|
||||
"Model operation {OperationCode} for {ScopeKey} was not created because the approved frozen context is unavailable or the idempotency key already exists.",
|
||||
operationCode,
|
||||
scopeKey);
|
||||
LogModelOperationNotCreated(logger, operationCode, scopeKey, null);
|
||||
return;
|
||||
}
|
||||
|
||||
logger.LogInformation(
|
||||
"Requested model operation {OperationCode} for {ScopeKey} with model {ModelVersion} and dataset {DatasetId}. No model mutation is performed by this job.",
|
||||
LogModelOperationRequested(
|
||||
logger,
|
||||
request.OperationCode,
|
||||
request.ScopeKey,
|
||||
request.Context.VersionSet.ModelVersion,
|
||||
request.Context.VersionSet.DatasetId);
|
||||
request.Context.VersionSet.DatasetId,
|
||||
null);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user