Skip to content

Commit 3ce2401

Browse files
committed
perf(security-contacts): batch sweep writes to fix pool contention
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
1 parent 9db3785 commit 3ce2401

5 files changed

Lines changed: 184 additions & 49 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- Supports fetchBatch's eligibility WHERE clause (contacts_last_refreshed IS NULL,
2+
-- and the daily/weekly age comparisons), which previously had no supporting index.
3+
CREATE INDEX IF NOT EXISTS repos_contacts_last_refreshed_idx
4+
ON repos (contacts_last_refreshed);

services/apps/packages_worker/src/security-contacts/ingestSingle.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { getSecurityContactsConfig } from '../config'
55

66
import { buildBaseDeps, processRepo } from './processBatch'
77
import { RepoPackage, RepoTarget } from './types'
8-
import { markRepoAttempted } from './writeContacts'
8+
import { markRepoAttempted, writeContacts } from './writeContacts'
99

1010
const log = getServiceChildLogger('security-contacts-ondemand')
1111

@@ -93,7 +93,12 @@ export async function ingestSecurityContactsForPurl(
9393
const target = toTarget(row)
9494
const baseDeps = buildBaseDeps(config)
9595
try {
96-
await processRepo(target, baseDeps, qx, cdpQx)
96+
const result = await processRepo(target, baseDeps, cdpQx)
97+
if (result.status === 'ok') {
98+
await writeContacts(qx, result.repoId, result.contacts, result.policies)
99+
} else {
100+
await markRepoAttempted(qx, result.repoId)
101+
}
97102
} catch (err) {
98103
log.error({ repoId: target.repoId, errMsg: (err as Error).message }, 'Repo processing failed')
99104
await markRepoAttempted(qx, target.repoId).catch(() => undefined)

services/apps/packages_worker/src/security-contacts/processBatch.ts

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@ import { resolveCdpEmails } from './resolveCdpEmails'
2020
import {
2121
Extractor,
2222
ExtractorDeps,
23+
ProcessRepoResult,
2324
RawContact,
2425
RepoPackage,
2526
RepoPolicies,
2627
RepoTarget,
2728
} from './types'
28-
import { markRepoAttempted, writeContacts } from './writeContacts'
29+
import { writeContactsBatch } from './writeContacts'
2930

3031
const log = getServiceChildLogger('security-contacts')
3132

@@ -120,9 +121,8 @@ export function buildBaseDeps(config: Config): Omit<ExtractorDeps, 'repoTree'> {
120121
export async function processRepo(
121122
target: RepoTarget,
122123
baseDeps: Omit<ExtractorDeps, 'repoTree'>,
123-
qx: QueryExecutor,
124124
cdpQx: QueryExecutor,
125-
): Promise<void> {
125+
): Promise<ProcessRepoResult> {
126126
// One tree fetch per repo, shared by extractors that probe well-known paths.
127127
let repoTree: ExtractorDeps['repoTree'] = { paths: null }
128128
try {
@@ -142,8 +142,7 @@ export async function processRepo(
142142
{ repoId: target.repoId, errMsg: failed.reason?.message },
143143
'Extractor failed — preserving existing data',
144144
)
145-
await markRepoAttempted(qx, target.repoId)
146-
return
145+
return { repoId: target.repoId, status: 'extractor-failed' }
147146
}
148147

149148
let contacts: RawContact[] = []
@@ -176,7 +175,7 @@ export async function processRepo(
176175
}
177176

178177
const scored = reconcile(contacts)
179-
await writeContacts(qx, target.repoId, scored, policies)
178+
return { repoId: target.repoId, status: 'ok', contacts: scored, policies }
180179
}
181180

182181
export async function processBatch(
@@ -199,30 +198,58 @@ export async function processBatch(
199198
log.warn({ errMsg: (err as Error).message }, 'Heartbeat failed')
200199
}
201200
}, 30_000)
201+
let outcomes: ProcessRepoResult[]
202+
const extractionStartedAt = Date.now()
202203
try {
203204
// A cancelled task (superseded by a newer activity attempt) is left to throw so it stops
204205
// scheduling further repos instead of racing the new attempt.
205-
await mapWithConcurrency(targets, CONCURRENCY, async (target) => {
206+
//
207+
// Extraction is collected here and persisted afterward in one batched call (below) rather
208+
// than per-repo, since per-repo writes meant up to CONCURRENCY concurrent transactions
209+
// against a packages-db pool sized for far fewer connections — the actual sweep bottleneck.
210+
outcomes = await mapWithConcurrency(targets, CONCURRENCY, async (target) => {
206211
if (cancellationSignal().aborted) {
207212
throw new Error(
208213
'Security contacts batch cancelled — superseded by a newer activity attempt',
209214
)
210215
}
211216
try {
212-
await processRepo(target, deps, qx, cdpQx)
217+
return await processRepo(target, deps, cdpQx)
213218
} catch (err) {
214219
log.error(
215220
{ repoId: target.repoId, errMsg: (err as Error).message },
216221
'Repo processing failed',
217222
)
218-
// Best-effort: keeps a persistently-failing repo from hot-looping the sweep.
219-
await markRepoAttempted(qx, target.repoId).catch(() => undefined)
223+
return { repoId: target.repoId, status: 'extractor-failed' as const }
220224
}
221225
})
222226
} finally {
223227
clearInterval(heartbeatTimer)
224228
}
225229

230+
const extractionDurationMs = Date.now() - extractionStartedAt
231+
const ok = outcomes.filter((o) => o.status === 'ok').length
232+
log.info(
233+
{
234+
ok,
235+
failed: outcomes.length - ok,
236+
durationMs: extractionDurationMs,
237+
reposPerSec: Number((outcomes.length / (extractionDurationMs / 1000)).toFixed(1)),
238+
},
239+
'Security contacts batch extraction complete — persisting',
240+
)
241+
242+
const writeStartedAt = Date.now()
243+
await writeContactsBatch(qx, outcomes)
244+
const writeDurationMs = Date.now() - writeStartedAt
245+
log.info(
246+
{
247+
durationMs: writeDurationMs,
248+
reposPerSec: Number((outcomes.length / (writeDurationMs / 1000)).toFixed(1)),
249+
},
250+
'Security contacts batch persistence complete',
251+
)
252+
226253
log.info({ processed: targets.length }, 'Security contacts batch complete')
227254
return { processed: targets.length }
228255
}

services/apps/packages_worker/src/security-contacts/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,7 @@ export interface ExtractorDeps {
7878
}
7979

8080
export type Extractor = (target: RepoTarget, deps: ExtractorDeps) => Promise<ExtractorResult>
81+
82+
export type ProcessRepoResult =
83+
| { repoId: string; status: 'ok'; contacts: ScoredContact[]; policies: Partial<RepoPolicies> }
84+
| { repoId: string; status: 'extractor-failed' }

services/apps/packages_worker/src/security-contacts/writeContacts.ts

Lines changed: 132 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,47 @@
1-
import { QueryExecutor } from '@crowd/data-access-layer/src/queryExecutor'
1+
import { QueryExecutor, formatQuery } from '@crowd/data-access-layer/src/queryExecutor'
22
import { prepareBulkInsert } from '@crowd/data-access-layer/src/utils'
33

4-
import { RepoPolicies, ScoredContact } from './types'
4+
import { ProcessRepoResult, RepoPolicies, ScoredContact } from './types'
5+
6+
const CONTACT_COLUMNS = [
7+
'repo_id',
8+
'channel',
9+
'value',
10+
'role',
11+
'name',
12+
'score',
13+
'confidence',
14+
'provenance',
15+
'reachable',
16+
'reachability_reason',
17+
]
18+
19+
const CONTACT_UPSERT_SET = `(repo_id, channel, value) DO UPDATE SET
20+
role = EXCLUDED.role,
21+
name = EXCLUDED.name,
22+
score = EXCLUDED.score,
23+
confidence = EXCLUDED.confidence,
24+
provenance = EXCLUDED.provenance,
25+
reachable = EXCLUDED.reachable,
26+
reachability_reason = EXCLUDED.reachability_reason,
27+
last_refreshed = NOW(),
28+
updated_at = NOW(),
29+
deleted_at = NULL`
30+
31+
function toContactRow(repoId: string, c: ScoredContact) {
32+
return {
33+
repo_id: repoId,
34+
channel: c.channel,
35+
value: c.value,
36+
role: c.role,
37+
name: c.name ?? null,
38+
score: c.score,
39+
confidence: c.confidence,
40+
provenance: JSON.stringify(c.provenance),
41+
reachable: c.reachable,
42+
reachability_reason: c.reachabilityReason,
43+
}
44+
}
545

646
// Advances contacts_last_refreshed only, so a failed pass isn't reprocessed this sweep.
747
export async function markRepoAttempted(qx: QueryExecutor, repoId: string): Promise<void> {
@@ -30,41 +70,9 @@ export async function writeContacts(
3070
await tx.result(
3171
prepareBulkInsert(
3272
'security_contacts',
33-
[
34-
'repo_id',
35-
'channel',
36-
'value',
37-
'role',
38-
'name',
39-
'score',
40-
'confidence',
41-
'provenance',
42-
'reachable',
43-
'reachability_reason',
44-
],
45-
contacts.map((c) => ({
46-
repo_id: repoId,
47-
channel: c.channel,
48-
value: c.value,
49-
role: c.role,
50-
name: c.name ?? null,
51-
score: c.score,
52-
confidence: c.confidence,
53-
provenance: JSON.stringify(c.provenance),
54-
reachable: c.reachable,
55-
reachability_reason: c.reachabilityReason,
56-
})),
57-
`(repo_id, channel, value) DO UPDATE SET
58-
role = EXCLUDED.role,
59-
name = EXCLUDED.name,
60-
score = EXCLUDED.score,
61-
confidence = EXCLUDED.confidence,
62-
provenance = EXCLUDED.provenance,
63-
reachable = EXCLUDED.reachable,
64-
reachability_reason = EXCLUDED.reachability_reason,
65-
last_refreshed = NOW(),
66-
updated_at = NOW(),
67-
deleted_at = NULL`,
73+
CONTACT_COLUMNS,
74+
contacts.map((c) => toContactRow(repoId, c)),
75+
CONTACT_UPSERT_SET,
6876
),
6977
)
7078
}
@@ -94,3 +102,90 @@ export async function writeContacts(
94102
)
95103
})
96104
}
105+
106+
export async function markReposAttempted(qx: QueryExecutor, repoIds: string[]): Promise<void> {
107+
if (repoIds.length === 0) return
108+
await qx.result(
109+
'UPDATE repos SET contacts_last_refreshed = NOW() WHERE id = ANY($(repoIds)::bigint[])',
110+
{ repoIds },
111+
)
112+
}
113+
114+
type OkResult = Extract<ProcessRepoResult, { status: 'ok' }>
115+
116+
// Bounds blast radius: a bad row only forces a re-extract of its chunk next sweep, not the whole batch.
117+
const WRITE_CHUNK_SIZE = 100
118+
119+
function prepareBulkPolicyUpdate(chunk: OkResult[]): string {
120+
const rows = chunk.map(
121+
(_, i) =>
122+
`($(rows.id${i}), $(rows.securityPolicyUrl${i}), $(rows.vulnerabilityReportingUrl${i}), $(rows.pvrResolved${i}), $(rows.bugBountyUrl${i}), $(rows.securityTxtUrl${i}), $(rows.pvrEnabled${i}))`,
123+
)
124+
125+
const params = chunk.reduce(
126+
(acc, r, i) => {
127+
acc[`id${i}`] = r.repoId
128+
acc[`securityPolicyUrl${i}`] = r.policies.securityPolicyUrl ?? null
129+
acc[`vulnerabilityReportingUrl${i}`] = r.policies.vulnerabilityReportingUrl ?? null
130+
acc[`pvrResolved${i}`] = r.policies.pvrEnabled !== undefined
131+
acc[`bugBountyUrl${i}`] = r.policies.bugBountyUrl ?? null
132+
acc[`securityTxtUrl${i}`] = r.policies.securityTxtUrl ?? null
133+
acc[`pvrEnabled${i}`] = r.policies.pvrEnabled ?? null
134+
return acc
135+
},
136+
{} as Record<string, unknown>,
137+
)
138+
139+
return formatQuery(
140+
`UPDATE repos AS r SET
141+
security_policy_url = COALESCE(v.security_policy_url, r.security_policy_url),
142+
vulnerability_reporting_url = CASE WHEN v.pvr_resolved
143+
THEN v.vulnerability_reporting_url
144+
ELSE COALESCE(v.vulnerability_reporting_url, r.vulnerability_reporting_url)
145+
END,
146+
bug_bounty_url = COALESCE(v.bug_bounty_url, r.bug_bounty_url),
147+
security_txt_url = COALESCE(v.security_txt_url, r.security_txt_url),
148+
pvr_enabled = COALESCE(v.pvr_enabled, r.pvr_enabled),
149+
contacts_last_refreshed = NOW()
150+
FROM (VALUES ${rows.join(',')}) AS v(id, security_policy_url, vulnerability_reporting_url, pvr_resolved, bug_bounty_url, security_txt_url, pvr_enabled)
151+
WHERE r.id = v.id::bigint`,
152+
params,
153+
)
154+
}
155+
156+
async function writeContactsChunk(qx: QueryExecutor, chunk: OkResult[]): Promise<void> {
157+
if (chunk.length === 0) return
158+
const repoIds = chunk.map((r) => r.repoId)
159+
160+
await qx.tx(async (tx) => {
161+
await tx.result(
162+
'UPDATE security_contacts SET deleted_at = NOW(), updated_at = NOW() WHERE repo_id = ANY($(repoIds)::bigint[]) AND deleted_at IS NULL',
163+
{ repoIds },
164+
)
165+
166+
const rows = chunk.flatMap((r) => r.contacts.map((c) => toContactRow(r.repoId, c)))
167+
if (rows.length > 0) {
168+
await tx.result(
169+
prepareBulkInsert('security_contacts', CONTACT_COLUMNS, rows, CONTACT_UPSERT_SET),
170+
)
171+
}
172+
173+
await tx.result(prepareBulkPolicyUpdate(chunk))
174+
})
175+
}
176+
177+
export async function writeContactsBatch(
178+
qx: QueryExecutor,
179+
outcomes: ProcessRepoResult[],
180+
): Promise<void> {
181+
const attemptedOnlyIds = outcomes
182+
.filter((o) => o.status === 'extractor-failed')
183+
.map((o) => o.repoId)
184+
const ok = outcomes.filter((o): o is OkResult => o.status === 'ok')
185+
186+
for (let i = 0; i < ok.length; i += WRITE_CHUNK_SIZE) {
187+
await writeContactsChunk(qx, ok.slice(i, i + WRITE_CHUNK_SIZE))
188+
}
189+
190+
await markReposAttempted(qx, attemptedOnlyIds)
191+
}

0 commit comments

Comments
 (0)