perf: load test every implemented protocol version, fix readUUID - #352
Merged
Conversation
The compatibility branch made Base[kGetApi] able to negotiate down to Produce v3 and Fetch v4, but every broker in CI negotiates to the newest version, so the legacy codecs were covered for correctness and never for speed. This adds the harness to measure them and records the results. The experiment pins the client codec against a fixed modern broker, so the protocol version is the only variable. Running against the 1.1.0 stack instead would vary the JVM, the storage engine and the codec at once, so that runs as a labelled sanity check rather than a measurement. Verdict: the legacy codecs are not slower. Across 9 Produce versions, 14 Fetch versions, three payload shapes and two acks settings, no legacy version is consistently slower than the newest. Where a reproducible difference exists it runs the other way. Two findings, both in the newest versions: - Reader.readUUID hyphenated with a capture group regex, 801ns against 56ns for the equivalent slicing. Fetch v13+ identifies topics by UUID, so this made the newest Fetch versions 15-18% slower to decode than the ones using topic names. Fixed here; 1653 protocol tests unchanged. - Flexible framing costs ~23% per message at 1MB fetch responses, with the break exactly at the v11/v12 boundary, reproduced over four runs. The mechanism is not isolated and is recorded rather than guessed at. LOAD_TESTING.md also records what the measurement cannot support: tier 1 Fetch at maxBytes=4096 and tier 0 Fetch decode are too noisy on this box for the 15% threshold, and the methodology errors found along the way - fixed record timestamps triggering broker retention mid-sweep, and a constant shuffle seed confounding version with position - both of which first presented as findings about the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA
Covers how to run the suite, what each script measures, and the configuration. The larger half is how to get numbers that mean something: vary the shuffle seed before believing any single-version result, pin CPUs, run one sweep at a time, and know the noise floor. Each of those countermeasures exists because its absence produced a plausible finding that turned out to be an artifact, so the README says which artifact and how it presented rather than just stating the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA
The plan document has served its purpose now that the suite exists and has run. Its two durable halves move into benchmarks/protocol-versions/README.md: the recorded verdict and the findings, which would otherwise survive only in the PR description, and the tier and guard naming that the source comments refer to. Nine references across seven files pointed at the deleted path; all now point at the README. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA
mcollina
added a commit
that referenced
this pull request
Aug 8, 2026
* perf: load test every implemented protocol version, fix readUUID The compatibility branch made Base[kGetApi] able to negotiate down to Produce v3 and Fetch v4, but every broker in CI negotiates to the newest version, so the legacy codecs were covered for correctness and never for speed. This adds the harness to measure them and records the results. The experiment pins the client codec against a fixed modern broker, so the protocol version is the only variable. Running against the 1.1.0 stack instead would vary the JVM, the storage engine and the codec at once, so that runs as a labelled sanity check rather than a measurement. Verdict: the legacy codecs are not slower. Across 9 Produce versions, 14 Fetch versions, three payload shapes and two acks settings, no legacy version is consistently slower than the newest. Where a reproducible difference exists it runs the other way. Two findings, both in the newest versions: - Reader.readUUID hyphenated with a capture group regex, 801ns against 56ns for the equivalent slicing. Fetch v13+ identifies topics by UUID, so this made the newest Fetch versions 15-18% slower to decode than the ones using topic names. Fixed here; 1653 protocol tests unchanged. - Flexible framing costs ~23% per message at 1MB fetch responses, with the break exactly at the v11/v12 boundary, reproduced over four runs. The mechanism is not isolated and is recorded rather than guessed at. LOAD_TESTING.md also records what the measurement cannot support: tier 1 Fetch at maxBytes=4096 and tier 0 Fetch decode are too noisy on this box for the 15% threshold, and the methodology errors found along the way - fixed record timestamps triggering broker retention mid-sweep, and a constant shuffle seed confounding version with position - both of which first presented as findings about the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: add a README for the protocol version benchmarks Covers how to run the suite, what each script measures, and the configuration. The larger half is how to get numbers that mean something: vary the shuffle seed before believing any single-version result, pin CPUs, run one sweep at a time, and know the noise floor. Each of those countermeasures exists because its absence produced a plausible finding that turned out to be an artifact, so the README says which artifact and how it presented rather than just stating the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: remove LOAD_TESTING.md, fold its results into the suite README The plan document has served its purpose now that the suite exists and has run. Its two durable halves move into benchmarks/protocol-versions/README.md: the recorded verdict and the findings, which would otherwise survive only in the PR description, and the tier and guard naming that the source comments refer to. Nine references across seven files pointed at the deleted path; all now point at the README. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ShogunPanda
pushed a commit
that referenced
this pull request
Aug 17, 2026
* perf: load test every implemented protocol version, fix readUUID The compatibility branch made Base[kGetApi] able to negotiate down to Produce v3 and Fetch v4, but every broker in CI negotiates to the newest version, so the legacy codecs were covered for correctness and never for speed. This adds the harness to measure them and records the results. The experiment pins the client codec against a fixed modern broker, so the protocol version is the only variable. Running against the 1.1.0 stack instead would vary the JVM, the storage engine and the codec at once, so that runs as a labelled sanity check rather than a measurement. Verdict: the legacy codecs are not slower. Across 9 Produce versions, 14 Fetch versions, three payload shapes and two acks settings, no legacy version is consistently slower than the newest. Where a reproducible difference exists it runs the other way. Two findings, both in the newest versions: - Reader.readUUID hyphenated with a capture group regex, 801ns against 56ns for the equivalent slicing. Fetch v13+ identifies topics by UUID, so this made the newest Fetch versions 15-18% slower to decode than the ones using topic names. Fixed here; 1653 protocol tests unchanged. - Flexible framing costs ~23% per message at 1MB fetch responses, with the break exactly at the v11/v12 boundary, reproduced over four runs. The mechanism is not isolated and is recorded rather than guessed at. LOAD_TESTING.md also records what the measurement cannot support: tier 1 Fetch at maxBytes=4096 and tier 0 Fetch decode are too noisy on this box for the 15% threshold, and the methodology errors found along the way - fixed record timestamps triggering broker retention mid-sweep, and a constant shuffle seed confounding version with position - both of which first presented as findings about the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: add a README for the protocol version benchmarks Covers how to run the suite, what each script measures, and the configuration. The larger half is how to get numbers that mean something: vary the shuffle seed before believing any single-version result, pin CPUs, run one sweep at a time, and know the noise floor. Each of those countermeasures exists because its absence produced a plausible finding that turned out to be an artifact, so the README says which artifact and how it presented rather than just stating the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: remove LOAD_TESTING.md, fold its results into the suite README The plan document has served its purpose now that the suite exists and has run. Its two durable halves move into benchmarks/protocol-versions/README.md: the recorded verdict and the findings, which would otherwise survive only in the PR description, and the tier and guard naming that the source comments refer to. Nine references across seven files pointed at the deleted path; all now point at the README. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ShogunPanda
pushed a commit
that referenced
this pull request
Aug 25, 2026
* perf: load test every implemented protocol version, fix readUUID The compatibility branch made Base[kGetApi] able to negotiate down to Produce v3 and Fetch v4, but every broker in CI negotiates to the newest version, so the legacy codecs were covered for correctness and never for speed. This adds the harness to measure them and records the results. The experiment pins the client codec against a fixed modern broker, so the protocol version is the only variable. Running against the 1.1.0 stack instead would vary the JVM, the storage engine and the codec at once, so that runs as a labelled sanity check rather than a measurement. Verdict: the legacy codecs are not slower. Across 9 Produce versions, 14 Fetch versions, three payload shapes and two acks settings, no legacy version is consistently slower than the newest. Where a reproducible difference exists it runs the other way. Two findings, both in the newest versions: - Reader.readUUID hyphenated with a capture group regex, 801ns against 56ns for the equivalent slicing. Fetch v13+ identifies topics by UUID, so this made the newest Fetch versions 15-18% slower to decode than the ones using topic names. Fixed here; 1653 protocol tests unchanged. - Flexible framing costs ~23% per message at 1MB fetch responses, with the break exactly at the v11/v12 boundary, reproduced over four runs. The mechanism is not isolated and is recorded rather than guessed at. LOAD_TESTING.md also records what the measurement cannot support: tier 1 Fetch at maxBytes=4096 and tier 0 Fetch decode are too noisy on this box for the 15% threshold, and the methodology errors found along the way - fixed record timestamps triggering broker retention mid-sweep, and a constant shuffle seed confounding version with position - both of which first presented as findings about the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: add a README for the protocol version benchmarks Covers how to run the suite, what each script measures, and the configuration. The larger half is how to get numbers that mean something: vary the shuffle seed before believing any single-version result, pin CPUs, run one sweep at a time, and know the noise floor. Each of those countermeasures exists because its absence produced a plausible finding that turned out to be an artifact, so the README says which artifact and how it presented rather than just stating the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: remove LOAD_TESTING.md, fold its results into the suite README The plan document has served its purpose now that the suite exists and has run. Its two durable halves move into benchmarks/protocol-versions/README.md: the recorded verdict and the findings, which would otherwise survive only in the PR description, and the tier and guard naming that the source comments refer to. Nine references across seven files pointed at the deleted path; all now point at the README. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ShogunPanda
added a commit
that referenced
this pull request
Aug 25, 2026
* feat: Added support for all Kafka API older versions. Signed-off-by: Paolo Insogna <paolo@cowtech.it> * fixup Signed-off-by: Paolo Insogna <paolo@cowtech.it> * fixup Signed-off-by: Paolo Insogna <paolo@cowtech.it> * fixup Signed-off-by: Paolo Insogna <paolo@cowtech.it> * test: Fix consumer lag races and stop fail-fast hiding CI results. The lag tests read committed offsets before autocommit had covered the partitions they assert on, so getLag returned -1n instead of the expected lag. Wait for the relevant partitions to have a committed offset first. Also disable matrix fail-fast: a single flaky job was cancelling the other 23, which made one failing test look like a fully broken matrix. Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * test: Fix producer and gh-300 CI flakes. Both tests used retry budgets too tight to survive CI load. send-when-another-destination-fails assumed exactly two Produce requests and that the non-mocked one always succeeds. The producer can issue a third (repeatOnStaleMetadata defaults to true), and under load the non-mocked destination hits a genuine retriable broker error. With retries: 0 both destinations failed and produced.offsets came back empty. The mocked error is canRetry: false, so that destination still fails permanently and the assertion keeps its original meaning. gh-300 failed its initial refresh with "listOffsets failed 2 times" when a single ListOffsets was slow. The wider budget does not affect the pause/resume window the test exercises. Verified on a 24-job matrix: the producer test failed 6 jobs and gh-300 failed 4 before this change, and neither failed any job after it. Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * fix: Address review findings on the API compatibility work. Consumer group states are now reported as Kafka writes them on the wire. listGroups and describeGroups upper cased the raw value, which turned PreparingRebalance into PREPARINGREBALANCE: a value present in neither ConsumerGroupStates nor KafkaConsumerGroupStates, yet cast to ConsumerGroupStateValue. The Pascal case constants this branch added are now the values actually returned. listGroups also translates the legacy Java enum constant names before sending them. Brokers match the states filter case insensitively but not underscore insensitively, so PREPARING_REBALANCE passed client side validation and then silently matched nothing on the broker. Other fixes: - allowedConfigSources is deduplicated, since DYNAMIC_TOPIC_CONFIG and its TOPIC_CONFIG alias share the value 1 and a JSON Schema enum requires unique items. - appendUUID rejects values which do not serialize to 16 bytes. UUIDs carry no length prefix and Buffer.from() stops at the first non hex character, so a topic name used as a topic id appended nothing at all and desynced the rest of the request. - appendVarIntBytes skips empty buffers, like appendString and appendBytes already do. Appending one corrupts DynamicBuffer's positional reads, and record keys and values are allowed to be empty without being null. Reported as platformatic/dynamic-buffer#12. - The Fetch v12 name to id map is built once per request instead of once per request and once per response, keeping the allocation out of the fetch loop's callback. - Produce v3 to v6 no longer set writer.context.requestTimeout: nothing reads it, the connection uses its own requestTimeout option, and v7 to v11 never set it. - The two admin fan outs aggregate throttleTimeMs the same way, and by reducing rather than spreading a caller sized array. - The ApiVersions v1 pinning documents what it costs, tracked in #343. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * test: Exercise the legacy API version codecs against real brokers. The protocol tests parse bytes the same test authored with Writer, so a misread schema is wrong in both directions and still passes. Nothing in this branch had been validated by a broker, and the CI matrix could not do it either: Base[kGetApi] always walks down from maxVersion, so against every supported broker the newest codec wins and the 148 legacy ones are never serialized, sent or parsed. pinApiVersions rewrites the negotiated range on a real client, which needs no production change, and forEachVersion sweeps an API across every version the broker still accepts. Versions below a broker's floor are reported as diagnostics rather than quietly skipped. No old broker images are needed: Confluent 7.5.0 still advertises a minimum of v0 for nearly every API, so 141 of the 148 new codecs are reachable on brokers already in the matrix. This found two real bugs: - DeleteTopics v4 and v5 sent a tagged field section after each topic name, which made the broker drop the connection. topic_names is an array of plain COMPACT_STRINGs at those versions and only becomes an array of structs in v6. The protocol tests asserted the same wrong bytes, so they are corrected too. A sweep of every other flexible codec found no second instance. - listGroups reported an empty state below ListGroups v4, where group_state does not exist on the wire. It now reports 'Unknown', which is the value Kafka defines for the case and was already in ConsumerGroupStates. Run them with `npm run test:compat`. The `.compat-test.ts` suffix keeps them out of `npm test`, following the convention the memory tests already use. The CI job which runs them is not in this commit: pushing a workflow change needs a token scope this machine does not have, so it has to be added separately. Delegation tokens need a broker secret key to be reachable at all, so broker-sasl now sets one. The codecs which remain unreachable, AlterPartition v0-v3 and delegation token v0, are documented in the API status page rather than left to look covered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * ci: Run the API version compatibility sweeps. The main matrix cannot reach the legacy codecs: the client always negotiates the newest version a broker advertises, so only pinning exposes them. This job runs the sweeps added in 287cfd9. Both broker versions are needed and neither is redundant. Kafka 4.0 raised the minimum accepted version of several APIs (KIP-896), so 7.5.0 reaches the oldest codecs (Fetch v0-v3, CreateTopics v0-v1, OffsetCommit v0-v1) while 8.2.0 catches anything which assumes an old broker. One Node.js version is enough because these exercise wire formats rather than runtime behaviour, so this adds two jobs rather than twenty four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * fix: Stop the delegation token config from breaking Confluent 7.5.0. 287cfd9 set KAFKA_DELEGATION_TOKEN_SECRET_KEY on broker-sasl so the delegation token codecs could be exercised. That broke all three Confluent 7.5.0 jobs: KRaft only gained delegation token support in Apache Kafka 3.6 (KIP-900), so a 3.5 broker configured with a secret key refuses to start outright. java.lang.UnsupportedOperationException: Delegation tokens are not supported The broker exited before any test ran, which is why the failure was Node.js version independent and confined to 7.5.0. The setting moves to docker-compose.delegation-tokens.yml, applied explicitly and only where the feature exists. The sweeps skip themselves with a diagnostic on brokers which report the APIs as UNSUPPORTED, which is what 7.5.0 does, so they pass either way. Verified both ways: broker-sasl starts clean on 7.5.0 with the base compose, and the sweeps stay at 272 of 272 on 8.2.0 with the override applied. The compatibility job still needs to pass the override file for 8.2.0. That is a workflow change, which needs a token scope this machine does not have, so it follows separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * test: Skip versions a broker advertises but then rejects. The compatibility job failed on Confluent 7.5.0. The broker reports a minimum of v0 for OffsetCommit and OffsetFetch, so the sweeps exercised those versions, and the broker answered every one of them with UNSUPPORTED_VERSION: ✖ OffsetCommit v0 + OffsetFetch v0 Error: The version of API is not supported. Those versions stored consumer offsets in ZooKeeper, which KRaft does not have, so a KRaft broker refuses them while still advertising them. That is a property of the broker rather than a defect in the codec, and the same mismatch can appear for any API on any broker, so it is now detected generically: runAtVersion turns a protocol level UNSUPPORTED_VERSION into a reported skip, and leaves every other error to fail as before. Verified against both ends of the matrix: 252 of 252 on Confluent 7.5.0 and 272 of 272 on 8.2.0. The 7.5.0 run reports exactly what it could not reach, which is the point of the harness: ℹ Produce: skipping v10, v11 — not accepted by this broker ℹ OffsetCommit v0 + OffsetFetch v0: the broker advertises this version but rejects it, skipping ℹ This broker does not support the delegation token APIs at all, skipping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * test: Run the compatibility sweeps against Apache Kafka 1.1.0. 1.1.0 is the oldest broker this package claims to support, and until now nothing verified that claim. It is the only broker where the legacy codecs are what the client actually negotiates rather than what a test pins, and the only one where ApiVersions maxes out at v1, which is the reason base.ts pins v1 in the first place. docker-compose.legacy.yml is a standalone stack rather than an override of docker-compose.yml: pre-KRaft brokers need ZooKeeper and must not receive the KAFKA_PROCESS_ROLES, CLUSTER_ID and KAFKA_CONTROLLER_* settings, and a compose override can add environment keys but never remove them. Each logical cluster gets its own ZooKeeper chroot, without which all five brokers join a single Kafka cluster and a client bootstrapping on one receives metadata for listeners it cannot speak. The sweeps pass 99 of 99 there. Three things had to change to get that far: - The compatibility helper creates topics with an explicit partition count and replication factor. Omitting them makes Admin send -1, meaning "use the broker default", which is KIP-464 and only understood from Apache Kafka 2.4. Older brokers answer INVALID_PARTITIONS or INVALID_REPLICATION_FACTOR. - OffsetCommit and OffsetFetch v0 read and write offsets in ZooKeeper while v1 and above use the group coordinator, so mixed pairs cannot round-trip on a broker old enough to still accept v0. Those combinations are no longer generated. - SASL and the delegation tokens which depend on it are skipped through COMPAT_LEGACY_BROKER, because Connection pins SaslAuthenticate to v2 and that version only exists from Apache Kafka 2.4. This is a real limitation rather than a missing broker feature, reported as #350, and the documented SASL floor is corrected from 1.0 to 2.4 to match reality. The flag is an explicit opt out rather than a catch of connection failures, so a genuine SASL regression on a modern broker still fails. Verified on both stacks: 99 of 99 on Apache Kafka 1.1.0 and 272 of 272 on Confluent 8.2.0, which is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * fix: Negotiate the SaslAuthenticate version instead of pinning v2. Connection used saslAuthenticateV2 unconditionally. v1 arrived in Apache Kafka 2.0 and v2 in 2.4, so on anything older the broker could not parse the request and dropped the connection, which surfaced as a bare connection failure: InvalidRequestException: Error getting request for apiKey: SASL_AUTHENTICATE, apiVersion: 2, listenerName: ListenerName(SASL) SASL therefore never worked below Kafka 2.4, even though everything else in the client works on 1.1.0 and the documentation claimed 1.0. The connection now asks the broker what it supports and picks the newest version it implements within that range. Brokers accept ApiVersions before authentication precisely so clients can ask, and the probe has to run before SaslHandshake because once the handshake is sent the connection only accepts SaslAuthenticate. The result is cached per connection, since reauthentication takes the same path. When a broker does not answer the probe the previous behaviour is kept, so a connection which worked before still works. SaslHandshake stays pinned to v1 on purpose: v0 selects the pre-1.0 flow, where SASL tokens are written raw on the wire rather than wrapped in SaslAuthenticate requests, which this package does not implement. Fixes #350. The compatibility sweeps now run in full against Apache Kafka 1.1.0, 106 of 106, with no opt out, so the COMPAT_LEGACY_BROKER flag added alongside the legacy stack is gone. That broker exercises SaslAuthenticate v0, which is what a regression to a pinned version would break, and it is also the only one which reaches the delegation token v0 codecs: every later broker advertises a minimum of v1, so those four are no longer listed as unreachable. Verified on Confluent 8.2.0 as well: 2530 of 2531 in the main suite and 272 of 272 in the sweeps, both unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * perf: load test every implemented protocol version, fix readUUID (#352) * perf: load test every implemented protocol version, fix readUUID The compatibility branch made Base[kGetApi] able to negotiate down to Produce v3 and Fetch v4, but every broker in CI negotiates to the newest version, so the legacy codecs were covered for correctness and never for speed. This adds the harness to measure them and records the results. The experiment pins the client codec against a fixed modern broker, so the protocol version is the only variable. Running against the 1.1.0 stack instead would vary the JVM, the storage engine and the codec at once, so that runs as a labelled sanity check rather than a measurement. Verdict: the legacy codecs are not slower. Across 9 Produce versions, 14 Fetch versions, three payload shapes and two acks settings, no legacy version is consistently slower than the newest. Where a reproducible difference exists it runs the other way. Two findings, both in the newest versions: - Reader.readUUID hyphenated with a capture group regex, 801ns against 56ns for the equivalent slicing. Fetch v13+ identifies topics by UUID, so this made the newest Fetch versions 15-18% slower to decode than the ones using topic names. Fixed here; 1653 protocol tests unchanged. - Flexible framing costs ~23% per message at 1MB fetch responses, with the break exactly at the v11/v12 boundary, reproduced over four runs. The mechanism is not isolated and is recorded rather than guessed at. LOAD_TESTING.md also records what the measurement cannot support: tier 1 Fetch at maxBytes=4096 and tier 0 Fetch decode are too noisy on this box for the 15% threshold, and the methodology errors found along the way - fixed record timestamps triggering broker retention mid-sweep, and a constant shuffle seed confounding version with position - both of which first presented as findings about the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: add a README for the protocol version benchmarks Covers how to run the suite, what each script measures, and the configuration. The larger half is how to get numbers that mean something: vary the shuffle seed before believing any single-version result, pin CPUs, run one sweep at a time, and know the noise floor. Each of those countermeasures exists because its absence produced a plausible finding that turned out to be an artifact, so the README says which artifact and how it presented rather than just stating the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA * docs: remove LOAD_TESTING.md, fold its results into the suite README The plan document has served its purpose now that the suite exists and has run. Its two durable halves move into benchmarks/protocol-versions/README.md: the recorded verdict and the findings, which would otherwise survive only in the PR description, and the tier and guard naming that the source comments refer to. Nine references across seven files pointed at the deleted path; all now point at the README. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Signed-off-by: Paolo Insogna <paolo@cowtech.it> Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Targets
compatibility(#338), because it exists to validate that branch's codecs.Why
#338 taught
Base[kGetApi]to negotiate down toProduce v3andFetch v4. Every broker in CI negotiates to the newest version, so those legacy codecs are covered for correctness bytest/integration/*.compat-test.tsand never for speed. If one were accidentally quadratic, allocated per record, or defeated a fast path inDynamicBuffer, nothing in the repo would notice.Design
The experiment pins the client codec against a fixed modern broker, so the protocol version is the only variable.
pinApiVersions()already existed intest/helpers/api-versions.ts; a Kafka 4.x broker still accepts every version this package implements (KIP-896 set the floor at exactly our lowest codecs), so the whole matrix runs against one broker.Running against the 1.1.0 stack instead would vary the JVM, the storage engine, the on-disk format and the codec at once — that can't attribute a difference to our code. It runs as an explicitly labelled sanity check (tier 2), not a measurement.
Three tiers: codecs with no broker, pinned versions against a live broker, and the legacy-broker sanity run.
scripts/run-protocol-load-test.shruns them in the right order.Verdict
The legacy codecs are not slower. Across 9 Produce versions, 14 Fetch versions, three payload shapes and two acks settings, no legacy version is consistently slower than the newest. Where a reproducible difference exists it runs the other way.
Against real Apache Kafka 1.1.0 the client negotiates Produce v5 / Fetch v7 unaided and matches the modern broker to within 1% (1.40 vs 1.39 µs/msg).
Findings — both in the newest versions
1.
Reader.readUUIDwas 14x slower than necessary — fixed here. It hyphenated with a capture-group regex on every call: 801 ns against 56 ns for the equivalent slicing. Fetch v13+ identifies topics by UUID, so this made the newest Fetch versions 15–18% slower to decode than the name-based ones. This is the onlysrc/change in the PR. 1653 protocol and codec tests pass unchanged.2. Flexible framing costs ~23% per message at 1 MB fetch responses. The break is exactly at the v11→v12 boundary, where compact collections and tagged fields begin. Four consecutive runs, tight clusters either side. Not fixed and the mechanism is not isolated — it is recorded rather than guessed at. It matters because v17 is what the client negotiates by default against a modern broker.
Also noted, version-independent so invisible to this comparison: the shared record encoder costs ~48% more per record at 10,000 records than at 100 (1898 → 5418 ns/record out to 400k). Roughly 1.3x per 10x, so allocation and cache pressure —
createRecordallocates aWriterper record — not anything quadratic. It is the largest single lever on producer throughput found here.What the measurement cannot support
Recorded in
LOAD_TESTING.mdrather than glossed over. Tier 1 Fetch atmaxBytes=4096and tier 0 Fetch decode both have a ~20% noise floor on the machine this ran on, well above the 15% threshold the criteria use. The verdict rests on tier 1 at 64 KB and 1 MB and on Produce, where clusters are tight across four runs and three shuffle seeds. Making tier 0 Fetch decode reliable would mean running each version in its own process.Methodology corrections
Three things that first presented as findings about the client and were not. They are documented because anyone re-running this will hit them:
1700000000000(Nov 2023) for byte reproducibility. Kafka applies retention by the largest record timestamp in a segment, so the broker deleted the seeded log mid-sweep and consumers correctly read an empty partition — which looked exactly like an intermittent consumer stall. Base is nowDate.now()captured once; byte reproducibility survives because a batch stores its base as a fixed-width INT64 and each record as a varint delta.Fetch v5measured 5.79 / 6.01 / 5.97 atmaxBytes=4096across three runs — entirely because it was always first. The seed is nowPROTOCOL_BENCH_SEED, and varying it is documented as mandatory before believing any single-version result.Validity guards
guards.ts. Without this the sweeps could be measuring the broker's down-conversion cost under the client's label.Not included
No CI job. Tier 0 needs no Docker and could reasonably run in CI later, but the live tiers need pinned CPUs to mean anything and would be noise in a shared runner.
Test plan
npm run lintandnpm run typecheckcleantest/apis/*/*.test.tsandtest/protocol/*.test.ts, covering thereadUUIDchange./scripts/run-protocol-load-test.sh🤖 Generated with Claude Code
https://claude.ai/code/session_01Kj49eYmNCELDfrP2xa1YpA