Skip to content

Enforce LOOP relayer replacement cleanup#2013

Draft
cawthorne wants to merge 1 commit intomainfrom
feature/plex-2873-relayer-contract
Draft

Enforce LOOP relayer replacement cleanup#2013
cawthorne wants to merge 1 commit intomainfrom
feature/plex-2873-relayer-contract

Conversation

@cawthorne
Copy link
Copy Markdown
Contributor

Summary

Adds server-side cleanup for replaced LOOP relayer resources, following the lifecycle issue described in PLEX-2873.

The LOOP host can refresh broker-backed connections and invoke NewRelayer again in a still-running plugin process. The server now tracks the currently served relayer resource and closes the previous served relayer once a replacement is successfully served. This makes the replacement contract explicit in the common relayer server instead of relying only on each plugin implementation to remember the lifecycle rule.

This PR also documents the PluginRelayer.NewRelayer contract: re-invocation replaces the active relayer, and implementations must not retain background resources from old relayers outside the returned service.

Validation

GOMODCACHE=/private/tmp/gomodcache GOCACHE=/private/tmp/gocache GOPATH=/private/tmp/gopath go test ./pkg/loop/internal/relayer -count=1

Notes

This complements the chainlink-aptos plugin-side fix, which closes the previous Aptos relayer/config emitter before constructing a replacement.

@github-actions
Copy link
Copy Markdown

📊 API Diff Results

No changes detected for module github.com/smartcontractkit/chainlink-common

View full report

Comment on lines +14 to +16
// NewRelayer returns the active relayer for the plugin. Re-invoking NewRelayer
// replaces the previously served relayer; implementations must not retain
// background resources from old relayers outside the returned service.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it desirable to introduce this property? If NewRelayer is being called more than once, that is definitionally a bug that we should diagnose. If we want to also be defensive about enforcing the existing singleton property, then we could do that, but it's really most important to find the upstream root cause that allowed this to happen.

Also, we used to run multiple relayers instances inside of one plugin, in which case multiple calls were expected, and so a singleton or even single swappable instance is actually too restrictive and keying on chain ID would be more aligned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants