Skip to content

Fall back when a topic loses every leader, count the drop, and refresh again (release-0.1) - #95

Merged
koloss2001 merged 4 commits into
release-0.1from
koloss2001/leader-drop-followup-v0.1.x
Oct 2, 2026
Merged

koloss2001 merged 4 commits into
release-0.1from
koloss2001/leader-drop-followup-v0.1.x

Conversation

@koloss2001

@koloss2001 koloss2001 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to the live-agent fallback in #88 (main) and #93 (release-0.1). This PR is against release-0.1. Port to main after it merges.

#93 falls back only when some other partition of the topic still has a leader. If every leader for a topic is excluded in one refresh, Candidates still returns nil and ProduceSync fails the whole batch, including healthy topics. This change treats that topic as known and hashes it onto a live agent. A topic Metadata has never returned, and a topic-level error, stay unknown so they still trigger an on-demand refresh.

A refresh that excludes a leader increments warpstream_agentpool_leader_dropped_total (once per excluded leader, including the constructor) and logs the count plus the first excluded leader (first_topic, first_partition, first_node_id). The background refresh nudges another fetch. After a periodic tick that follow-up starts at once; later repeats wait out OnDemandMetadataRefreshInterval (default 1s). The constructor counts and logs, and does not nudge. Produce does not wait.

This also covers the exact broker/topic mismatch RequestCachedMetadata can return, described upstream in twmb/franz-go#1481: a leader id present in Topics but missing from that same response's Brokers now hashes onto a live agent instead of failing the batch.

Also addresses the #88 review comment on the benchmark sink: the comment now describes the benchmark's own allocation count, not how AgentPool.Refresh uses the result.

Left for follow-up PRs, still part of the leader-map fallback:

  • Partition isolation. A topic Metadata has never returned, a topic-level error, or an empty agent pool still makes ProduceSync fail the whole batch. The next PR routes the partitions that have a candidate and fails only the ones that do not.
  • Stale leader still in the map. The named leader is present but the agent is gone. A later PR refreshes on unknown broker, dial failure, and a short list of topology errors. It does not retry the write already in flight. Timeouts and connection resets stay out.

@koloss2001
koloss2001 requested review from a team as code owners September 30, 2026 23:37

@stephclay stephclay left a comment

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.

Two inline findings on the new leader-drop path. The knownTopics change is correct. A topic goes into the set only when Metadata listed partitions for it and the client kept none. Topics with a topic-level error or no partitions stay out, and this matches the README.

— via Claude Code

Comment thread pkg/wgo/client.go
Comment thread pkg/wgo/agentpool.go
@koloss2001
koloss2001 requested review from stephclay and a balanced review from Copilot October 2, 2026 00:39

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The exported AgentPool.Refresh signature introduces a source-incompatible API change, and the affected configuration documentation is stale.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)
What changed in this PR

Adds resilient routing when metadata drops all leaders for a known topic, with observability and follow-up refreshes.

Changes:

  • Falls back to live agents while preserving unknown/no-leader behavior.
  • Counts, logs, and refreshes after excluded leaders.
  • Adds routing, metadata, metric, and integration coverage.
File Description
README.md Documents fallback behavior.
pkg/​wgo/​partition_assignment.go Tracks known topics and leaderless partitions.
pkg/​wgo/​partition_assignment_test.go Tests fallback selection.
pkg/​wgo/​metrics.go Adds the dropped-leader counter.
pkg/​wgo/​demoter_test.go Updates constructor usage.
pkg/​wgo/​client.go Records drops and schedules refreshes.
pkg/​wgo/​client_test.go Tests routing, metrics, and nudging.
pkg/​wgo/​agentpool.go Detects and reports excluded leaders.
pkg/​wgo/​agentpool_test.go Tests metadata classification.
docs/​internal/​metrics.md Documents the metric category.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/wgo/agentpool.go Outdated
Comment thread pkg/wgo/client.go

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The README’s metadata-refresh overview contradicts the newly added successful fallback follow-up path.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
Resolved since last review (1)

Comment thread README.md Outdated

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The fallback, refresh, observability, compatibility, tests, and documentation changes are consistent and complete.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@koloss2001
koloss2001 merged commit f1517df into release-0.1 Oct 2, 2026
13 checks passed
@koloss2001
koloss2001 deleted the koloss2001/leader-drop-followup-v0.1.x branch October 2, 2026 17:47
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