Skip to content

Commit dfafcbe

Browse files
committed
fix: restore merge dedup and relax deps resume guards (CM-1296)
Signed-off-by: Uroš Marolt <uros@marolt.me>
1 parent 18f79ef commit dfafcbe

4 files changed

Lines changed: 37 additions & 11 deletions

File tree

services/apps/packages_worker/src/deps-dev/workflows/bootstrapOsspckgs.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@ export async function bootstrapOsspckgs(opts: {
9393
const activeKinds = opts.kinds ? new Set(opts.kinds) : null
9494
const runs = (kind: string) => !activeKinds || activeKinds.has(kind)
9595

96+
// Resume reuses a prior package_dependencies export and skips watermark/partition validation.
97+
// Hard-enforce it targets ONLY package_dependencies so a stray resumeJobId can't silently run other
98+
// kinds without their safety checks. The CLI validates this too; this is the fail-fast backstop.
99+
if (
100+
resume &&
101+
!(activeKinds && activeKinds.size === 1 && activeKinds.has('package_dependencies'))
102+
) {
103+
throw new ApplicationFailure(
104+
'resumeJobId is only valid with kinds=[package_dependencies] — refusing to skip validation for other kinds',
105+
)
106+
}
107+
96108
const jobKinds: JobKind[] = (
97109
['packages', 'versions', 'package_dependencies', 'advisories', 'advisory_packages'] as JobKind[]
98110
).filter((k) => runs(k))

services/apps/packages_worker/src/deps-dev/workflows/ingestDependencies.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ INSERT INTO package_dependencies (
7676
package_id, version_id, depends_on_id, depends_on_version_id,
7777
version_constraint, dependency_kind, is_optional, created_at, updated_at
7878
)
79-
SELECT
79+
SELECT DISTINCT ON (pv.id, pd.id)
8080
pv.package_id, pv.id, pd.id, dv.id,
8181
sp.version_constraint, 'direct', FALSE, NOW(), NOW()
8282
FROM staging.osspckgs_deps_raw sp
@@ -100,6 +100,11 @@ JOIN packages pd ON pd.ecosystem = sp.ecosystem
100100
WHEN sp.to_name LIKE '@%/%' THEN SPLIT_PART(sp.to_name, '/', 2)
101101
ELSE sp.to_name END
102102
LEFT JOIN versions dv ON dv.package_id = pd.id AND dv.number = sp.to_version
103+
-- DISTINCT ON collapses duplicate (root, dep) pairs BQ emits with different resolved to_version;
104+
-- ORDER BY keeps the highest to_version (best-resolvable depends_on_version_id), matching the fill
105+
-- variant. ON CONFLICT then handles cross-chunk / pre-existing rows. Full lost this when MERGE_SQL_FULL
106+
-- (which had the same DISTINCT ON) was removed with the drop/rebuild-index step.
107+
ORDER BY pv.id, pd.id, sp.to_version DESC NULLS LAST
103108
ON CONFLICT (version_id, depends_on_id, dependency_kind) DO NOTHING
104109
`
105110

services/apps/packages_worker/src/deps-dev/workflows/ingestVersions.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ const { mergeStagingToTable } = proxyActivities<typeof depsDevActivities>({
2525
retry: { maximumAttempts: 1 },
2626
})
2727

28+
const { setJobStep } = proxyActivities<typeof depsDevActivities>({
29+
startToCloseTimeout: '30 seconds',
30+
retry: { maximumAttempts: 3 },
31+
})
32+
2833
const STAGING_TABLE = 'staging.osspckgs_versions_raw'
2934

3035
const STAGING_DDL = `
@@ -46,11 +51,15 @@ INSERT INTO versions (
4651
package_id, ecosystem, namespace, name, number, published_at, is_prerelease, licenses, last_synced_at,
4752
created_at
4853
)
49-
SELECT
54+
SELECT DISTINCT ON (p.id, s.number)
5055
p.id, s.ecosystem, p.namespace, p.name, s.number, s.published_at, s.is_prerelease, s.licenses, NOW(),
5156
NOW()
5257
FROM staging.osspckgs_versions_raw s
5358
JOIN packages p ON p.purl = s.purl
59+
-- DISTINCT ON + ORDER BY keeps the most recently published row when BQ emits duplicate
60+
-- (purl, number) rows with differing published_at/licenses, so ON CONFLICT DO NOTHING no longer
61+
-- retains an arbitrary insert-order row.
62+
ORDER BY p.id, s.number, s.published_at DESC NULLS LAST
5463
ON CONFLICT (package_id, number) DO NOTHING
5564
`
5665

@@ -127,6 +136,10 @@ export async function ingestVersions(opts: {
127136
let priorStagingRows = 0
128137
const priorTableRowCounts: Record<string, number> = {}
129138

139+
// Mark the phase before the loop so the monitor shows 'merging' instead of the stale prior step
140+
// (e.g. 'loading') during the merge — mirrors ingestDependencies.
141+
await setJobStep({ jobId: exportResult.jobId, step: 'merging' })
142+
130143
for (let chunkIndex = 0; chunkIndex < totalChunks; chunkIndex++) {
131144
const start = chunkIndex * filesPerChunk
132145
const chunk = fileNames.slice(start, start + filesPerChunk)

services/apps/packages_worker/src/scripts/triggerBootstrap.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ Options:
4848
--resume-job <id> Resume a partially-merged package_dependencies job by id. Skips BQ,
4949
reuses that job's exact parquet export, and restarts the chunk loop
5050
where its staging load left off (idempotent ON CONFLICT overlap).
51-
Requires: --kinds package_dependencies AND (incremental | --fill-constraints).
51+
Requires: --kinds package_dependencies. Works in any mode (all deps
52+
merges are idempotent).
5253
--help Show this help
5354
5455
Examples:
@@ -170,19 +171,14 @@ async function main(): Promise<void> {
170171
}
171172
}
172173

173-
// Resume is deps-only and merge-idempotent-only: it re-runs already-committed chunks, safe under
174-
// incremental (ON CONFLICT DO NOTHING) or --fill-constraints (ON CONFLICT DO UPDATE), not a full load.
174+
// Resume is deps-only: it re-runs already-committed chunks. Safe in every mode because all deps
175+
// merges are now idempotent — full and incremental use ON CONFLICT DO NOTHING, --fill-constraints
176+
// uses ON CONFLICT DO UPDATE. (Full became idempotent when the drop/rebuild-index step was removed.)
175177
if (resumeJobId !== undefined) {
176178
if (!kinds || kinds.length !== 1 || kinds[0] !== 'package_dependencies') {
177179
console.error('--resume-job requires: --kinds package_dependencies (resume is deps-only)')
178180
process.exit(1)
179181
}
180-
if (mode !== 'incremental' && !fillConstraints) {
181-
console.error(
182-
'--resume-job requires incremental mode or --fill-constraints (idempotent merges only)',
183-
)
184-
process.exit(1)
185-
}
186182
}
187183

188184
const cfg = TEMPORAL_CONFIG()

0 commit comments

Comments
 (0)