kafka: make topic manager shutdown interrupt in-flight metadata refresh
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 56
- Forks
- 63
- Avg merge
- 2d 17h
- Merged PRs (30d)
- 39
Description
What problem does this issue track?
The Kafka topic manager refreshes partition metadata every ten minutes. Its background goroutine calls AdminClient.GetTopicsPartitionsNum, which does not accept a context and eventually calls Sarama Client.Partitions.
PR #6081 briefly added a WaitGroup and made TopicManager.Close cancel the background context and wait for the goroutine. That ordering can delay sink shutdown when a refresh is already in progress:
- The background goroutine enters
GetTopicsPartitionsNum. - Kafka is slow or unavailable, so the Sarama metadata request blocks.
TopicManager.Closecancels its context and waits for the goroutine.- The in-flight metadata call cannot observe that context.
- The admin client that could close the Sarama client is closed only after
TopicManager.Closereturns.
The wait can therefore last until the Kafka metadata request returns or reaches its configured timeout. With many topics, normal refresh work can also make shutdown latency proportional to the topic count.
The WaitGroup change is being reverted from PR #6081 so that the topic-visibility fix does not introduce this shutdown regression. The existing cancel-only behavior still does not guarantee that the background goroutine has exited before the admin client is released, so the complete lifecycle fix belongs in a separate change.
Expected behavior
Kafka sink shutdown should follow an explicit lifecycle:
- Stop scheduling new topic metadata refreshes.
- Interrupt or unblock an in-flight Kafka metadata request.
- Wait for the topic-manager background goroutine to exit.
- Release the remaining sink resources.
The component that owns the admin client should also own this shutdown ordering. Cancellation, client ownership, and waiting should not depend on an implicit call order between unrelated Close methods.
Suggested implementation direction
- Split topic-manager shutdown into a non-blocking stop/cancel phase and a wait phase, or make the metadata API genuinely context-aware down to the operation that can interrupt Sarama IO.
- Close the dedicated admin client between stop and wait so an in-flight
Partitionscall can return. - Keep stop and close operations idempotent.
- Avoid adding a wait before there is a path that can interrupt the operation being waited on.
Acceptance criteria
- Add a deterministic test with a fake admin client whose metadata call blocks until the client is closed.
- Assert the shutdown order is stop topic refresh, close admin client, then wait for topic-manager exit.
- Assert sink/component shutdown returns after the blocked metadata call is released.
- Run the affected Kafka sink and topic-manager tests with the race detector.
Related to #6076 and #6081.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Locate TopicManager, its owning Kafka sink component, and the AdminClient.GetTopicsPartitionsNum path through Sarama Client.Partitions. Read the affected topic-manager and Kafka sink tests first, then add the blocking fake-admin-client test described in the acceptance criteria. Done means shutdown stops refreshes, closes the admin client, waits for the goroutine, and passes the affected tests with the race detector.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kafka
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100