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
38 changes: 36 additions & 2 deletions tools/ci/postgres-partition.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,27 @@ export function packageWeights(entries) {
}
weights.set(pkg, (weights.get(pkg) ?? 0) + entry.measured_seconds);
}
// Packages where EVERY entry is unmeasured. Imputing these from the global
// mean is unbounded: the estimate borrows from unrelated packages, and the
// error is invisible because the imputed value -- not the true one -- is what
// the share guard weighs. Measured: dropping the single 318.8s
// `writer-ownership-canonical-census-pg` (sole member of its package) imputes
// it at 14.3s, 1/209 = 0.48% of entries, and reports `partition ok` while its
// shard really runs 904.1s against a planned 599.6s.
const packagesWithMeasurement = new Set(measured.map((entry) => String(entry.package)));
const unmeasuredPackages = [
...new Set(
workflow
.filter((entry) => !isMeasured(entry))
.map((entry) => String(entry.package))
.filter((pkg) => !packagesWithMeasurement.has(pkg)),
),
].sort();

return {
weights,
unmeasured: [...unmeasured].sort(),
unmeasuredPackages,
imputedSeconds,
measuredCount: measured.length,
};
Expand Down Expand Up @@ -154,13 +172,14 @@ export function packPackages(weights, shardCount) {
* assignment maps package -> shard index.
*/
export function partitionByDuration(entries, shardCount) {
const { weights, unmeasured, imputedSeconds, measuredCount } = packageWeights(entries);
const { weights, unmeasured, unmeasuredPackages, imputedSeconds, measuredCount } =
packageWeights(entries);
const bins = packPackages(weights, shardCount);
const assignment = new Map();
for (const bin of bins) {
for (const pkg of bin.packages) assignment.set(pkg, bin.index);
}
return { assignment, bins, unmeasured, imputedSeconds, measuredCount };
return { assignment, bins, unmeasured, unmeasuredPackages, imputedSeconds, measuredCount };
}

/**
Expand Down Expand Up @@ -235,6 +254,21 @@ export function partitionFailures(entries, shardCount) {
"no workflow entry has a measured_seconds, so there is no basis to impute from",
);
}
// The share guard is denominated in ENTRIES while the cost is SECONDS, and
// the map's seconds are wildly non-uniform -- one entry is worth 318.8s
// against a 15.8s mean. A share of entries therefore cannot bound the error
// a lost measurement introduces. What CAN bound it is refusing to impute
// across a package that retains no measurement of its own: within a package
// the sibling mean is a real estimate, and outside one it is a guess with no
// ceiling.
if (result.unmeasuredPackages?.length) {
failures.push(
`${result.unmeasuredPackages.length} package(s) have no measured entry at all, so their `
+ "weight would be guessed from unrelated packages with no bound on the error; "
+ "regenerate weights before trusting the balance:\n "
+ result.unmeasuredPackages.slice(0, 10).join("\n "),
);
}
}
return failures;
}
Expand Down
37 changes: 37 additions & 0 deletions tools/ci/postgres-partition.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,43 @@ test("postgres-partition", async (t) => {
assert.deepEqual(partitionFailures(entries, 5), []);
});

await t.test("a package that keeps NO measurement fails closed", () => {
// The hole the entry-count share guard cannot close. Cost is seconds and the
// map's seconds are wildly non-uniform, so a share of ENTRIES cannot bound
// the error a lost measurement introduces: on the real map, dropping the
// single 318.8s writer-ownership-canonical-census-pg (sole member of its
// package, 0.48% of entries) imputed it at 14.3s from the global mean and
// reported "partition ok" while its shard really ran 904.1s against a
// planned 599.6s. Within a package the sibling mean is a real estimate;
// across packages it is a guess with no ceiling, so that is what is refused.
const entries = Array.from({ length: 40 }, (_, i) => e(`m${i}`, `pkg${i % 5}`, 10));
entries.push({ name: "lone", package: "solo-pkg", in_workflow_postgres_job: true });
const failures = partitionFailures(entries, 5);
assert.ok(
failures.some((f) => /no measured entry at all/.test(f)),
`expected an unmeasured-package failure, got ${JSON.stringify(failures)}`,
);
assert.ok(failures.some((f) => /solo-pkg/.test(f)), "the offending package must be named");
});

await t.test("a new test in a package that still has measured siblings passes", () => {
// The case imputation exists to allow must keep working: this is exactly
// #802's shape, one new suite joining a package full of measured ones.
const entries = Array.from({ length: 40 }, (_, i) => e(`m${i}`, `pkg${i % 5}`, 10));
entries.push({ name: "brand-new", package: "pkg0", in_workflow_postgres_job: true });
assert.deepEqual(partitionFailures(entries, 5), []);
});

await t.test("packageWeights names the packages that kept no measurement", () => {
const { unmeasuredPackages } = packageWeights([
e("a", "measured-pkg", 10),
{ name: "n", package: "measured-pkg", in_workflow_postgres_job: true },
{ name: "x", package: "blind-pkg", in_workflow_postgres_job: true },
]);
assert.deepEqual(unmeasuredPackages, ["blind-pkg"],
"a package with a measured sibling is bounded; one without is not");
});

await t.test("a drifted map still fails closed", () => {
// The case the guard was actually for: entries that silently lost weights
// they once had. Distinguished from "new test" by share, not by kind.
Expand Down
Loading