Skip to content

Two bugs seen from the SQL editor, and the KIP-848 protocol as the configured default - #271

Merged
fracompagnon-sudo merged 3 commits into
mainfrom
claude/sql-editor-full-text-replace-f7amww
Aug 24, 2026
Merged

Two bugs seen from the SQL editor, and the KIP-848 protocol as the configured default#271
fracompagnon-sudo merged 3 commits into
mainfrom
claude/sql-editor-full-text-replace-f7amww

Conversation

@fracompagnon-sudo

@fracompagnon-sudo fracompagnon-sudo commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Description

Three independent changes, one commit each. The third arrived after the first two were green; it is a configuration change with no relation to the bugs, and it is easiest reviewed on its own commit (4de5387) — say so and it can be split out into its own pull request.

1. IllegalStateException: MiniCluster is not yet running or has already been shut down

The embedded runtime gives each job its own MiniCluster and takes it down when the job reaches a terminal state, so every later call on that JobClient throws — synchronously, from Flink's own Preconditions.checkState. Three call sites read that as a failure when it is an answer about the job: it is over.

  • cancelQuery called cancel() unguarded, so pressing Stop as a query finishes — the most ordinary race there is — answered 500 with a stack trace about a MiniCluster the operator does not know they are running. It now reports NO_ACTIVE_JOB, which is what actually happened: nothing was cancelled. Saying CANCELLED there is the exact claim CancelOutcome exists to prevent, one step further along than the case it was written for.
  • cancelJobInternal runs from inside a catch and from the timeout branch, where the caller already holds the answer it owes the user — so a cleanup that throws replaced both the real error and the "query timed out" result (with its remedy) by this trace. Best-effort now, logged at debug.
  • buildJobSummary caught every exception into "UNKNOWN" and left endedAt null, so a job whose runtime was gone never left activeJobs. The three callers of getActiveJobsDetails() act on that answer: POST /api/config refuses a cluster repoint with 409, the lineage graph draws a node, the KPI suggestions derive an edge — a finished query went on refusing the operator's next settings save for the rest of the process. A gone runtime now marks the job ended; the status stays UNKNOWN, since we know it ended and not how.

2. The Window Assistant wrote the dotted topic name

resolveScope resolves a cited name to the catalogue key, and for a topic that key is the dotted name (demo.orders.1.received). The assistant wrote it into the SQL verbatim, while the table is registered as demo_orders_1_receivedtoTableName turns dots and hyphens into underscores. Flink read it as a four-part identifier (catalog.database.table…) and resolved nothing, so every query the assistant produced on a topic failed on a name that looks right.

The rule was already applied on both sides of that one line: the sidebar poses SELECT * FROM under the table name, and the schema consulted two lines below was already looked up through toTableName. Only the name written into the query stayed raw. The panel renders the same value, so it now names the table the query will actually reach.

3. kafka.consumer-group-protocol ships as consumer

It was classic in application.yml while every bundled stack — docker-compose.yml, -kafka4, -llm, -dev, -spectra, -spectra-hub and .release — set KAFKA_CONSUMER_GROUP_PROTOCOL=consumer explicitly. The shipped configuration was the one nothing here actually runs: those stacks all run Kafka 4.3 in KRaft mode, which is what KIP-848 needs.

What it costs is stated where it is paid — in the YAML comment, in the DOCKERHUB.md table, in FEATURES.md and in CLAUDE.md: consumer requires a Kafka 4.x broker, so a deployment against an older one sets classic. That value stays the fallback in KafkaConfig, so the code default is still the compatible one and only the shipped YAML asserts the newer protocol — an absent property behaves exactly as before.

docs/check-config-table.py resolves a documented default against application.yml, so the DOCKERHUB.md row had to move with it.

Related Issue

None — the two bugs were reported directly from a running deployment; the configuration change was asked for separately.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update — the shipped default of KAFKA_CONSUMER_GROUP_PROTOCOL moved, so the four places that stated it moved with it

No API shape changes: CancelOutcome keeps its two values, so nothing moves in api/types.ts.

How Has This Been Tested?

  • Java suite: 791 tests, 0 failures, through ./verify-offline.shpackages.confluent.io answers 403 from the machine this was written on, which is the situation that script exists for; CI's build job has since run the same suite against the real jars and is green.
  • Frontend gate: tsc --noEmit clean, ESLint clean (--max-warnings 0), 1307 Vitest cases green.
  • Documentation checks: check-config-table.py, check-compose.py, check-links.py, check-doc-paths.py, check-api-types.py — all resolve.

Four tests added, each verified by mutation — reverted the fix, watched the test fail, restored it:

  • cancelQuerySaysNothingWasCancelledWhenTheJobHasAlreadyFinished and aJobWhoseRuntimeIsGoneStopsBeingActive fail against the previous code with exactly the reported exception.
  • aJobWhoseStatusTimesOutStaysActive passes on both sides, deliberately: a status call that merely times out says nothing about the job, so the job must stay active — otherwise the fix would weaken the 409 guard on a slow runtime.
  • names the Flink table, not the dotted topic asserts both halves — what the panel displays and what the query contains.

