KAFKA-12677: return not_controller error in envelope response itself #10794

showuon · 2021-05-30T11:53:11Z

In Kafka Raft mode, the flow sending request from client to controller is like this:

client send reqesut to a random controller (ex: A-controller)
A-controller will forward the request to active controller (ex: B-controller) to handle the request
After active B-controller completed the request, the A-controller will receive the response, and do a check:
3.1. if the response has "disconnected" or "NOT_CONTROLLER" error, which means the cached active controller is changed. So, clear the cached active controller, and wait for next retry to get the updated active controller from controllerNodeProvider
3.2. else, complete the request and respond back to client

In this bug, we have 2 issues existed:

"NOT_CONTROLLER" exception won't be correctly send back to the requester, instead, UNKNOWN_SERVER_ERROR will be returned. The reason is the NotControllerException is wrapped by a CompletionException when the Future completeExceptionally. And the CompletionException will not match any Errors we defined, so the UNKNOWN_SERVER_ERROR will be returned. Even if we don't want the NotControllerException return back to client, we need to know it to do some check.

fix 1: unwrap the CompletionException before encoding the exception to error.

Even if we fixed 1st bug, we still haven't fixed this issue. After the 1st bug fixed, the client can successfully get NotControllerException now, and keep retrying... until timeout. So, why won't it meet the flow 3.1 mentioned above, since it has NotControllerException? The reason is, we wrapped the original request with EnvelopeRequest and forwarded to active controller. So, after the active controller completed the request, responded with NotControllerException, and then, wrapped into an EnvelopeResponse with no error, and then send the EnvelopeResponse back. That is, in the flow 3.1, we only got "no error" from EnvelopeResponse, not the NotControllerException inside.

new: fix 2: Make the envelope response return NotControllerException if the controller response has NotControllerException. So that we can catch the NotControllerException on envelopeResponse to update the active controller.

old: fix 2: in flow 3.1, parse the EnvelopeResponse to check if there's NotControllerException inside.

Note: in the jira ticket we think there's Recorded new controller log, which should already changed to new active controller:

[broker0_:BrokerToControllerChannelManager broker=0 name=heartbeat]: Recorded new controller, from now on will use broker localhost:54229 (id: 3000 rack: null)

It's is true, but after further investigation, I found it only changed for this thread: broker0_:BrokerToControllerChannelManager broker=0 name=heartbeat. The topic creation thread is forwarding thread: broker0_:BrokerToControllerChannelManager broker=0 name=forwarding, which still using old active controller.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

… existed

dengziming · 2021-05-30T12:37:36Z

This is awesome, I also notice that there is no logic about NOT_CONTROLLER and active controller, I will take some time to test this PR. 👍

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala

showuon · 2021-05-31T04:05:12Z

@hachikuji @mumrah @abbccdda @cmccabe , could you help review this PR? I'll add tests later. Thank you.

showuon · 2021-05-31T12:08:19Z

Tests added. Thanks.

core/src/main/scala/kafka/server/AutoTopicCreationManager.scala

core/src/main/scala/kafka/server/ForwardingManager.scala

dengziming

It seems the problem has been fixed, you can make some small changes to trigger the QA some more times to prove this approach is right.

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala

