Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-- Align in_flight_scripts_json to the JSON ARRAY shape it actually carries.
--
-- The column was created (20260417_add_per_target_checkpoint.sql) with DEFAULT
-- '{}'::jsonb and documented as the object form { "<machineId>": "<scriptTicket>" }.
-- That object shape was abandoned: InFlightScriptMap serializes a List<Entry> —
-- an ARRAY keyed by dispatch slot:
-- [{ "m": machineId, "s": stepId, "a": actionId, "t": scriptTicket }]
-- and its Parse() throws (caught → treated as empty) on an object, so '{}' only
-- ever worked via that catch fallback.
--
-- This migration:
-- 1. Re-points the column DEFAULT to the empty array '[]'::jsonb so the default
-- matches the EF model (HasDefaultValue("[]")) and the EnsureExistsAsync seed.
-- 2. Rewrites EXISTING empty '{}' rows to '[]'. An empty object means "no
-- in-flight scripts" — identical in meaning to '[]' — so this is a pure
-- shape normalization with no behavioural change. Non-empty legacy object
-- rows (if any) are left untouched and remain handled by InFlightScriptMap's
-- Parse() catch (re-dispatch fresh), exactly as before.
--
-- Non-breaking: InFlightScriptMap tolerates any leftover legacy shape, and EF
-- always supplies an explicit value on insert, so the DB default is belt-and-braces.

ALTER TABLE deployment_execution_checkpoint
ALTER COLUMN in_flight_scripts_json SET DEFAULT '[]'::jsonb;

UPDATE deployment_execution_checkpoint
SET in_flight_scripts_json = '[]'::jsonb
WHERE in_flight_scripts_json = '{}'::jsonb;
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ public class DeploymentExecutionCheckpoint : IEntity<int>, IAuditable
/// <summary>
/// Tickets of scripts that were dispatched to agents but whose completion was
/// not yet observed. On resume, the server can probe the agent with the same
/// ticket rather than launching a duplicate script. JSON shape:
/// <c>{ "&lt;machineId&gt;": "&lt;scriptTicket&gt;" }</c>
/// ticket rather than launching a duplicate script. JSON shape is the array
/// <c>InFlightScriptMap</c> emits, keyed by dispatch slot (machine + step + action):
/// <c>[{ "m": machineId, "s": stepId, "a": actionId, "t": scriptTicket }]</c>.
/// Empty is the array form <c>[]</c>.
/// </summary>
public string InFlightScriptsJson { get; set; } = "{}";
public string InFlightScriptsJson { get; set; } = "[]";

public DateTimeOffset CreatedDate { get; set; }
public int CreatedBy { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public void Configure(EntityTypeBuilder<DeploymentExecutionCheckpoint> builder)
builder.HasIndex(p => p.ServerTaskId).IsUnique();

builder.Property(p => p.BatchStatesJson).HasColumnType("jsonb").HasDefaultValue("{}");
builder.Property(p => p.InFlightScriptsJson).HasColumnType("jsonb").HasDefaultValue("{}");
// InFlightScriptsJson is a JSON ARRAY (InFlightScriptMap serializes List<Entry>); empty = "[]".
builder.Property(p => p.InFlightScriptsJson).HasColumnType("jsonb").HasDefaultValue("[]");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ await repository.InsertAsync(new DeploymentExecutionCheckpoint
FailureEncountered = false,
OutputVariablesJson = "[]",
BatchStatesJson = "{}",
InFlightScriptsJson = "{}"
InFlightScriptsJson = "[]" // array shape — matches what InFlightScriptMap emits
}, ct).ConfigureAwait(false);

