BaseTrigger Max Retries + Pre-Ack Handling by DylanTinianov · Pull Request #1969 · smartcontractkit/chainlink-common

DylanTinianov · 2026-04-07T15:07:49Z

Adds max retries to base trigger along with DB pruning loop for cleanup.
Adds pre-ACK cache for slow nodes which receive an ACK before even seeing the trigger event
Fixes race condition on ACK during event Delivery

github-actions · 2026-04-07T15:09:12Z

⚠️ API Diff Results - `github.com/smartcontractkit/chainlink-common`

⚠️ Breaking Changes (1)

`pkg/capabilities.BaseTriggerMetrics` (1)

IncGaveUp — ➕ Added

✅ Compatible Changes (3)

`pkg/capabilities.(*BaseTriggerBeholderMetrics)` (1)

IncGaveUp — ➕ Added

`pkg/settings/cresettings.Schema` (2)

BaseTriggerMaxRetries — ➕ Added
BaseTriggerPruneAge — ➕ Added

📄 View full apidiff report

fernandezlautaro · 2026-04-07T16:31:10Z

pkg/capabilities/base_trigger.go

 		return
 	}

+	maxAttempts := b.maxRetries(ctx)


could you be consistent, either maxAttempt or maxRetries?

fernandezlautaro · 2026-04-07T16:32:53Z

pkg/capabilities/base_trigger_metrics.go

 	activeRegistrations      metric.Int64UpDownCounter
 	pendingEvents            metric.Int64UpDownCounter
 	stuckEvents              metric.Int64UpDownCounter
+	gaveUpCount              metric.Int64Counter


gaveUp shouldn't be something more like stopResendingEvents? specially since the other metrics are stuckEvents/pendingEvents/etc.

fernandezlautaro · 2026-04-07T17:22:09Z

pkg/capabilities/base_trigger.go

