Skip to content

Fix shard wedge: bound REST rate-limit waits, drop events on full guild queue - #447

Merged
ewohltman merged 2 commits into
masterfrom
develop
Jul 11, 2026
Merged

Fix shard wedge: bound REST rate-limit waits, drop events on full guild queue#447
ewohltman merged 2 commits into
masterfrom
develop

Conversation

@ewohltman

Copy link
Copy Markdown
Owner

Summary

Shard 3 re-entered the zombie/invalid-session reconnect loop on v1.16.4 (~2h after rollout; same pattern as v1.16.2/v1.16.3: clean for hours, then permanently wedged until pod restart). This time the pod was diagnosed live via its pprof endpoint (/debug/pprof/goroutine?debug=2), which produced a definitive failure chain:

  1. The affected guild's sequencer worker was blocked 96+ minutes inside CreateRole, waiting in disgo's REST rate limiter. Discord can return retry_after values of an hour or more for role mutations, and none of our REST calls carried a deadline, so the rate limiter blocked for the entire window.
  2. Voice events kept arriving for that guild until its 64-slot queue filled; sequencer.Submit then blocked the gateway read loop on the channel send — while holding disgo's event-manager mutex (eventManagerImpl.mu, held for the duration of HandleGatewayEvent).
  3. Every reconnect's fresh read loop blocked on that same mutex at its first dispatch, so heartbeat ACKs were never read: zombie close → reconnect → wedge again, indefinitely (the exact 82.5s = 2× heartbeat-interval periodicity seen in the logs).

The same unbounded rate-limiter wait also explains the pre-sequencer incidents on v1.16.2/v1.16.3, where the wait ran directly on the read loop.

Changes

  • Bound all Discord REST calls (CreateRole, AddMemberRole, RemoveMemberRole, GetGuild, DeleteRole) with a 1-minute context via rest.WithCtx, which disgo's REST rate limiter honors. Oversized retry_after windows now fail fast as context.DeadlineExceeded, which callers already classify (KindDeadlineExceeded) and log at debug.
  • sequencer.Submit never blocks: if a guild's queue is full the event is dropped (reported to the caller, logged at warn). A dropped event risks at most a stale ephemeral role — corrected by the member's next voice event — while blocking wedged the entire shard's event dispatch.
  • Bump StatefulSet image tag to v1.16.5.

Test plan

  • make lint — 0 issues
  • make test — 57 tests passing (with -race)
  • make build
  • Post-deploy: watch kubectl logs -n ephemeral-roles across all shards; shard 3 should either stay clean or log dropping VoiceStateUpdate event: guild queue full / debug-level deadline-exceeded instead of wedging.

Shard 3 re-entered its zombie-reconnect loop on v1.16.4. A goroutine
dump from the live pod (pprof) showed the actual failure chain:

- The guild's sequencer worker sat 96+ minutes inside CreateRole,
  blocked in disgo's REST rate limiter: Discord can answer role
  mutations with retry_after values of an hour or more, and none of
  our REST calls carried a deadline, so the rate limiter waited out
  the entire window.
- Voice events kept arriving for that guild until its 64-slot queue
  filled, at which point sequencer.Submit blocked the gateway read
  loop on the channel send - while it held disgo's event-manager
  mutex.
- Every reconnect's new read loop then blocked on that same mutex at
  its first dispatch, so heartbeat ACKs were never processed: zombie
  close, reconnect, wedge again, indefinitely. The same unbounded
  rate-limiter wait also explains the pre-sequencer (v1.16.2/v1.16.3)
  incidents, where the wait ran directly on the read loop.

Two fixes:

- All Discord REST calls (create/add/remove/delete role, get guild)
  now run under a 1-minute context via rest.WithCtx, which disgo's
  rate limiter honors. Oversized retry_after windows now fail fast as
  context.DeadlineExceeded, which callers already classify
  (KindDeadlineExceeded) and log at debug.
- sequencer.Submit no longer blocks when a guild's queue is full: the
  event is dropped and reported, with the drop logged at warn by the
  callbacks. A dropped event risks at most a stale ephemeral role,
  corrected by the member's next voice event; blocking wedged the
  whole shard.