await unitOfWork.SaveChangesAsync(ct).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ public async Task<bool> IsMachineBusyAsync(int machineId, CancellationToken canc
// checkpoint is deleted, so the scanned set is just the active/paused deployments. No per-task
// stripe: this is a stand-alone read on the caller's own (upgrade) scope, not the deploy worker's.
//
// Both IDLE shapes are excluded at the DB layer: "[]" (an array emptied after add-then-remove)
// AND "{}" (the value EnsureExistsAsync seeds + the column default) — a checkpoint that exists
// but has never dispatched, or is between batches, carries "{}". Filtering both keeps the scan
// tight to checkpoints that genuinely hold an in-flight entry. (ContainsMachine would return
// false for either idle shape anyway via its parse-fallback, so this is a narrowing, not a
// correctness fix.)
// Both IDLE shapes are excluded at the DB layer: "[]" (the current seed + column default, and an
// array emptied after add-then-remove) AND "{}" (the LEGACY object default, kept defensively for
// rows written by older servers / not yet normalized by 20260615_align_inflight_scripts_default_
// to_array.sql). Filtering both keeps the scan tight to checkpoints that genuinely hold an
// in-flight entry. (ContainsMachine would return false for either idle shape anyway via its
// parse-fallback, so this is a narrowing, not a correctness fix.)
var jsons = await repository.QueryNoTracking<DeploymentExecutionCheckpoint>(
c => c.InFlightScriptsJson != null && c.InFlightScriptsJson != "[]" && c.InFlightScriptsJson != "{}")
.Select(c => c.InFlightScriptsJson)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Squid.Core.Persistence.Db;
using Squid.Core.Persistence.Entities.Deployments;
using Squid.Core.Services.Deployments.Checkpoints;
using Squid.IntegrationTests.Base;
Expand Down Expand Up @@ -275,18 +277,77 @@ public async Task IsMachineBusy_NoCheckpointForMachine_ReturnsFalse()
[Fact]
public async Task IsMachineBusy_CheckpointSeededButNeverDispatched_ReturnsFalse()
{
// A checkpoint that EXISTS but has not (yet) dispatched any script carries the idle
// seed value "{}" (EnsureExistsAsync). It must report NOT busy — and the scan predicate
// now excludes BOTH idle shapes ("[]" and "{}") at the DB layer, so such rows are never
// even materialized into the busy-check.
// A checkpoint that EXISTS but has not (yet) dispatched any script carries the empty-array
// seed "[]" (EnsureExistsAsync). It must report NOT busy — the scan predicate excludes the
// empty array at the DB layer (and still excludes the legacy "{}" defensively), so such rows
// are never even materialized into the busy-check.
const int taskId = 700024;
const int machineId = 9106;
await EnsureRowAsync(taskId).ConfigureAwait(false);

(await IsBusyAsync(machineId).ConfigureAwait(false)).ShouldBeFalse(
customMessage: "A seeded-but-never-dispatched checkpoint ('{}') must not report any machine busy.");
customMessage: "A seeded-but-never-dispatched checkpoint ('[]') must not report any machine busy.");
}

[Fact]
public async Task EnsureExists_SeedsInFlightScriptsJson_AsEmptyArray()
{
// Discriminating pin for the data-shape alignment: EnsureExistsAsync must seed the in-flight
// column as the empty ARRAY "[]" that InFlightScriptMap emits — NOT the legacy object "{}".
// This makes the IsMachineBusyAsync pre-filter ("!= []") exact for fresh checkpoints rather
// than relying on the parse-fallback for "{}".
const int taskId = 700031;
await EnsureRowAsync(taskId).ConfigureAwait(false);

var row = await LoadAsync(taskId).ConfigureAwait(false);

row.InFlightScriptsJson.ShouldBe("[]",
customMessage: "the seed must be the empty array shape '[]' matching InFlightScriptMap, not the legacy object '{}'.");
}

[Fact]
public async Task Migration_NormalizesEmptyLegacyObject_ButLeavesNonEmptyLegacyObjectUntouched()
{
// Pins the 20260615 migration's UPDATE clause: rewrite the empty legacy object '{}' to the
// array '[]', but NEVER clobber a NON-EMPTY legacy object (a pre-#433 {machineId:ticket} row that
// still carries data). The integration harness migrates a FRESH (empty) DB so the UPDATE is a
// no-op there — exercise it explicitly by seeding both legacy shapes, then running the same UPDATE.
const int emptyTaskId = 700040;
const int nonEmptyTaskId = 700041;

await Run<IRepository, IUnitOfWork>(async (repo, uow) =>
{
await repo.InsertAsync(NewCheckpoint(emptyTaskId, "{}")).ConfigureAwait(false);
await repo.InsertAsync(NewCheckpoint(nonEmptyTaskId, "{\"11\":\"legacy-ticket\"}")).ConfigureAwait(false);
await uow.SaveChangesAsync().ConfigureAwait(false);
}).ConfigureAwait(false);

// Mirrors the UPDATE in 20260615_align_inflight_scripts_default_to_array.sql. Parameterized
// ({0}/{1} are bound params, the [] / {} values pass as text cast to jsonb) — semantically the
// same as the migration's literal '[]'::jsonb / '{}'::jsonb, but avoids ExecuteSqlRaw treating
// the literal braces as String.Format placeholders.
await Run<DbContext>(ctx => ctx.Database.ExecuteSqlRawAsync(
"UPDATE deployment_execution_checkpoint SET in_flight_scripts_json = {0}::jsonb WHERE in_flight_scripts_json = {1}::jsonb",
"[]", "{}")).ConfigureAwait(false);

(await LoadAsync(emptyTaskId).ConfigureAwait(false)).InFlightScriptsJson.ShouldBe("[]",
customMessage: "the migration must normalize an empty legacy '{}' to the array '[]'.");

(await LoadAsync(nonEmptyTaskId).ConfigureAwait(false)).InFlightScriptsJson.ShouldContain("legacy-ticket",
customMessage: "the migration's WHERE = '{}' must NOT clobber a non-empty legacy object row.");
}

private static DeploymentExecutionCheckpoint NewCheckpoint(int serverTaskId, string inFlightScriptsJson) => new()
{
ServerTaskId = serverTaskId,
DeploymentId = 1,
LastCompletedBatchIndex = -1,
FailureEncountered = false,
OutputVariablesJson = "[]",
BatchStatesJson = "{}",
InFlightScriptsJson = inFlightScriptsJson
};

[Fact]
public async Task DependencyInjection_ResolvesRealInFlightScriptStore_SoTheUpgradeDeferGuardStaysWired()
{
Expand Down
Loading