Skip to content

feat(proxyd): CL consensus-aware Redis integration for uniform sync status - #611

Open
jelias2 wants to merge 9 commits into
mainfrom
jelias2/proxyd-cl-redis-integration
Open

jelias2 wants to merge 9 commits into
mainfrom
jelias2/proxyd-cl-redis-integration

Conversation

@jelias2

@jelias2 jelias2 commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Extends the ConsensusTracker interface with GetCLSyncBody/SetCLSyncBody methods to store the CL consensus pin-backend's optimism_syncStatus response body
  • InMemoryConsensusTracker: stores the body with an RWMutex (preserves existing single-pod behavior)
  • RedisConsensusTracker: the leader pod writes the selected sync body to Redis on each heartbeat; follower pods read it back, ensuring all pods behind a load balancer serve the same optimism_syncStatus response
  • Removes the now-unnecessary syncStatusBodyMu, consensusSyncBody, and lastServedCLL1Num fields from ConsensusPoller, delegating to the tracker instead

Ref: ethereum-optimism/core-team#2211

Test plan

  • go build ./... compiles cleanly
  • go test ./... passes (unit + integration tests)
  • Deploy to a staging environment with multiple proxyd pods using consensus_ha = true and routing_strategy = "consensus_aware_consensus_layer", verify all pods return identical optimism_syncStatus responses
  • Verify single-pod (no Redis HA) behavior is unchanged

🤖 Generated with Claude Code

@jelias2
jelias2 requested a review from a team as a code owner April 26, 2026 15:31
@jelias2
jelias2 requested a review from bitwiseguy April 26, 2026 15:31
@jelias2
jelias2 force-pushed the jelias2/proxyd-cl-redis-integration branch 2 times, most recently from 48b2bbf to 14f3132 Compare April 26, 2026 17:45
@jelias2
jelias2 force-pushed the jelias2/proxyd-cl-redis-integration branch 2 times, most recently from 6f42ea8 to 62be375 Compare May 6, 2026 21:48
@jelias2

jelias2 commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Dev Cluster Validation — CL Redis HA

Deployed proxyd:cl-redis-ha-62be375 to interop-jnt-v0-1-proxyd-cl on oplabs-dev-infra with Redis HA enabled (10.128.24.107:6379, namespace proxyd-cl-ha).

Results

Leader election: Working. One pod acquires the redsync mutex, the other follows via Redis.

CL sync body propagation: Verified. Both pods serve the same optimism_syncStatus response via the ingress endpoint. The follower reads the CL body from Redis (zero cache misses after initial startup).

Metrics (both pods reporting):

Metric Leader Follower
group_consensus_ha_cl_pin_l1 10803832 10803833
group_consensus_ha_latest_block 1906715 1906717
group_consensus_ha_safe_block 1906533 1906533
group_consensus_ha_finalized_block 1905911 1905911
cl_sync_status_cache_hit_total 8 8

Failure modes observed:

  • Initial startup: both pods show "local state is not valid or behind remote, skipping" for ~4s while first poll completes — expected behavior, resolves after first consensus cycle.
  • First cycle: "CL pin selection: no valid candidate in consensus group" with group_size=0 — expected, backends haven't been polled yet. Resolves on second cycle.

k8s changes: Pushed to jelias2/proxyd-cl-redis-ha-test in the k8s repo (image tag + Redis HA config for the overlay).

@jelias2

jelias2 commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Failover & Store Reuse Analysis

Pod Deletion Test (Leader Kill)

Deleted the leader pod (smvwl) to test worst-case failover:

Time Event
T+0s Leader deleted. Follower (7mbpk) immediately serves cached CL body from Redis — zero downtime.
T+10s current_l1 advances (10803846 → 10803847). Endpoint responsive throughout.
T+20s Old redsync lock TTL expires. 7mbpk acquires leadership, begins writing to Redis under new lock token.
T+25s New pod (w2c4r) joins as follower, reads from Redis. Both pods report matching ha_cl_pin_l1 metric under new leader label.

3 consecutive requests after failover all returned identical current_l1=10803851, safe_l2=1906533 — both pods serving the same pinned response.

Redis Store Reuse

All Redis keys are scoped by the redsync lock token (a random value per leader election):

consensus:proxyd-cl-ha:cl_sync_body:<lock-token>
consensus:proxyd-cl-ha:state:<lock-token>

Old keys are never reused — they expire via TTL (lockPeriod). Each new leader election produces a fresh set of keys.

Scenario Behavior
L1 reorg CL body refreshed every 5s poll cycle. Next poll picks up new L1 state naturally. No stale data risk.
Pod restart / deletion Surviving pod serves from cached Redis data. New pod cold-starts (~5s until first CL body).
Rolling deployment New lock token → new Redis keys. Old keys TTL out. Brief ~5-10s gap on first consensus cycle.
Full restart (all pods) Fresh lock token, clean start. ~10s until first CL body is available. Same cold-start window that exists without Redis HA.

