Skip to content

fix(topics): a created topic is described once the broker knows it - #564

Merged
bmscomp merged 2 commits into
mainfrom
fix/topic-create-describe-retry
Oct 30, 2026
Merged

bmscomp merged 2 commits into
mainfrom
fix/topic-create-describe-retry

Conversation

@bmscomp

@bmscomp bmscomp commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Why

main is red again after #563 (d120b8c, run 41136562284), on a different test: KafkaApiIT.producedRecordIsReadableBackThroughTheConsumeEndpoint. Its POST /api/kafka/topics answered 500 for a topic that was created: the log has Created topic: produce-consume-it-a8747ac2 at 22:20:46.135 and then "Failed to describe topic".

CreateTopics returns once the controller has the topic, but the broker that answers the describe can still call it unknown until its metadata cache catches up. A fresh test broker shows it most; a client on a real cluster can hit it too, and gets a 500 for a create that worked. Nothing from #560/#561/#563 touches this path.

What

  • TopicService.describeCreatedTopic retries describeTopicDetail every 200 ms, up to 10 times, and POST /api/kafka/topics uses it. It retries only the broker's UnknownTopicOrPartition answer (review: retrying every failure held a request for ~5.5 min when Kafka doesn't answer); anything else surfaces at once, which a second unit test checks.
  • A plain GET /api/kafka/topics/{name} keeps its immediate answer.
  • MCP comment refs to TopicService.java re-pointed past the 12 new lines.

Tests

Locally:

  • TopicServiceTest.aCreatedTopicIsDescribedOnceTheBrokerKnowsIt: the broker answers UnknownTopicOrPartition twice, then knows the topic. It fails with the @Retry removed (checked).
  • TopicServiceTest, KafkaClientResourceTest, and KafkaApiIT on a real Kafka (Testcontainers), all green.
  • spotless:apply on the changed files; go test ./cmd -run TestMCPCaveat.
  • The full ./mvnw verify on 032fbc4: 1784 unit tests and 98 ITs, 0 failures, coverage gate passed. golangci-lint: 0 issues. On d638298 (the review fix): TopicServiceTest, KafkaClientResourceTest, KafkaApiIT, spotless, TestMCPCaveat, and the full ./mvnw verify: 1785 unit tests and 98 ITs, 0 failures, coverage gate passed.

I couldn't force the original race on a real broker; the unit test reproduces the broker's answer.

POST /api/kafka/topics answered 500 for a topic it had just created on
main's CI (d120b8c, run 41136562284, KafkaApiIT): the topic was created
at 22:20:46.135, and describing it right after failed with "Failed to
describe topic". CreateTopics returns once the controller has the topic,
and a broker can still answer it unknown until its metadata catches up,
which a fresh test broker shows most.

TopicService.describeCreatedTopic retries describeTopicDetail every
200 ms, up to 10 times, and the create endpoint uses it. A plain GET of a
topic keeps its immediate answer. TopicServiceTest has the broker answer
UnknownTopicOrPartition twice before it knows the topic; without the retry
the test fails.

MCP comment refs to TopicService.java re-pointed past the 12 new lines.

@graphify-labs graphify-labs 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.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.

Not checked: tests were not run; no formal proof of the changed code.


Graphify review — findings

Fixes the intermittent 500 on POST /api/kafka/topics by describing a freshly created topic through the new describeCreatedTopic, which retries up to 10 times at 200 ms intervals while the broker's metadata catches up. The retry applies to any exception, so a genuine describe failure can hold the request for a long time before surfacing, and the new unit test expects the @Retry interceptor to fire on a plainly instantiated service. Also updates the TopicService.java line references in the MCP caveat and topology-detail error comments to match the shifted source.

Worth a look

  • Retry on every exception can hold a request for minutes — kates/src/main/java/com/bmscomp/kates/service/TopicService.java:135 · Escalate · medium
    • NOT verified (no proof, no reproducing execution) — needs human review
  • Unit test relies on the @Retry interceptor, which does not run on a plain-instantiated service — kates/src/test/java/com/bmscomp/kates/service/TopicServiceTest.java:160 · Escalate · medium
    • NOT verified (no proof, no reproducing execution) — needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 521 functions depend on the 113 functions this change touches.

Health — grade B; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):

  • newMCPHarness() — 226 callers, 10 callees (high)
  • mcpAssessRun() — 2 callers, 17 callees (high)
  • mcpGetRun() — 1 callers, 20 callees (high)
  • mcpRunRowFrom() — 3 callers, 5 callees (medium)
  • newMCPServer() — 2 callers, 7 callees (medium)
  • mcpScnCheckScenario() — 1 callers, 13 callees (high)
  • mcpListRuns() — 1 callers, 10 callees (medium)
  • runMCP() — 1 callers, 9 callees (medium)
  • …and 2 more

Verification — 521 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 261 function(s) in the blast radius were not formally verified this run

Review on #564: describeCreatedTopic retried every failure, so a broker
that does not answer held the request for 11 describe timeouts, about
5.5 minutes, before the 500. It now retries only the broker's
UnknownTopicOrPartition answer, which it rethrows as such for @Retry's
retryOn; anything else surfaces at once. TopicServiceTest checks that a
timed-out describe is tried once.

The new import moved TopicService.java down a line: MCP refs re-pointed,
the anchored ones included.
@bmscomp
bmscomp merged commit 44df715 into main Oct 30, 2026
58 checks passed
@bmscomp
bmscomp deleted the fix/topic-create-describe-retry branch October 30, 2026 23:00
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