Skip to content

Offload reveal publish calls so one slot can't block another's deadline - #165

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:mainfrom
damilolaedwards:fix-reveal-service-parallel-publish
Open

Offload reveal publish calls so one slot can't block another's deadline#165
damilolaedwards wants to merge 1 commit into
ethpandaops:mainfrom
damilolaedwards:fix-reveal-service-parallel-publish

Conversation

@damilolaedwards

Copy link
Copy Markdown
Contributor

Problem

The reveal loop built and published every slot's envelope inline, on its
single run loop goroutine. A slow or hanging publish call for one slot
(clients report the submit call taking several hundred ms; a hung one could
take the full ctx timeout) held that goroutine hostage, so any other slot
that came due in the meantime missed its own deadline through no fault of
its own, purely from sharing a queue with a slow neighbor.

Fix

Each due slot's build+publish now runs in its own goroutine, reporting the
outcome back to the run loop over a channel. The run loop stays the sole
owner of the pending map and all its bookkeeping (dedup, retry scheduling,
success/failure recording) — it just no longer does the network call itself.
An in-flight guard keeps at most one attempt running per slot, and an
identity check on the reported outcome discards it if a reorg re-bound the
slot to a different block while the attempt was still running, so a stale
attempt can never be misapplied to whatever now occupies the slot.

Also scaled the publish call's timeout to at most half the slot duration:
the previous flat 5 seconds could by itself run past a short devnet slot's
own deadlines regardless of the above.

Testing

All 15 existing RevealService tests pass unchanged, including the
retry/dedup/vote-gate ones that exercise the exact bookkeeping this moved —
good evidence the refactor preserved behavior.

Added TestRevealService_SlowPublishDoesNotHeadOfLineBlockAnotherSlot,
which reproduces the original scenario (a 400ms-slow publish for slot 1,
slot 2 due 300ms later): slot 2 now starts publishing right at its own due
time instead of being delayed behind slot 1's slow call.

go build, go vet, and go test -race ./pkg/... all pass (aside from a
pre-existing, unrelated failure in pkg/webui caused by the frontend not
being built in this checkout).

The reveal loop built and published every slot's envelope inline, on its
single run loop goroutine. A slow or hanging publish call for one slot
(clients report the submit call taking several hundred ms; a hung one could
take the full ctx timeout) held that goroutine hostage, so any other slot
that came due in the meantime missed its own deadline through no fault of
its own, purely from sharing a queue with a slow neighbor.

Each due slot's build+publish now runs in its own goroutine, reporting the
outcome back to the run loop over a channel. The run loop stays the sole
owner of the pending map and all its bookkeeping (dedup, retry scheduling,
success/failure recording) — it just no longer does the network call itself.
An in-flight guard keeps at most one attempt running per slot, and an
identity check on the reported outcome discards it if a reorg re-bound the
slot to a different block while the attempt was still running, so a stale
attempt can never be misapplied to whatever now occupies the slot.

Also scaled the publish call's timeout to at most half the slot duration:
the previous flat 5 seconds could by itself run past a short devnet slot's
own deadlines regardless of the above.
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.

1 participant