The configuration commit carries no test: what would assert it is check-config-table.py, which already does, by refusing a documented default that no longer matches application.yml.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules — n/a

🤖 Generated with Claude Code

https://claude.ai/code/session_01GNi5imVLugHPr3hnzUpyHw

claude added 2 commits August 24, 2026 12:23
`IllegalStateException: MiniCluster is not yet running or has already been shut
down` was reaching users. The embedded runtime gives each job its own
MiniCluster and takes it down when the job reaches a terminal state, so every
later call on that JobClient throws — synchronously, from Flink's own
`Preconditions.checkState`. Three call sites took that exception as a failure
when it is an answer about the job: it is over.

- `cancelQuery` called `cancel()` unguarded, so pressing Stop as a query
  finishes — the most ordinary race there is — answered 500 with a stack trace
  about a MiniCluster the operator does not know they are running. It now
  reports NO_ACTIVE_JOB, which is what actually happened: nothing was
  cancelled. Saying CANCELLED there is the exact claim `CancelOutcome` exists
  to prevent, one step further along than the case it was written for.
- `cancelJobInternal` runs from inside a `catch` and from the timeout branch,
  where the caller already holds the answer it owes the user — so a cleanup
  that throws replaced both the real error and the "query timed out" result
  with this trace. It is best-effort now, logged at debug.
- `buildJobSummary` caught every exception into "UNKNOWN" and left `endedAt`
  null, so a job whose runtime was gone never left `activeJobs`:
  `getActiveJobsDetails()` went on reporting a finished query as running for
  the rest of the process — `POST /api/config` refusing a cluster repoint with
  409, the lineage graph drawing a node, the KPI suggestions deriving an edge.
  A gone runtime now marks the job ended; the status stays UNKNOWN, since we
  know it ended and not how.

Three tests, verified by mutation: the first two fail against the previous code
with exactly this exception, and the third — a status call that merely times
out leaves the job active — passes on both, so the fix cannot be over-applied
to a slow runtime.

Full Java suite offline: 791 tests, 0 failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNi5imVLugHPr3hnzUpyHw
`resolveScope` resolves a cited name to the *catalogue* key, and for a topic
that key is the dotted name (`demo.orders.1.received`). The assistant wrote it
into the SQL verbatim, while the table is registered as
`demo_orders_1_received` — `toTableName` turns dots and hyphens into
underscores. Flink therefore read `demo.orders.1.received` as a four-part
identifier (catalog.database.table…) and resolved nothing, so every query the
assistant produced on a topic failed on a name that looks right.

The rule was already applied on both sides of this one line: the sidebar poses
`SELECT * FROM` under the table name, and the schema consulted two lines below
was already looked up through `toTableName`. Only the name written into the
query stayed raw. The panel renders the same value, so it now names the table
the query will actually reach.

Pinned by a component test, verified by mutation: it fails against the previous
code and passes here. Frontend gate: tsc clean, ESLint clean, 1307 tests green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNi5imVLugHPr3hnzUpyHw
`kafka.consumer-group-protocol` was `classic` in application.yml while every
bundled stack — docker-compose.yml, -kafka4, -llm, -dev, -spectra, -spectra-hub
and .release — set `KAFKA_CONSUMER_GROUP_PROTOCOL=consumer` explicitly. The
shipped configuration was therefore the one nothing here actually runs: all
those stacks run Kafka 4.3 in KRaft mode, which is what KIP-848 needs.

What it costs is stated where it is paid — in the YAML comment, in the
DOCKERHUB.md table, in FEATURES.md and in CLAUDE.md: `consumer` **requires a
Kafka 4.x broker**, so a deployment against an older one sets `classic`. That
value stays the fallback in `KafkaConfig`, so the *code* default is still the
compatible one and only the shipped YAML asserts the newer protocol — an
absent property behaves exactly as before.

`docs/check-config-table.py` resolves a documented default against
application.yml, so the DOCKERHUB.md row had to move with it; it and the three
other documentation checks pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNi5imVLugHPr3hnzUpyHw
@fracompagnon-sudo fracompagnon-sudo changed the title Two bugs seen from the SQL editor: a finished job's cancellation, and a dotted table name Two bugs seen from the SQL editor, and the KIP-848 protocol as the configured default Aug 24, 2026
@fracompagnon-sudo
fracompagnon-sudo merged commit 387478c into main Aug 24, 2026
22 of 23 checks passed
@fracompagnon-sudo
fracompagnon-sudo deleted the claude/sql-editor-full-text-replace-f7amww branch August 24, 2026 13:53
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.

2 participants