Skip to content

admin: reuse cluster._coordinators; use CoordinatorType#3050

Merged
dpkp merged 3 commits into
masterfrom
kip-664-prep-coordinator-cache
Jun 9, 2026
Merged

admin: reuse cluster._coordinators; use CoordinatorType#3050
dpkp merged 3 commits into
masterfrom
kip-664-prep-coordinator-cache

Conversation

@dpkp

@dpkp dpkp commented Jun 9, 2026

Copy link
Copy Markdown
Owner

No description provided.

dpkp added 3 commits June 9, 2026 14:58
…collide with group lookups

Prep for KIP-664 (DescribeTransactions). FindCoordinator's KeyType
distinguishes group (0) from transaction (1) coordinators, and the
same string id can legitimately be both. The previous
`_coordinator_cache` was keyed by raw key only, so a transaction
coordinator lookup could silently return a group-coordinator node id
(and vice versa).

  - `_find_coordinator_id(s)` gain a `key_type` parameter, default 0.
  - `_coordinator_cache` is now keyed by `(key_type, key)`.
  - All existing callers (consumer-group paths) keep the default
    `key_type=0` and behaviour is unchanged.
…cache

KafkaAdminClient owned its own `_coordinator_cache` dict that
mirrored ClusterMetadata._coordinators in both shape (keyed by
(CoordinatorType, key)) and lifecycle. Drop the duplicate.

  - _find_coordinator_ids now reads through cluster.get_coordinator
    and writes through cluster.add_coordinator(synthesize_node_id=False)
    so admin keeps reusing the existing broker connection (unlike
    the producer's TransactionManager which gets a dedicated
    `coordinator-N` connection).
  - The v4+ Coordinator row is duck-compatible with the v0-v3
    response shape (both have error_code/error_message/node_id/host/port),
    so add_coordinator handles either with no adapter.
  - Per-row errors raise from add_coordinator now, so the explicit
    error_code check in the admin path is gone too.
@dpkp dpkp merged commit 9a01553 into master Jun 9, 2026
19 checks passed
@dpkp dpkp deleted the kip-664-prep-coordinator-cache branch June 9, 2026 22:07
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