perf(siteruntime): projected single-row deployed-config reads (P1-4)
HandleEnsureInstanceStarted, HandleDeploymentStateQuery, and HandleEnable each called GetAllDeployedConfigsAsync — materializing every deployed config, including every ConfigJson blob — just to FirstOrDefault one row. A volume-restore reconcile healing hundreds of instances turned that into hundreds of full-table reads. Add SiteStorageService.GetDeployedConfigAsync(name) (single-row, full projection incl. ConfigJson, for callers that (re)start an Instance Actor) and GetDeployedConfigMetadataAsync(name) (single-row, ConfigJson-free projection, for callers that only need DeploymentId/RevisionHash/IsEnabled). Switch the three call sites; leave decisions/semantics identical. HandleEnsureInstanceStarted gets its own _deployedConfigLoader seam, deliberately separate from the existing _configLoader (still used by the boot-time load, which legitimately needs every row) so a test overriding one cannot accidentally gate the other. Generation fencing (StoreGenerationAtRead, Medium wave T1) is untouched: the generation is still captured on the actor thread before the read dispatches. Tests: new SiteStorageServiceTests lock in that both projections return the same values GetAllDeployedConfigsAsync().FirstOrDefault(...) did. The three row-101 ensure-started race tests (ReconcileStartsInstanceTests) are ported to the new seam and still pass, proving generation fencing survives the projection. Proof of failure: since this is a pure I/O-cost fix with no observable behavior change, the new/changed tests were confirmed to fail to even compile against the unfixed code (stash/unstash) — GetDeployedConfigAsync, GetDeployedConfigMetadataAsync, and the deployedConfigLoader ctor param don't exist there.
This commit is contained in:
@@ -82,6 +82,21 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
/// </summary>
|
||||
private readonly Func<Task<List<DeployedInstance>>> _configLoader;
|
||||
/// <summary>
|
||||
/// Reads a single deployed config row by instance name. Defaults to
|
||||
/// <see cref="SiteStorageService.GetDeployedConfigAsync"/>; overridable via the ctor so a
|
||||
/// test can hold the read open mid-flight (row 101's ensure-started race tests).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Register P1-4: the row-67 ensure-started heal used to read through
|
||||
/// <see cref="_configLoader"/> — the same full-table-including-ConfigJson load the boot
|
||||
/// sequence uses — just to <c>FirstOrDefault</c> one instance. A volume-restore reconcile
|
||||
/// that raises hundreds of <see cref="EnsureInstanceStartedCommand"/>s in a row turned that
|
||||
/// into hundreds of full-table reads. This seam is deliberately separate from
|
||||
/// <see cref="_configLoader"/>: the two are read at different cardinalities (all rows for
|
||||
/// boot, one row per heal) and a test overriding one must not accidentally gate the other.
|
||||
/// </remarks>
|
||||
private readonly Func<string, Task<DeployedInstance?>> _deployedConfigLoader;
|
||||
/// <summary>
|
||||
/// Reads the durable <c>data_connection_definitions</c> rows (deferred register rows 89 + 90).
|
||||
/// Defaults to <see cref="SiteStorageService.GetAllDataConnectionDefinitionsAsync"/>;
|
||||
/// overridable via the ctor so a test can drive a read failure.
|
||||
@@ -276,6 +291,11 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
/// defaults to reading from <paramref name="storage"/>. Primarily for tests that need to
|
||||
/// drive a read failure.
|
||||
/// </param>
|
||||
/// <param name="deployedConfigLoader">
|
||||
/// Register P1-4: optional override for the single-row deployed-config read the row-67
|
||||
/// ensure-started heal uses; defaults to reading from <paramref name="storage"/>. Primarily
|
||||
/// for tests that need to hold the read open mid-flight to drive the row-101 race.
|
||||
/// </param>
|
||||
public DeploymentManagerActor(
|
||||
SiteStorageService storage,
|
||||
ScriptCompilationService compilationService,
|
||||
@@ -292,7 +312,8 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
Func<Task<List<DeployedInstance>>>? configLoader = null,
|
||||
ExternalSystemDefinitionCache? externalSystemCache = null,
|
||||
ScriptExecutionScheduler? scriptScheduler = null,
|
||||
Func<Task<List<StoredDataConnectionDefinition>>>? connectionDefinitionLoader = null)
|
||||
Func<Task<List<StoredDataConnectionDefinition>>>? connectionDefinitionLoader = null,
|
||||
Func<string, Task<DeployedInstance?>>? deployedConfigLoader = null)
|
||||
{
|
||||
_scriptScheduler = scriptScheduler;
|
||||
_storage = storage;
|
||||
@@ -311,6 +332,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
? TimeSpan.FromSeconds(options.InstanceSweepIntervalSeconds)
|
||||
: TimeSpan.Zero;
|
||||
_configLoader = configLoader ?? (() => _storage.GetAllDeployedConfigsAsync());
|
||||
_deployedConfigLoader = deployedConfigLoader ?? (name => _storage.GetDeployedConfigAsync(name));
|
||||
_connectionDefinitionLoader = connectionDefinitionLoader
|
||||
?? (() => _storage.GetAllDataConnectionDefinitionsAsync());
|
||||
_logger = logger;
|
||||
@@ -1502,8 +1524,9 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
Task.Run(async () =>
|
||||
{
|
||||
await _storage.SetInstanceEnabledAsync(instanceName, true);
|
||||
var configs = await _storage.GetAllDeployedConfigsAsync();
|
||||
var config = configs.FirstOrDefault(c => c.InstanceUniqueName == instanceName);
|
||||
// Row P1-4: a single-row read — this used to materialize every deployed config
|
||||
// (including every ConfigJson blob) just to FirstOrDefault this one instance.
|
||||
var config = await _storage.GetDeployedConfigAsync(instanceName);
|
||||
return new EnableResult(command, config, null, sender);
|
||||
}).ContinueWith(t =>
|
||||
{
|
||||
@@ -1595,12 +1618,11 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
var generationAtRead = _storeWriteGeneration;
|
||||
Task.Run(async () =>
|
||||
{
|
||||
// Read through the same seam the boot load and the sweep read through
|
||||
// (_configLoader defaults to _storage.GetAllDeployedConfigsAsync, so this is the
|
||||
// identical production call), which lets a test hold the read open and land the
|
||||
// result behind a committed disable/delete.
|
||||
var configs = await _configLoader();
|
||||
var config = configs.FirstOrDefault(c => c.InstanceUniqueName == instanceName);
|
||||
// Register P1-4: read a single row through _deployedConfigLoader (defaults to
|
||||
// _storage.GetDeployedConfigAsync — a WHERE instance_unique_name = @name read, not
|
||||
// the full-table-including-ConfigJson load the boot sequence uses), which still lets
|
||||
// a test hold the read open and land the result behind a committed disable/delete.
|
||||
var config = await _deployedConfigLoader(instanceName);
|
||||
return new EnsureInstanceStartedResult(command, config, null, generationAtRead);
|
||||
}).ContinueWith(t => t.IsCompletedSuccessfully
|
||||
? t.Result
|
||||
@@ -2095,7 +2117,10 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
var sender = Sender;
|
||||
var instanceName = request.InstanceUniqueName;
|
||||
|
||||
_storage.GetAllDeployedConfigsAsync().ContinueWith(t =>
|
||||
// Row P1-4: a single-row, ConfigJson-free projection — this used to materialize every
|
||||
// deployed config (including every ConfigJson blob) just to FirstOrDefault this one
|
||||
// instance's DeploymentId/RevisionHash. Neither is ever read here.
|
||||
_storage.GetDeployedConfigMetadataAsync(instanceName).ContinueWith(t =>
|
||||
{
|
||||
if (!t.IsCompletedSuccessfully)
|
||||
{
|
||||
@@ -2110,7 +2135,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
|
||||
request.CorrelationId, instanceName, false, null, null, DateTimeOffset.UtcNow);
|
||||
}
|
||||
|
||||
var config = t.Result.FirstOrDefault(c => c.InstanceUniqueName == instanceName);
|
||||
var config = t.Result;
|
||||
return config == null
|
||||
? new DeploymentStateQueryResponse(
|
||||
request.CorrelationId, instanceName, false, null, null, DateTimeOffset.UtcNow)
|
||||
|
||||
@@ -108,6 +108,89 @@ public class SiteStorageService
|
||||
return results;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the single deployed instance configuration for <paramref name="instanceUniqueName"/>,
|
||||
/// or <see langword="null"/> if no row exists for it.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Register P1-4: <see cref="GetAllDeployedConfigsAsync"/> plus a <c>FirstOrDefault</c> was
|
||||
/// materializing every deployed config — including every <c>ConfigJson</c> blob — just to find
|
||||
/// one row. A volume-restore reconcile healing hundreds of instances turned that into hundreds
|
||||
/// of full-table reads. Callers that need the full row (including <c>ConfigJson</c>) for exactly
|
||||
/// one instance — e.g. to (re)start its Instance Actor — should use this instead. Callers that
|
||||
/// only need identity/staleness metadata should use <see cref="GetDeployedConfigMetadataAsync"/>,
|
||||
/// which never touches the <c>config_json</c> column.
|
||||
/// </remarks>
|
||||
/// <param name="instanceUniqueName">The unique name of the instance.</param>
|
||||
/// <returns>
|
||||
/// A task that resolves to the deployed configuration row, or <see langword="null"/> if none
|
||||
/// exists for the instance.
|
||||
/// </returns>
|
||||
public async Task<DeployedInstance?> GetDeployedConfigAsync(string instanceUniqueName)
|
||||
{
|
||||
await using var connection = OpenConnection();
|
||||
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText = @"
|
||||
SELECT instance_unique_name, config_json, deployment_id, revision_hash, is_enabled, deployed_at
|
||||
FROM deployed_configurations
|
||||
WHERE instance_unique_name = @name
|
||||
LIMIT 1";
|
||||
command.Parameters.AddWithValue("@name", instanceUniqueName);
|
||||
|
||||
await using var reader = await command.ExecuteReaderAsync();
|
||||
if (!await reader.ReadAsync())
|
||||
return null;
|
||||
|
||||
return new DeployedInstance
|
||||
{
|
||||
InstanceUniqueName = reader.GetString(0),
|
||||
ConfigJson = reader.GetString(1),
|
||||
DeploymentId = reader.GetString(2),
|
||||
RevisionHash = reader.GetString(3),
|
||||
IsEnabled = reader.GetInt64(4) != 0,
|
||||
DeployedAt = reader.GetString(5)
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the deployment-identity metadata for <paramref name="instanceUniqueName"/> —
|
||||
/// <c>(name, deployment id, revision hash, is_enabled)</c> — without reading the
|
||||
/// <c>config_json</c> column, or <see langword="null"/> if no row exists for it.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Register P1-4: for a caller that only needs staleness/identity metadata (e.g. the
|
||||
/// central deployment-state query, which reports only <c>DeploymentId</c>/<c>RevisionHash</c>),
|
||||
/// reading the <c>config_json</c> blob is pure waste. This projection is that lighter read.
|
||||
/// </remarks>
|
||||
/// <param name="instanceUniqueName">The unique name of the instance.</param>
|
||||
/// <returns>
|
||||
/// A task that resolves to the metadata row, or <see langword="null"/> if none exists for the
|
||||
/// instance.
|
||||
/// </returns>
|
||||
public async Task<DeployedConfigMetadata?> GetDeployedConfigMetadataAsync(string instanceUniqueName)
|
||||
{
|
||||
await using var connection = OpenConnection();
|
||||
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText = @"
|
||||
SELECT instance_unique_name, deployment_id, revision_hash, is_enabled
|
||||
FROM deployed_configurations
|
||||
WHERE instance_unique_name = @name
|
||||
LIMIT 1";
|
||||
command.Parameters.AddWithValue("@name", instanceUniqueName);
|
||||
|
||||
await using var reader = await command.ExecuteReaderAsync();
|
||||
if (!await reader.ReadAsync())
|
||||
return null;
|
||||
|
||||
return new DeployedConfigMetadata(
|
||||
InstanceUniqueName: reader.GetString(0),
|
||||
DeploymentId: reader.GetString(1),
|
||||
RevisionHash: reader.GetString(2),
|
||||
IsEnabled: reader.GetInt64(3) != 0);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Stores or updates a deployed instance configuration. Uses UPSERT semantics.
|
||||
/// </summary>
|
||||
@@ -939,6 +1022,22 @@ public class DeployedInstance
|
||||
public string DeployedAt { get; init; } = string.Empty;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Deployment-identity metadata for one deployed instance — the projection of
|
||||
/// <see cref="DeployedInstance"/> that omits <c>ConfigJson</c>. Returned by
|
||||
/// <see cref="SiteStorageService.GetDeployedConfigMetadataAsync"/> for callers that only need to
|
||||
/// answer "is this instance deployed, and at what revision" (register P1-4).
|
||||
/// </summary>
|
||||
/// <param name="InstanceUniqueName">The unique name of the instance.</param>
|
||||
/// <param name="DeploymentId">The unique deployment identifier.</param>
|
||||
/// <param name="RevisionHash">The configuration revision hash for staleness detection.</param>
|
||||
/// <param name="IsEnabled">Whether the instance is enabled.</param>
|
||||
public record DeployedConfigMetadata(
|
||||
string InstanceUniqueName,
|
||||
string DeploymentId,
|
||||
string RevisionHash,
|
||||
bool IsEnabled);
|
||||
|
||||
/// <summary>
|
||||
/// Represents a shared script stored locally in SQLite.
|
||||
/// </summary>
|
||||
|
||||
Reference in New Issue
Block a user