Skip to content

kfake: make faults consistent across requests; let When call the cluster - #1479

Merged
twmb merged 1 commit into
masterfrom
kfake-fault-rules
Sep 27, 2026
Merged

twmb merged 1 commit into
masterfrom
kfake-fault-rules

Conversation

@twmb

@twmb twmb commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Faults behaved differently depending on the request. This makes them follow one set of rules, documents those rules, and fixes handler bugs an audit of all 62 handlers turned up.

Rules

  • Each check site knows every identifier around the entity it checks, including parents: a partition in a group commit is also matched by Group, one in a transaction by TxnID. Unset selectors still match anything, so Partitions alone still matches that partition in every request. The topic name and ID are filled in centrally, so Topic and TopicID both work whether a request carries names or IDs.
  • A fault is checked after ACLs and after routing (NOT_COORDINATOR, NOT_CONTROLLER, NOT_LEADER_OR_FOLLOWER for a partition that exists), and before existence checks. A fault fires only on the broker that would handle the request, and it can fail an entity that does not exist, including an unknown topic ID. For Fetch, a follower counts as the right broker. Two exceptions: an Observe fault counts the request on any broker, and a fault that sets Nodes fires on those brokers even when they are the wrong one, to simulate a leader or coordinator that has not learned it was replaced.
  • TopLevel matches Group/TxnID against the request and rejects other selectors. It fires only on versions whose response serializes a top-level ErrorCode. OffsetFetch v0-1, DescribeLogDirs v0-2, and ElectLeaders v0 put a request-level fault on each entity instead.

When can call Cluster methods. While a When runs, admin runs its function inline under a mutex. Methods that wait for the cluster to make progress (WaitGroupInfo, FaultHandle.Wait) and Close still can't be called from When.

Handler fixes

  • AddPartitionsToTxn v4-5 was advertised, but the handler never read the batched Transactions field. It now handles batches and VerifyOnly as Kafka does, with CLUSTER_ACTION required for v4+. At every version, an unauthorized, unknown, or faulted partition fails the transaction, and the rest get OPERATION_NOT_ATTEMPTED.
  • TxnOffsetCommit v6 dropped unknown-ID topics from an error response.
  • ConsumerGroupHeartbeat created an empty group before rejecting the heartbeat.
  • GROUP config resources had no ACL or fault check. They now check DESCRIBE_CONFIGS / ALTER_CONFIGS on the group.
  • Timed-out AlterUserSCRAMCredentials and share offset alter/delete faults were counted but never answered.

Test changes

  • TestFaultObserveAndWhen: its When now calls TopicInfo.
  • TestFaultTopLevel: a v6 Fetch must not be faulted.
  • TestTxnDescribeTransactions: runs a v4 VerifyOnly check.
  • TestFaultNode: a fault without Nodes leaves a non-leader to answer NOT_LEADER; one naming the old leader fires there.

Behavior changes for existing kfake users

  • With ACLs enabled, GROUP config resources now need DESCRIBE_CONFIGS / ALTER_CONFIGS on the group, as in Kafka.
  • An AddPartitionsToTxn partition fault now fails that partition and answers OPERATION_NOT_ATTEMPTED for the rest of the transaction, as in Kafka, rather than failing the whole request with the fault's code.

Changelog

  • kfake: faults follow one set of rules across requests (keys name every identifier, checked after ACLs and routing), TopLevel faults honor Group/TxnID and fire only when the code reaches the wire, Fault.When can call Cluster methods, and AddPartitionsToTxn supports v4-5 batches and VerifyOnly.

@twmb
twmb force-pushed the kfake-fault-rules branch 5 times, most recently from d6d729e to 00c4a5c Compare September 27, 2026 18:53
What a fault selector matched depended on the request. TopicID was
filled at a few sites only, nested keys dropped their group or txnID,
and faults fired before the coordinator check on some requests and
after it on others, before ACLs on some, and on non-leaders for
partitions. Faults now follow one rule: the key names every identifier
known at the site, topic name and ID filled in centrally, and a fault
is checked after ACLs and routing (NOT_COORDINATOR, NOT_CONTROLLER,
NOT_LEADER_OR_FOLLOWER) and before existence checks. The ACL and the
fault stay one deny call; a misrouted key skips faults that answer. An
Observe fault still counts a misrouted request, and a fault that names
Nodes still fires there, to simulate a leader or coordinator that has
not learned it was replaced.

A TopLevel fault ignored its selectors and fired on versions whose
response has no top-level ErrorCode, which the client saw as an empty
success. It now matches Group and TxnID against the request, rejects
other selectors, and fires only when the code reaches the wire.
OffsetFetch v0-1, DescribeLogDirs v0-2, and ElectLeaders v0 put a
request-level fault on each entity instead of an unserialized field.

When ran on the cluster goroutine, so a When that called TopicInfo
deadlocked. While a When runs, admin now serves Cluster methods inline.

Handler fixes found along the way: AddPartitionsToTxn v4-5 was
advertised but never read the batched Transactions field; it now adds
and verifies batches as Kafka does. TxnOffsetCommit v6 dropped
unknown-ID topics from an error response. ConsumerGroupHeartbeat
created a group before rejecting the heartbeat. Group configs had no
ACL or fault check. Timed-out AlterUserSCRAMCredentials and share
offset alter/delete faults were counted but never answered.
@twmb
twmb force-pushed the kfake-fault-rules branch from 00c4a5c to f77970d Compare September 27, 2026 18:54
@twmb
twmb merged commit 67a270e into master Sep 27, 2026
13 of 14 checks passed
@twmb
twmb deleted the kfake-fault-rules branch September 27, 2026 20:04
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