KAFKA-20443: Move RequestChannel Requests to server module by mimaison · Pull Request #22075 · apache/kafka

mimaison · 2026-04-16T11:43:20Z

Rewrite in Java and move to server module

Reviewers: Ken Huang s7133700@gmail.com, Chia-Ping Tsai
chia7712@gmail.com

m1a2st

Thanks for this patch, one nit comment

m1a2st · 2026-04-18T15:14:34Z

        buffer,
        requestChannelMetrics,
-        Some(envelope)
+        Optional.ofNullable(envelope)


I think Optional.of() is preferable to Optional.ofNullable(), since envelope should never be null.

You're right, not need for ofNullable() here. Updated, thanks

m1a2st

Thanks, LGTM

mimaison · 2026-05-07T18:36:29Z

@showuon can you take a look?

chia7712

@mimaison thanks for this rewrite!

chia7712 · 2026-05-10T19:17:32Z

+
+    private volatile OptionalLong callbackRequestDequeueTimeNanos = OptionalLong.empty();
+    private volatile OptionalLong callbackRequestCompleteTimeNanos = OptionalLong.empty();
+    private volatile Optional<Consumer<Long>> recordNetworkThreadTimeCallback = Optional.empty();


private volatile LongConsumer recordNetworkThreadTimeCallback = __ -> {};

chia7712 · 2026-05-10T19:19:27Z