@jelias2
jelias2 force-pushed the jelias2/proxyd-cl-redis-integration branch from 62be375 to 64a5c05 Compare May 6, 2026 23:16
Move the CL sync body cache (optimism_syncStatus response) from
ConsensusPoller into the ConsensusTracker interface. The InMemory
implementation stores it locally; the Redis implementation propagates
it via Redis so all pods serve the same response regardless of which
pod the load balancer routes to.

Add group_consensus_ha_cl_pin_l1 Prometheus gauge so operators can
track the CL pin L1 block number on both leader and follower.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jelias2
jelias2 force-pushed the jelias2/proxyd-cl-redis-integration branch from 64a5c05 to 9045f0d Compare May 7, 2026 15:30
Comment thread proxyd/consensus_tracker.go Outdated
Comment on lines +275 to +276
ct.clRemoteSyncBody = payload.Body
ct.clRemoteL1Num = payload.L1Num

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.

shouldn't you also update ct.remote.clSyncBody and ct.remote.clLastServedL1?

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.

Also, why not mirror this data to ct.clLocalSyncBody and ct.clLocalL1Num as well? (i.e. directly call c.SetCLSyncBody(...))

Comment thread proxyd/consensus_tracker.go Outdated
Comment on lines +126 to +129
clLocalSyncBody json.RawMessage
clLocalL1Num uint64
clRemoteSyncBody json.RawMessage
clRemoteL1Num uint64

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.

how are these different from the fields under local and remote trackers?

jelias2 and others added 2 commits May 8, 2026 15:09
…sync state

Eliminate duplicate CL sync body fields on RedisConsensusTracker by
delegating to the existing local/remote InMemoryConsensusTracker
instances, matching the pattern already used for EL block numbers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lability

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread proxyd/consensus_tracker.go
jelias2 and others added 2 commits May 11, 2026 11:30
… write

Ensures postPayload updates ct.remote with the CL sync body after a
successful Redis write, matching the pattern used for consensus state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jelias2
jelias2 force-pushed the jelias2/proxyd-cl-redis-integration branch from bef6c40 to 9604802 Compare May 11, 2026 19:45
The 1.3.0 orb uses cimg/python:3.9 which is incompatible with the
latest pip bootstrap script (requires Python 3.10+). v3.0.0 removes
the Python dependency entirely, using pure Bash instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented May 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 22.03390% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.82%. Comparing base (89ca421) to head (d035791).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
proxyd/consensus_tracker.go 16.98% 44 Missing ⚠️
proxyd/metrics.go 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #611      +/-   ##
==========================================
- Coverage   59.01%   58.82%   -0.20%     
==========================================
  Files          97       97              
  Lines       15121    15170      +49     
==========================================
  Hits         8923     8923              
- Misses       5651     5700      +49     
  Partials      547      547              
Flag Coverage Δ
cci-stats 2.61% <ø> (ø)
op-acceptor 58.94% <ø> (-0.05%) ⬇️
op-signer 47.32% <ø> (ø)
op-txproxy 31.81% <ø> (ø)
peer-mgmt-service 8.48% <ø> (ø)
proxyd 72.38% <22.03%> (-0.57%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
proxyd/consensus_poller.go 85.94% <ø> (ø)
proxyd/consensus_poller_cl.go 81.17% <100.00%> (-0.29%) ⬇️
proxyd/metrics.go 84.82% <0.00%> (-1.19%) ⬇️
proxyd/consensus_tracker.go 10.81% <16.98%> (+1.58%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cp.consensusSyncBody = pin.body
cp.lastServedCLL1Num = pin.l1
cp.syncStatusBodyMu.Unlock()
cp.tracker.SetCLSyncBody(pin.body, pin.l1)

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.

this ends up setting both the local and remote trackers.

The fact that this is setting the remote tracker, shouldn't it also make this update to Redis as well?

Comment thread proxyd/consensus_tracker.go Outdated

func (ct *RedisConsensusTracker) SetCLSyncBody(body json.RawMessage, l1Num uint64) {
ct.local.SetCLSyncBody(body, l1Num)
// Mirror to remote so GetCLSyncBody returns fresh data on the leader immediately.

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.

Q: Am I misunderstanding this comment? How would this freshly set data get to the leader?

  • assuming leadder is a difference instance/pod of proxyd.

This method updates ct.remote which is an in-memory view scoped to this proxyd process only, and it doesn't actually propagate this update to Redis or other instances. I think the comment should clarify that this mirrors to the local (not to be confused with ct.local) in-memory ct.remote state so GetCLSyncBody returns fresh data immediately on this instance, without waiting for the next Redis poll.

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.

Unless, the intention is to actually propagate this update all the way to the leader, which then would require updating this method to also a wrap up a corresponding write to Redis as well (like what postPayload(...) method does)

Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
@yashvardhan-kukreja
yashvardhan-kukreja force-pushed the jelias2/proxyd-cl-redis-integration branch from fd6c8de to 29f091b Compare May 12, 2026 22:38
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>
Signed-off-by: Yashvardhan Kukreja <yashvardhan@oplabs.co>

This branch has not been deployed

No deployments
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.

3 participants