+	var toGiveUp []gaveUpEvent
 	for triggerID, pendingForTrigger := range b.pending {
 		for eventID, rec := range pendingForTrigger {
+			if maxAttempts > 0 && rec.Attempts >= maxAttempts {


replace maxAttempts > 0 by a function to make it clear

also, could you make a new method with this new logic which is "stop resending and fire a metric", and the old behaviour the code for "appendToTryResending"

so there's a clear

if reachedMaxAttempts(rec.Attempts){ "stop resending and fire a metric"() } else { "appendToTryResending"() }

fernandezlautaro · 2026-04-07T17:30:43Z

pkg/capabilities/base_trigger.go

+					triggerID:   triggerID,
+					eventID:     eventID,
+					attempts:    rec.Attempts,
+					wasCritical: wasCritical,


maybe for another PR but this metric of criticality somehow collides with AddPendingEvents() one.

I believe critically shouldn't be handled by the code here but in alerts, so you might drop this metric of critical and just have a metric of resending.

In an ideal scenario, resending should be almost near 0

fernandezlautaro · 2026-04-07T17:35:36Z

pkg/capabilities/base_trigger.go

+		if ev.wasCritical {
+			b.metrics.DecStuckEvent(ev.triggerID, ev.eventID)
+		}
+		if err := b.store.DeleteEvent(ctx, ev.triggerID, ev.eventID); err != nil {


I'm really wondering if we want to do this DB deletion here, or having another long lived process that prunes old data from DB.

Specially since if this happens for a payload that's unrecoverable such as the HTTP Trigger, you have no means to somehow restore this data to the customer (just a thought)

fernandezlautaro · 2026-04-07T17:50:25Z

pkg/capabilities/base_trigger.go

+		b.mu.Unlock()
+
+		if inMemory {
+			// Still actively tracked — scanPending will handle it (gave-up or ACK).


this should technically never happen, right?

If the prune time is set to 24hs, and max attempts to 20, with a retrial of 30 seconds, that's 10m of retrials, so eventually this should have been deleted from in-mem.

I believe you should error here, or even throw a metric to show there's an inconsistence

fernandezlautaro · 2026-04-07T17:53:05Z

pkg/capabilities/base_trigger.go

+	}
+	cutoff := time.Now().Add(-age)
+
+	recs, err := b.store.List(b.ctx)


it might be better to have a query just to hit the DB asking for events which have been modified recently, and potentially also parameterize the maxAttempts to it

fernandezlautaro · 2026-04-07T17:53:51Z

pkg/capabilities/base_trigger.go

+	}
+
+	for _, rec := range recs {
+		if rec.FirstAt.After(cutoff) {


why FirstAt field and not LastSeenAt?
Shouldn't you remove based on the last time you modified the row in the DB instead of the first time you inserted it?

…m/smartcontractkit/chainlink-common into CRE-3248-basetrigger-attempts-max

Implement max retries

79eab54

DylanTinianov self-assigned this Apr 7, 2026

DylanTinianov temporarily deployed to integration April 7, 2026 15:08 — with GitHub Actions Inactive

DylanTinianov had a problem deploying to integration April 7, 2026 15:08 — with GitHub Actions Failure

DylanTinianov temporarily deployed to integration April 7, 2026 15:08 — with GitHub Actions Inactive

Fix test

32debdf

DylanTinianov temporarily deployed to integration April 7, 2026 15:33 — with GitHub Actions Inactive

DylanTinianov had a problem deploying to integration April 7, 2026 15:33 — with GitHub Actions Failure

DylanTinianov temporarily deployed to integration April 7, 2026 15:33 — with GitHub Actions Inactive

DylanTinianov requested a review from fernandezlautaro April 7, 2026 15:34

DylanTinianov changed the title ~~Implement max retries~~ BaseTrigger Max Retries Apr 7, 2026

DylanTinianov marked this pull request as ready for review April 7, 2026 15:55

DylanTinianov requested a review from a team as a code owner April 7, 2026 15:55

product-security-plaid-production bot requested a review from MStreet3 April 7, 2026 15:55

Merge branch 'main' into CRE-3248-basetrigger-attempts-max

6838e49

DylanTinianov temporarily deployed to integration April 7, 2026 16:21 — with GitHub Actions Inactive

DylanTinianov had a problem deploying to integration April 7, 2026 16:21 — with GitHub Actions Failure

fernandezlautaro reviewed Apr 7, 2026

View reviewed changes

DylanTinianov added 2 commits April 7, 2026 17:41

Handle Pre-ACKs

a526128

Merge branch 'CRE-3248-basetrigger-attempts-max' of https://github.co…

42de4b2

…m/smartcontractkit/chainlink-common into CRE-3248-basetrigger-attempts-max

DylanTinianov changed the title ~~BaseTrigger Max Retries~~ BaseTrigger Max Retries + Pre-Ack Handling Apr 7, 2026

DylanTinianov temporarily deployed to integration April 7, 2026 21:42 — with GitHub Actions Inactive

DylanTinianov had a problem deploying to integration April 7, 2026 21:42 — with GitHub Actions Failure

DylanTinianov temporarily deployed to integration April 7, 2026 21:42 — with GitHub Actions Inactive

Fix DeliverEvent ACK race

2a30cae

DylanTinianov had a problem deploying to integration April 8, 2026 15:40 — with GitHub Actions Error

Merge branch 'main' into CRE-3248-basetrigger-attempts-max

f6e9db8

DylanTinianov temporarily deployed to integration April 8, 2026 15:44 — with GitHub Actions Inactive

DylanTinianov had a problem deploying to integration April 8, 2026 15:44 — with GitHub Actions Failure

DylanTinianov temporarily deployed to integration April 8, 2026 15:44 — with GitHub Actions Inactive

DylanTinianov added 2 commits April 8, 2026 14:10

Fix preAcked cache

f4e31f0

Merge branch 'CRE-3248-basetrigger-attempts-max' of https://github.co…

de48dcb

…m/smartcontractkit/chainlink-common into CRE-3248-basetrigger-attempts-max

DylanTinianov temporarily deployed to integration April 8, 2026 18:10 — with GitHub Actions Inactive

DylanTinianov had a problem deploying to integration April 8, 2026 18:10 — with GitHub Actions Failure

DylanTinianov temporarily deployed to integration April 8, 2026 18:10 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BaseTrigger Max Retries + Pre-Ack Handling#1969

BaseTrigger Max Retries + Pre-Ack Handling#1969
DylanTinianov wants to merge 9 commits intomainfrom
CRE-3248-basetrigger-attempts-max

DylanTinianov commented Apr 7, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 7, 2026 •

edited

Loading

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

fernandezlautaro Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

DylanTinianov commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ API Diff Results - github.com/smartcontractkit/chainlink-common

⚠️ Breaking Changes (1)

pkg/capabilities.BaseTriggerMetrics (1)

✅ Compatible Changes (3)

pkg/capabilities.(*BaseTriggerBeholderMetrics) (1)

pkg/settings/cresettings.Schema (2)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DylanTinianov commented Apr 7, 2026 •

edited

Loading

github-actions bot commented Apr 7, 2026 •

edited

Loading

⚠️ API Diff Results - `github.com/smartcontractkit/chainlink-common`

`pkg/capabilities.BaseTriggerMetrics` (1)

`pkg/capabilities.(*BaseTriggerBeholderMetrics)` (1)

`pkg/settings/cresettings.Schema` (2)