showuon · 2021-06-01T06:03:41Z

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala

   */
  def sendRequest(
    request: AbstractRequest.Builder[_ <: AbstractRequest],
-    callback: ControllerRequestCompletionHandler
+    callback: ControllerRequestCompletionHandler,
+    requestHeader: RequestHeader


add a requestHeader parameter to do envelope response parsing.

showuon · 2021-06-01T06:06:22Z

Jenkins PR build results proved the RaftClusterTest tests doesn't fail anymore:
#4: 1 failed test:

Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.integration.BlockingConnectorTest.testBlockInSinkTaskStop

#5: all tests passed
#6: all tests passed

showuon · 2021-06-01T06:08:21Z

@dengziming , thanks for the comments. I've updated. Thanks.

showuon · 2021-06-03T10:21:10Z

@hachikuji @mumrah @abbccdda @cmccabe , call for review since the tests keep failing. Thank you.

build.gradle

wenbingshen

@showuon Great finding, thanks for your PR, a trivial suggestion below.

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala

showuon · 2021-06-18T06:49:22Z

@hachikuji @mumrah @abbccdda @cmccabe , please help review when available. Thank you.

hachikuji · 2021-06-18T20:52:37Z

@showuon Thanks for the patch. One detail in the description does not seem right to me.

"NOT_CONTROLLER" exception won't be correctly send back, instead, UNKNOWN_SERVER_ERROR will be returned. The reason is the NotControllerException is wrapped by a CompletionException when the Future completeExceptionally. And the CompletionException will not match any Errors we defined, so the UNKNOWN_SERVER_ERROR will be returned.

The client is not necessarily trying to reach the controller. It is sending a request to a broker and the fact that we forward it to a controller is an implementation detail. So the NOT_CONTROLLER error does not make sense for the client. That is why we do not return it. We decided instead that if the controller could not be reached before expiration of the timeout, then we would return REQUEST_TIMED_OUT instead.

That aside, it is not very clear to me why we need to parse the envelope response. The intent was to return NOT_CONTROLLER as the error code in the EnvelopeResponse itself. Perhaps there is a race condition in the controller handling where this is not working? Probably that is what we should fix.

showuon · 2021-06-19T02:33:58Z

@hachikuji , thanks for your explanation.

The client is not necessarily trying to reach the controller. It is sending a request to a broker and the fact that we forward it to a controller is an implementation detail. So the NOT_CONTROLLER error does not make sense for the client. That is why we do not return it.

The intent was to return NOT_CONTROLLER as the error code in the EnvelopeResponse itself. Perhaps there is a race condition in the controller handling where this is not working? Probably that is what we should fix.

That makes sense! I'll look into it and let you know. Thank you.

showuon · 2021-06-19T08:19:54Z

@hachikuji , I checked and there's no race condition there. The reason why we didn't put NOT_CONTROLLER in EnvelopResponse itself is because we will build envelopeResponse with no error as long as we got response from handler (createTopics handler in this case) here. So, in ControllerApi#handleCreateTopics, we'll sendResponse when future completed, and then RequestHandlerHelper#sendResponseMaybeThrottle, we'll buildResponseSend and then go to the above link location.

So, to fix this issue, I need to build EnvelopeResponse with NotControllerError when the response from handler is having NotControllerError (fix 2). But if we don't have (fix 1), we can only get Unknown_Server_Error in the response from handler because the NotControllerError is wrapped with CompletionException.

That's my solution. Please help review. Thank you very much!

showuon · 2021-06-20T02:37:14Z

clients/src/main/java/org/apache/kafka/common/requests/ApiError.java

+        Throwable throwableToBeEncode = t;
+        // Future will wrap the original exception with completionException, which will return unexpected UNKNOWN_SERVER_ERROR.
+        if (t instanceof CompletionException) {
+            throwableToBeEncode = t.getCause();


fix 1: unwrap the CompletionException to get the original exception inside. Even if we don't want the NotControllerException return back to client, we need to know it to do some check.

showuon · 2021-06-24T14:02:30Z

@hachikuji , call for review. Thanks.

hachikuji

@showuon Thanks for the updates. Makes sense overall, but left a couple comments.

hachikuji · 2021-07-07T20:49:44Z

core/src/main/scala/kafka/network/RequestChannel.scala

+            // Since it's Not Controller error response, we need to make envelope response with Not Controller error
+            // to notify the requester (i.e. BrokerToControllerRequestThread) to update active controller
+            new EnvelopeResponse(new EnvelopeResponseData()
+              .setErrorCode(Errors.NOT_CONTROLLER.code()))


Ok, I think this makes sense. Basically we were missing the same logic we have in KafkaApis.handleEnvelope to verify the controller status. Instead, we just send the request into the controller thread. Do I have that right? I guess another way we could do it is to send the full envelope into the controller and let it set the NOT_CONTROLLER error at the right level, but that might take some refactoring and this solution seems reasonable as well.

hachikuji · 2021-07-07T20:52:56Z

clients/src/main/java/org/apache/kafka/common/requests/ApiError.java

@@ -36,10 +37,16 @@
    private final String message;

    public static ApiError fromThrowable(Throwable t) {
+        Throwable throwableToBeEncode = t;
+        // Future will wrap the original exception with completionException, which will return unexpected UNKNOWN_SERVER_ERROR.


Hmm, a little strange to handle this case explicitly and not others. Would we want to handle ExecutionException as well for example?

Also, the comment seems to be missing some context. Which future does it refer to? Maybe it would be helpful to point to the specific usage that requires this.

ExecutionException -> good point! Updated. Thanks.

showuon · 2021-07-08T03:54:27Z

@hachikuji , I've updated addressed your comments. Please take a look again. Thank you.

showuon · 2021-07-08T08:44:07Z

Failed tests are unrelated. Thanks.

    Build / ARM / kafka.raft.KafkaMetadataLogTest.testDeleteSnapshots()
    Build / ARM / kafka.raft.KafkaMetadataLogTest.testDeleteSnapshots()
    Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.integration.BlockingConnectorTest.testBlockInConnectorInitialize
    Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.integration.ExampleConnectIntegrationTest.testSinkConnector
    Build / JDK 8 and Scala 2.12 / kafka.raft.KafkaMetadataLogTest.testDeleteSnapshots()
    Build / JDK 8 and Scala 2.12 / kafka.raft.KafkaMetadataLogTest.testDeleteSnapshots()
    Build / JDK 11 and Scala 2.13 / kafka.api.ConsumerBounceTest.testCloseDuringRebalance()
    Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.EOSUncleanShutdownIntegrationTest.shouldWorkWithUncleanShutdownWipeOutStateStore[exactly_once_v2]
    Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.EosV2UpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosV2[true]
    Build / JDK 16 and Scala 2.13 / org.apache.kafka.connect.integration.ConnectWorkerIntegrationTest.testBrokerCoordinator
    Build / JDK 16 and Scala 2.13 / org.apache.kafka.connect.integration.ConnectorClientPolicyIntegrationTest.testCreateWithAllowedOverridesForPrincipalPolicy
    Build / JDK 16 and Scala 2.13 / kafka.api.ConsumerBounceTest.testCloseDuringRebalance()
    Build / JDK 16 and Scala 2.13 / kafka.api.ConsumerBounceTest.testCloseDuringRebalance()
    Build / JDK 16 and Scala 2.13 / kafka.api.TransactionsTest.testAbortTransactionTimeout()
    Build / JDK 16 and Scala 2.13 / kafka.api.TransactionsTest.testAbortTransactionTimeout()

hachikuji

Thanks for the updates. Left a couple minor comments.

hachikuji · 2021-07-08T16:53:03Z

clients/src/main/java/org/apache/kafka/common/requests/ApiError.java

@@ -36,10 +38,21 @@
    private final String message;

    public static ApiError fromThrowable(Throwable t) {
+        Throwable throwableToBeEncode = t;


nit: throwableToBeEncoded?

hachikuji · 2021-07-08T17:02:24Z

core/src/main/scala/kafka/network/RequestChannel.scala

-          val responseBytes = context.buildResponseEnvelopePayload(abstractResponse)
-          val envelopeResponse = new EnvelopeResponse(responseBytes, Errors.NONE)
+          val envelopeResponse = if (abstractResponse.errorCounts().containsKey(Errors.NOT_CONTROLLER)) {
+            // Since it's a Not Controller error response, we need to make envelope response with Not Controller error


Do we have a test case which fails without this fix?

nit: we're a little inconsistent in the comments when referring to the NOT_CONTROLLER error. Here we use "Not Controller" while in the tests we use "Not_Controller." I would suggest using "NOT_CONTROLLER" consistently.

showuon · 2021-07-09T07:56:40Z

@hachikuji , thanks for the reminder. Yes, I forgot to add tests for that area. Added. Thank you.

cmccabe · 2021-07-09T16:28:27Z

core/src/test/scala/integration/kafka/server/RaftClusterTest.scala

@@ -29,7 +29,7 @@ import java.util
 import java.util.Collections
 import scala.jdk.CollectionConverters._

-@Timeout(120000)
+@Timeout(120)


Hmm, I don't think 0.12 seconds is a good timeout for these integration tests. I would be OK cutting the time down to 60 seconds per test, though, rather than 120.

I think the default unit is seconds, so this is 120s.

hachikuji

@showuon Thanks for the updates. Just a few more small comments.

hachikuji · 2021-07-09T22:47:47Z

core/src/test/scala/unit/kafka/network/RequestChannelTest.scala

+    Mockito.doReturn(mockSend).when(channelRequest.envelope.get.context).buildResponseSend(envelopResponseArgumentCaptor.capture())
+
+    // create an inner response without error
+    val ResponseWithoutError = RequestTestUtils.metadataUpdateWith(2, Collections.singletonMap("a", 2))


nit: use lower case ResponseWithoutError

hachikuji · 2021-07-09T22:48:28Z

core/src/test/scala/unit/kafka/network/RequestChannelTest.scala

+  }
+
+  @Test
+  def testEnvelopeBuildResponseSendShouldReturnNotControllerErrorIfInnerResponseHasOne(): Unit = {


Another useful test case is when the inner response has an error which is different from NOT_CONTROLLER. In this case, we expect no envelope error.

Good suggestion! Added testEnvelopeBuildResponseSendShouldReturnNoErrorIfInnerResponseHasNoNotControllerError

hachikuji · 2021-07-09T22:54:34Z

core/src/test/scala/unit/kafka/network/RequestChannelTest.scala

@@ -179,6 +184,108 @@ class RequestChannelTest {
    assertTrue(isValidJson(RequestConvertToJson.request(alterConfigs.loggableRequest).toString))
  }

+  @Test
+  def testEnvelopeBuildResponseSendShouldReturnNoErrorIfInnerResponseHasNoError(): Unit = {
+    val channelRequest = buildRequestWithEnvelope()


nit: it's a bit confusing that buildRequestWithEnvelope creates an alter config request, which we then respond to with a metadata response. Could we let buildRequestWithEnvelope provide the AbstractRequest so that we can use a MetadataRequest? For example, see KafkaApis.buildRequestWithEnvelope. Probably we could factor out a common method.

Put the buildRequestWithEnvelope method in TestUtils class to share between KafkaApisTest and RequestChannelTest. And also use MetadataRequest as input. Thank you.

hachikuji · 2021-07-09T22:55:03Z

core/src/test/scala/unit/kafka/network/RequestChannelTest.scala

+    val requestContextSpy: RequestContext = Mockito.spy(envelopeContext)
+
+    RequestHeader.parse(envelopeBuffer)
+    val EnvelopeBuffer2 = envelopeBuffer.duplicate()


nit: lower case envelopeBuffer2

hachikuji · 2021-07-09T23:01:40Z

clients/src/main/java/org/apache/kafka/common/requests/ApiError.java

@@ -36,10 +38,21 @@
    private final String message;

    public static ApiError fromThrowable(Throwable t) {
+        Throwable throwableToBeEncoded = t;
+        // Get the original cause from `CompletionException` and `ExecutionException`, otherwise, it will return unexpected UNKNOWN_SERVER_ERROR.


nit: I think we can make this a little more concise. How about this?

// Get the underlying cause for common exception types from the concurrent library. // This is useful to handle cases where exceptions may be raised from a future or a // completion stage (as might be the case for requests sent to the controller in // `ControllerApis`)

Thanks for the suggestion! Updated.

hachikuji · 2021-07-09T23:02:49Z

core/src/main/scala/kafka/network/RequestChannel.scala

-          val responseBytes = context.buildResponseEnvelopePayload(abstractResponse)
-          val envelopeResponse = new EnvelopeResponse(responseBytes, Errors.NONE)
+          val envelopeResponse = if (abstractResponse.errorCounts().containsKey(Errors.NOT_CONTROLLER)) {
+            // Since it's a Not Controller error response, we need to make envelope response with Not Controller error


nit: we're a little inconsistent in the comments when referring to the NOT_CONTROLLER error. Here we use "Not Controller" while in the tests we use "Not_Controller." I would suggest using "NOT_CONTROLLER" consistently.

… KAFKA-12677

showuon · 2021-07-10T23:17:42Z

@hachikuji , I've addressed your comments. Please take a look again.

Failed tests are unrelated. Thanks.

    Build / JDK 11 and Scala 2.13 / kafka.api.ConsumerBounceTest.testCloseDuringRebalance()
    Build / JDK 11 and Scala 2.13 / kafka.api.ConsumerBounceTest.testCloseDuringRebalance()
    Build / JDK 11 and Scala 2.13 / kafka.api.TransactionsTest.testSendOffsetsToTransactionTimeout()
    Build / JDK 11 and Scala 2.13 / kafka.api.TransactionsTest.testCommitTransactionTimeout()
    Build / JDK 8 and Scala 2.12 / kafka.api.ConsumerBounceTest.testCloseDuringRebalance()
    Build / JDK 8 and Scala 2.12 / kafka.server.RaftClusterSnapshotTest.testContorllerSnapshotGenerated()

hachikuji

LGTM. Thanks for the patch!

…in KRaft mode (#10794) In Kafka Raft mode, the flow sending request from client to controller is like this: 1. client send request to a random controller (ex: A-controller) 2. A-controller will forward the request to active controller (ex: B-controller) to handle the request 3. After active B-controller completed the request, the A-controller will receive the response, and do a check: 3.1. if the response has "disconnected" or "NOT_CONTROLLER" error, which means the cached active controller is changed. So, clear the cached active controller, and wait for next retry to get the updated active controller from `controllerNodeProvider` 3.2. else, complete the request and respond back to client In this bug, we have 2 issues existed: 1. "NOT_CONTROLLER" exception won't be correctly send back to the requester, instead, `UNKNOWN_SERVER_ERROR` will be returned. The reason is the `NotControllerException` is wrapped by a `CompletionException` when the `Future` completeExceptionally. And the `CompletionException` will not match any Errors we defined, so the `UNKNOWN_SERVER_ERROR` will be returned. Even if we don't want the `NotControllerException` return back to client, we need to know it to do some check. fix 1: unwrap the `CompletionException` before encoding the exception to error. 2. Even if we fixed 1st bug, we still haven't fixed this issue. After the 1st bug fixed, the client can successfully get `NotControllerException` now, and keep retrying... until timeout. So, why won't it meet the flow `3.1` mentioned above, since it has `NotControllerException`? The reason is, we wrapped the original request with `EnvelopeRequest` and forwarded to active controller. So, after the active controller completed the request, responded with `NotControllerException`, and then, wrapped into an `EnvelopeResponse` **with no error**, and then send the `EnvelopeResponse` back. That is, in the flow `3.1`, we only got "no error" from `EnvelopeResponse`, not the `NotControllerException` inside. fix 2: Make the envelope response return `NotControllerException` if the controller response has `NotControllerException`. So that we can catch the `NotControllerException` on envelopeResponse to update the active controller. Reviewers: wenbingshen <oliver.shen999@gmail.com>, Ismael Juma <ismael@juma.me.uk>, dengziming <dengziming1993@gmail.com>, Jason Gustafson <jason@confluent.io>

…in KRaft mode (apache#10794) In Kafka Raft mode, the flow sending request from client to controller is like this: 1. client send request to a random controller (ex: A-controller) 2. A-controller will forward the request to active controller (ex: B-controller) to handle the request 3. After active B-controller completed the request, the A-controller will receive the response, and do a check: 3.1. if the response has "disconnected" or "NOT_CONTROLLER" error, which means the cached active controller is changed. So, clear the cached active controller, and wait for next retry to get the updated active controller from `controllerNodeProvider` 3.2. else, complete the request and respond back to client In this bug, we have 2 issues existed: 1. "NOT_CONTROLLER" exception won't be correctly send back to the requester, instead, `UNKNOWN_SERVER_ERROR` will be returned. The reason is the `NotControllerException` is wrapped by a `CompletionException` when the `Future` completeExceptionally. And the `CompletionException` will not match any Errors we defined, so the `UNKNOWN_SERVER_ERROR` will be returned. Even if we don't want the `NotControllerException` return back to client, we need to know it to do some check. fix 1: unwrap the `CompletionException` before encoding the exception to error. 2. Even if we fixed 1st bug, we still haven't fixed this issue. After the 1st bug fixed, the client can successfully get `NotControllerException` now, and keep retrying... until timeout. So, why won't it meet the flow `3.1` mentioned above, since it has `NotControllerException`? The reason is, we wrapped the original request with `EnvelopeRequest` and forwarded to active controller. So, after the active controller completed the request, responded with `NotControllerException`, and then, wrapped into an `EnvelopeResponse` **with no error**, and then send the `EnvelopeResponse` back. That is, in the flow `3.1`, we only got "no error" from `EnvelopeResponse`, not the `NotControllerException` inside. fix 2: Make the envelope response return `NotControllerException` if the controller response has `NotControllerException`. So that we can catch the `NotControllerException` on envelopeResponse to update the active controller. Reviewers: wenbingshen <oliver.shen999@gmail.com>, Ismael Juma <ismael@juma.me.uk>, dengziming <dengziming1993@gmail.com>, Jason Gustafson <jason@confluent.io>

KAFKA-12677: parse envelope response to check if not_controller error…

c453b5a

… existed

showuon changed the title ~~KAFKA-12677: parse envelope response to check if not_controller error existed~~ [WIP]KAFKA-12677: parse envelope response to check if not_controller error existed May 31, 2021

showuon added 2 commits May 31, 2021 10:29

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-12677

ac27128

KAFKA-12677: fix broken tests

d0bc8df

showuon commented May 31, 2021

View reviewed changes

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala Outdated Show resolved Hide resolved

showuon commented May 31, 2021

View reviewed changes

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala Outdated Show resolved Hide resolved

showuon added 3 commits May 31, 2021 14:28

KAFKA-12677: refactor

564f790

KAFKA-12677: add tests

33ed9c1

KAFKA-12677: add requestHeader in AutoTopicCreationManager

995a2c3

showuon changed the title ~~[WIP]KAFKA-12677: parse envelope response to check if not_controller error existed~~ KAFKA-12677: parse envelope response to check if not_controller error existed May 31, 2021

showuon commented May 31, 2021

View reviewed changes

core/src/main/scala/kafka/server/AutoTopicCreationManager.scala Outdated Show resolved Hide resolved

showuon commented May 31, 2021

View reviewed changes

core/src/main/scala/kafka/server/ForwardingManager.scala Outdated Show resolved Hide resolved

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-12677

15c0052

dengziming reviewed May 31, 2021

View reviewed changes

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala Outdated Show resolved Hide resolved

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala Outdated Show resolved Hide resolved

KAFKA-12677: refactor

97d589d

showuon commented Jun 1, 2021

View reviewed changes

ijuma reviewed Jun 4, 2021

View reviewed changes

build.gradle Outdated Show resolved Hide resolved

wenbingshen reviewed Jun 5, 2021

View reviewed changes

core/src/main/scala/kafka/server/BrokerToControllerChannelManager.scala Outdated Show resolved Hide resolved

KAFKA-12677: address reviewer's comment to refactor code

15c6e78

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-12677

42d64ad

showuon force-pushed the KAFKA-12677 branch from efe044d to b601023 Compare June 19, 2021 07:40

showuon changed the title ~~KAFKA-12677: parse envelope response to check if not_controller error existed~~ KAFKA-12677: return not_controller error in envelope response itself Jun 20, 2021

showuon commented Jun 20, 2021

View reviewed changes

cmccabe added the kraft label Jun 29, 2021

hachikuji reviewed Jul 7, 2021

View reviewed changes

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-12677

8b6784d

showuon force-pushed the KAFKA-12677 branch from 77efc00 to dd26d7e Compare July 8, 2021 04:25

KAFKA-12677: refactor comment and add tests

3d5efdf

showuon force-pushed the KAFKA-12677 branch from dd26d7e to 3d5efdf Compare July 8, 2021 12:26

hachikuji reviewed Jul 8, 2021

View reviewed changes

KAFKA-12677: rename variable

fcf2a3e

showuon force-pushed the KAFKA-12677 branch from 43c27ba to cbec924 Compare July 9, 2021 07:57

KAFKA-12677: add test for RequestChannel fix

88d0266

showuon force-pushed the KAFKA-12677 branch from cbec924 to 88d0266 Compare July 9, 2021 08:00

cmccabe reviewed Jul 9, 2021

View reviewed changes

hachikuji reviewed Jul 9, 2021

View reviewed changes

showuon added 3 commits July 10, 2021 13:18

KAFKA-12677: refactor

7a13edf

Merge branch 'KAFKA-12677_2' of https://github.com/showuon/kafka into…

b3d22bc

… KAFKA-12677

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-12677

c441c18

showuon force-pushed the KAFKA-12677 branch from 17fac18 to c441c18 Compare July 10, 2021 06:27

KAFKA-12677: fix scala 2.12 compilation error

d1f678d

hachikuji approved these changes Jul 12, 2021

View reviewed changes

hachikuji merged commit c003cc6 into apache:trunk Jul 12, 2021

KAFKA-12677: return not_controller error in envelope response itself #10794

KAFKA-12677: return not_controller error in envelope response itself #10794

Conversation

showuon commented May 30, 2021 • edited Loading

Committer Checklist (excluded from commit message)

dengziming commented May 30, 2021

showuon commented May 31, 2021

showuon commented May 31, 2021

dengziming left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

showuon commented Jun 1, 2021 • edited Loading

showuon commented Jun 1, 2021

showuon commented Jun 3, 2021 • edited Loading

wenbingshen left a comment

Choose a reason for hiding this comment

showuon commented Jun 18, 2021

hachikuji commented Jun 18, 2021

showuon commented Jun 19, 2021

showuon commented Jun 19, 2021

Choose a reason for hiding this comment

showuon commented Jun 24, 2021

hachikuji left a comment

Choose a reason for hiding this comment

hachikuji Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

showuon commented Jul 8, 2021

showuon commented Jul 8, 2021

hachikuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

showuon commented Jul 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hachikuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hachikuji Jul 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

showuon commented Jul 10, 2021

hachikuji left a comment

Choose a reason for hiding this comment

showuon commented May 30, 2021 •

edited

Loading

showuon commented Jun 1, 2021 •

edited

Loading

showuon commented Jun 3, 2021 •

edited

Loading

hachikuji Jul 7, 2021 •

edited

Loading

hachikuji Jul 9, 2021 •

edited

Loading