Also bump the StatefulSet image tag to v1.16.5 to roll this out.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces non-blocking event submission to the guild sequencer to prevent blocking the main event loop, and adds a 1-minute request timeout context to all Discord REST operations. The review feedback highlights two key areas for improvement: first, dropping ChannelDelete events when the queue is full can lead to leaked roles, so running them asynchronously as a fallback is recommended; second, the 1-minute request timeout exceeds the Kubernetes 30-second termination grace period, which could cause forceful pod termination during rolling updates, suggesting an alignment of these values.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +27 to +31
if !accepted {
handler.Log.Warn("dropping ChannelDelete event: guild queue full",
"guildID", event.GuildID,
)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If the guild's event queue is full, dropping the ChannelDelete event means the ephemeral role associated with the deleted channel will never be deleted from Discord. Since Discord has a strict limit of 250 roles per guild, repeatedly leaking roles will eventually prevent the bot from creating new ephemeral roles in that guild.

Instead of dropping the event entirely, we can run the deletion asynchronously in a new goroutine as a fallback. Since the channel is already deleted, any concurrent or pending events in the queue for this channel will safely handle the missing channel anyway, making an asynchronous fallback safe and robust.

	if !accepted {
		handler.Log.Warn("guild queue full; deleting channel role asynchronously",
			"guildID", event.GuildID,
		)
		go handler.handleChannelDelete(event)
	}

// production). Failing fast with context.DeadlineExceeded instead lets
// callers classify the error (KindDeadlineExceeded) and drop the
// operation.
requestTimeout = 1 * time.Minute

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There is a mismatch between the REST request timeout (requestTimeout = 1 * time.Minute) and the Kubernetes termination grace period (terminationGracePeriodSeconds: 30 in statefulset.yml).

During a rolling update or pod deletion, Kubernetes sends a SIGTERM and waits up to 30 seconds for the pod to shut down. If the shutdown sequence attempts to flush/drain the queues (which can block up to 1 minute per request due to this timeout), the pod will be forcefully terminated with SIGKILL before it can complete gracefully.

To align these, consider either:

  1. Increasing terminationGracePeriodSeconds in deployments/kubernetes/statefulset.yml to at least 90 (allowing the 1-minute timeout plus buffer to complete).
  2. Or reducing requestTimeout to a lower value (e.g., 15s or 20s), if appropriate for your rate-limiting expectations.

A dropped ChannelDelete is never retried by a later event, so each drop
would permanently leak a role toward the guild's 250-role cap. On a
full queue, fall back to a goroutine that does a blocking enqueue
(SubmitWait): the gateway read loop stays unblocked and the deletion
still runs serialized on the guild's worker rather than racing it.
VoiceStateUpdate keeps plain dropping - those are high-volume and
self-correcting via the member's next voice event.

Raise terminationGracePeriodSeconds from 30 to 90 so in-flight REST
requests (bounded at 1 minute) have room to finish during shutdown.
@ewohltman

Copy link
Copy Markdown
Owner Author

Both review comments addressed in c10e56d:

  • ChannelDelete role leak: agreed on the problem, but running handleChannelDelete directly in a goroutine would bypass the sequencer and race the guild's worker (the ordering problem the sequencer exists to prevent). Instead, on a full queue the callback now spawns a goroutine that does a blocking enqueue (SubmitWait) — the gateway read loop stays unblocked, and the deletion still executes serialized on the guild's worker once capacity frees up. VoiceStateUpdate keeps plain drop semantics since those are high-volume and self-correcting via the member's next voice event.
  • Timeout vs grace period: raised terminationGracePeriodSeconds from 30 to 90 so in-flight REST requests (bounded at 1 minute) have headroom during shutdown. Worth noting shutdown doesn't currently flush guild queues — in-flight jobs die with the process — but the alignment is correct either way.

@ewohltman
ewohltman merged commit 9d7deca into master Jul 11, 2026
3 checks passed
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