Fix deadlock in concurrent tracker mapping
The tracker-mapping worker runs many Process calls in parallel. In Phase 4 a single transaction locked the worker's own claimed pattern row via UpdateMapping and then locked sibling rows on the same banner via the re-enqueue. Two workers mapping sibling patterns on one banner each held their own row and waited on the other's, forming a lock cycle that Postgres aborted with deadlock detected (40P01). Split the sibling re-enqueue into its own short transaction that runs after the mapping commits, so the claimed-row lock is released before any sibling row is locked. Also take the sibling UPDATE row locks in a deterministic id order through an ORDER BY id ... FOR UPDATE subquery, so overlapping re-enqueues can no longer invert lock order between themselves. The re-enqueue only flags siblings, so deferring it past the commit is safe and lets reprocessed siblings observe committed data. Signed-off-by: Émile Ré <emile@probo.com>
This commit is contained in:
@@ -289,7 +289,9 @@ func (h *trackerMappingHandler) Process(ctx context.Context, tp coredata.Tracker
|
|||||||
// Phase 4: persist the pattern mapping in a short transaction. The
|
// Phase 4: persist the pattern mapping in a short transaction. The
|
||||||
// unmatched fallback keeps catalog coverage complete even when no
|
// unmatched fallback keeps catalog coverage complete even when no
|
||||||
// vendor was resolved.
|
// vendor was resolved.
|
||||||
return h.pg.WithTx(
|
mapped := true
|
||||||
|
|
||||||
|
if err := h.pg.WithTx(
|
||||||
ctx,
|
ctx,
|
||||||
func(ctx context.Context, tx pg.Tx) error {
|
func(ctx context.Context, tx pg.Tx) error {
|
||||||
if commonPatternID == nil {
|
if commonPatternID == nil {
|
||||||
@@ -331,6 +333,8 @@ func (h *trackerMappingHandler) Process(ctx context.Context, tp coredata.Tracker
|
|||||||
log.String("tracker_pattern_id", tp.ID.String()),
|
log.String("tracker_pattern_id", tp.ID.String()),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
mapped = false
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -344,20 +348,39 @@ func (h *trackerMappingHandler) Process(ctx context.Context, tp coredata.Tracker
|
|||||||
log.String("tracker_pattern_id", tp.ID.String()),
|
log.String("tracker_pattern_id", tp.ID.String()),
|
||||||
)
|
)
|
||||||
|
|
||||||
// This run newly resolved a catalog third party, so
|
return nil
|
||||||
// same-banner siblings that share an initiator domain but
|
},
|
||||||
// were processed earlier and left unmatched can now match
|
); err != nil {
|
||||||
// against it. Re-arm their mapping so the worker revisits
|
return err
|
||||||
// them; the guards keep already-mapped siblings untouched.
|
}
|
||||||
if commonThirdPartyID != nil && !det.commonThirdPartyPreexisted {
|
|
||||||
if err := h.reenqueueUnmappedSiblings(ctx, tx, tp, det.domains); err != nil {
|
// Phase 5: re-arm same-banner siblings in a separate short
|
||||||
|
// transaction, after the mapping above has committed. This run newly
|
||||||
|
// resolved a catalog third party, so siblings that share an initiator
|
||||||
|
// domain but were processed earlier and left unmatched can now match
|
||||||
|
// against it. Re-arm their mapping so the worker revisits them; the
|
||||||
|
// guards keep already-mapped siblings untouched.
|
||||||
|
//
|
||||||
|
// The re-enqueue must not run inside the Phase 4 transaction: that
|
||||||
|
// transaction holds the row lock on tp, and the sibling UPDATE then
|
||||||
|
// takes locks on other tracker_patterns rows while holding it. Two
|
||||||
|
// workers mapping sibling patterns on the same banner would acquire
|
||||||
|
// those row locks in opposite orders and deadlock. Committing Phase 4
|
||||||
|
// first releases tp's lock, and RequestMappingForUnmappedSiblings
|
||||||
|
// takes its locks in a deterministic id order, so the two can no
|
||||||
|
// longer cycle.
|
||||||
|
if mapped && commonThirdPartyID != nil && !det.commonThirdPartyPreexisted {
|
||||||
|
if err := h.pg.WithTx(
|
||||||
|
ctx,
|
||||||
|
func(ctx context.Context, tx pg.Tx) error {
|
||||||
|
return h.reenqueueUnmappedSiblings(ctx, tx, tp, det.domains)
|
||||||
|
},
|
||||||
|
); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// deterministicResult carries the outcome of the pure-SQL catalog
|
// deterministicResult carries the outcome of the pure-SQL catalog
|
||||||
|
|||||||
@@ -1286,11 +1286,19 @@ func (tps *TrackerPatterns) RequestMappingForUnmappedSiblings(
|
|||||||
return 0, nil
|
return 0, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The target rows are locked through an ORDER BY id ... FOR UPDATE
|
||||||
|
// subquery so concurrent re-enqueues over overlapping sibling sets
|
||||||
|
// always acquire their row locks in the same ascending id order. Two
|
||||||
|
// workers mapping sibling patterns on the same banner would otherwise
|
||||||
|
// lock the shared rows in opposite orders and deadlock (40P01).
|
||||||
q := `
|
q := `
|
||||||
UPDATE tracker_patterns
|
UPDATE tracker_patterns
|
||||||
SET
|
SET
|
||||||
mapping_requested_at = NOW(),
|
mapping_requested_at = NOW(),
|
||||||
updated_at = NOW()
|
updated_at = NOW()
|
||||||
|
WHERE id IN (
|
||||||
|
SELECT id
|
||||||
|
FROM tracker_patterns
|
||||||
WHERE
|
WHERE
|
||||||
%[1]s
|
%[1]s
|
||||||
AND cookie_banner_id = @cookie_banner_id
|
AND cookie_banner_id = @cookie_banner_id
|
||||||
@@ -1306,6 +1314,9 @@ WHERE
|
|||||||
AND initiator_domain = ANY(@domains)
|
AND initiator_domain = ANY(@domains)
|
||||||
AND tracker_pattern_id IS NOT NULL
|
AND tracker_pattern_id IS NOT NULL
|
||||||
)
|
)
|
||||||
|
ORDER BY id
|
||||||
|
FOR UPDATE
|
||||||
|
)
|
||||||
`
|
`
|
||||||
|
|
||||||
q = fmt.Sprintf(q, scope.SQLFragment())
|
q = fmt.Sprintf(q, scope.SQLFragment())
|
||||||
|
|||||||
Reference in New Issue
Block a user