+import java.util.Objects;
+import java.util.function.Consumer;
+
+public final class CallbackRequest implements BaseRequest {


That looks like a good candidate for a record class. WDYT?

chia7712 · 2026-05-10T19:21:39Z

+     * to the nearest long.
+     */
+    private static double nanosToMs(long nanos) {
+        long positiveNanos = Math.max(nanos, 0);


private static double nanosToMs(long nanos) { return (double) TimeUnit.NANOSECONDS.toMicros(nanos) / 1000.0; }

chia7712 · 2026-05-10T19:23:10Z

+ */
+package org.apache.kafka.network;
+
+public final class ShutdownRequest implements BaseRequest {


public enum ShutdownRequest implements BaseRequest { INSTANCE; }

chia7712 · 2026-05-10T19:23:18Z

+ */
+package org.apache.kafka.network;
+
+public final class WakeupRequest implements BaseRequest {


mimaison · 2026-05-11T12:37:09Z

Thanks @chia7712 for the review. All your suggestions made sense, so I applied them all.

chia7712

@mimaison thanks for the updates. overall LGTM

chia7712 · 2026-05-12T03:51:23Z

+     * to the nearest long.
+     */
+    private static double nanosToMs(long nanos) {
+        return (double) TimeUnit.NANOSECONDS.toMicros(nanos) / 1000;


ugh, my previous comment missed the protection Math.max(0L, nanos). Would you please add it back?

It's not entirely clear if nanos can ever be negative here but I think it's probably best to be safe and keep this protection. I pushed an update

chia7712 · 2026-05-12T03:54:37Z

+        double messageConversionsTimeMs = nanosToMs(messageConversionsTimeNanos);
+        double totalTimeMs = nanosToMs(endTimeNanos - startTimeNanos);
+
+        List<String> overrideMetricNames;


How about using switch expression ?

List<String> overrideMetricNames = switch (header().apiKey()) { case FETCH -> { String specifiedMetricName = body(FetchRequest.class).isFromFollower() ? RequestMetrics.FOLLOW_FETCH_METRIC_NAME : RequestMetrics.CONSUMER_FETCH_METRIC_NAME; yield List.of(specifiedMetricName, ApiKeys.FETCH.name); } case ADD_PARTITIONS_TO_TXN -> body(AddPartitionsToTxnRequest.class).allVerifyOnlyRequest() ? List.of(RequestMetrics.VERIFY_PARTITIONS_IN_TXN_METRIC_NAME) : List.of(ApiKeys.ADD_PARTITIONS_TO_TXN.name); case LIST_CONFIG_RESOURCES -> (header().apiVersion() == 0) ? List.of(RequestMetrics.LIST_CLIENT_METRICS_RESOURCES_METRIC_NAME, ApiKeys.LIST_CONFIG_RESOURCES.name) : List.of(ApiKeys.LIST_CONFIG_RESOURCES.name); default -> List.of(header().apiKey().name); };

Good idea that's slightly more elegant

chia7712 · 2026-05-13T02:55:10Z

@mimaison would you mind fixing the conflicts?

Rewrite in Java and move to server module

mimaison · 2026-05-13T12:42:39Z

Rebased on trunk

chia7712

LGTM

chia7712 · 2026-05-15T05:17:11Z

    expectedNode.set("isForwarded", if (req.isForwarded) BooleanNode.TRUE else BooleanNode.FALSE)
    expectedNode.set("requestHeader", RequestConvertToJson.requestHeaderNode(req.header))
-    expectedNode.set("request", req.requestLog.getOrElse(new TextNode("")))
+    expectedNode.set("request", req.requestLog.orElse(new TextNode("")))


This comment is orthogonal to the PR, but it appears NullNode.getInstance() is more suitable than new TextNode("") if the filed is non-existent

I've opened a PR(#22295) to address this.

…ponse fields (#22295) See #22075 (comment) , use `NullNode.getInstance()` instead of `new TextNode("")` as the fallback for missing `request`/`response` fields. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

Rewrite in Java and move to server module Reviewers: Ken Huang <s7133700@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>

…ponse fields (apache#22295) See apache#22075 (comment) , use `NullNode.getInstance()` instead of `new TextNode("")` as the fallback for missing `request`/`response` fields. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

chia7712 · 2026-07-09T18:41:42Z

+        };
+
+        for (String metricName : overrideMetricNames) {
+            RequestMetrics m = metrics.apply(metricName);


The apply naming wasn't migrated to follow Java conventions

I'll label this as a 'good first issue' :)

mimaison marked this pull request as ready for review April 16, 2026 11:43

github-actions Bot added core Kafka Broker performance labels Apr 16, 2026

mimaison force-pushed the kafka-20443 branch from 8356dbd to c251222 Compare April 16, 2026 11:48

m1a2st reviewed Apr 18, 2026

View reviewed changes

mimaison force-pushed the kafka-20443 branch 2 times, most recently from 72ed87e to 65e8328 Compare April 21, 2026 16:05

m1a2st approved these changes Apr 25, 2026

View reviewed changes

mimaison force-pushed the kafka-20443 branch 2 times, most recently from 0a753b4 to d9cac70 Compare May 7, 2026 09:12

chia7712 reviewed May 10, 2026

View reviewed changes

chia7712 reviewed May 12, 2026

View reviewed changes

mimaison added 5 commits May 13, 2026 14:41

KAFKA-20443: Move RequestChannel Requests to server module

6d7f79a

Rewrite in Java and move to server module

Address feedback from Ken

6b5c201

Address feedback from Chia-Ping

e300bc3

Re-add Math.max

5eddced

Use switch block

6e6c113

mimaison force-pushed the kafka-20443 branch from fd8e7e6 to 6e6c113 Compare May 13, 2026 12:41

chia7712 approved these changes May 15, 2026

View reviewed changes

chia7712 merged commit dc3fcbb into apache:trunk May 15, 2026
23 checks passed

majialoong mentioned this pull request May 15, 2026

MINOR: Use NullNode instead of empty TextNode for missing request/response fields #22295

Merged

JiayaoS pushed a commit to JiayaoS/kafka that referenced this pull request May 21, 2026

KAFKA-20443 Move RequestChannel Requests to server module (apache#22075)

23552eb

Rewrite in Java and move to server module Reviewers: Ken Huang <s7133700@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>

chia7712 reviewed Jul 9, 2026

View reviewed changes

dino2895 mentioned this pull request Jul 9, 2026

MINOR: Rename RequestChannelMetrics.apply to get #22798

Open

mimaison deleted the kafka-20443 branch July 9, 2026 23:14

Uh oh!

Conversation

mimaison commented Apr 16, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

mimaison commented May 7, 2026

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mimaison commented May 11, 2026

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chia7712 commented May 13, 2026

Uh oh!

mimaison commented May 13, 2026

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mimaison commented Apr 16, 2026 •

edited by github-actions Bot

Loading