AndrewJSchofield commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1536692361
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -42,14 +42,26 @@ object AddPartitionsToTxnManager {
val
jolshan merged PR #15486:
URL: https://github.com/apache/kafka/pull/15486
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail:
sjhajharia commented on PR #15486:
URL: https://github.com/apache/kafka/pull/15486#issuecomment-2014388345
Thank you @jolshan for the review.
I have updated the test mentioned. Hopefully it gets into trunk soon.
--
This is an automated message from the Apache Git Service.
To respond to
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1534705114
##
core/src/test/scala/unit/kafka/server/AddPartitionsToTxnManagerTest.scala:
##
@@ -106,27 +106,27 @@ class AddPartitionsToTxnManagerTest {
val
sjhajharia commented on PR #15486:
URL: https://github.com/apache/kafka/pull/15486#issuecomment-2008655935
On that note @jolshan , do we any more open questions to address or we are
good to go with this PR? Or are we in flux for
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1530741929
##
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##
@@ -3040,6 +3048,7 @@ class ReplicaManagerTest {
transactionalId = transactionalId,
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1529794514
##
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##
@@ -3040,6 +3048,7 @@ class ReplicaManagerTest {
transactionalId = transactionalId,
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1529793846
##
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java:
##
@@ -3523,6 +3524,128 @@ public void
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1529005304
##
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##
@@ -3040,6 +3048,7 @@ class ReplicaManagerTest {
transactionalId = transactionalId,
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1528996611
##
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java:
##
@@ -3523,6 +3524,128 @@ public void
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1528996611
##
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java:
##
@@ -3523,6 +3524,128 @@ public void
jolshan commented on PR #15486:
URL: https://github.com/apache/kafka/pull/15486#issuecomment-2004546341
Hey @sjhajharia thanks for the updates. I think `ApiVersionErrorMapper` is
also a bit tricky since the addPartitions change will not be related to errors
at all. That's why I was
sjhajharia commented on PR #15486:
URL: https://github.com/apache/kafka/pull/15486#issuecomment-2003011892
@jolshan, I updated the PR addressing the comments you left. I have renamed
the enum to `ApiVersionErrorMapper`. Please do let me know if that looks better.
I will also wait for
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1526869018
##
core/src/test/scala/unit/kafka/server/AddPartitionsToTxnManagerTest.scala:
##
@@ -72,6 +72,8 @@ class AddPartitionsToTxnManagerTest {
private val
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1526868437
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -975,12 +978,13 @@ class ReplicaManager(val config: KafkaConfig,
/**
*
- * @param
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1526864714
##
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java:
##
@@ -3523,6 +3524,129 @@ public void
jolshan commented on PR #15486:
URL: https://github.com/apache/kafka/pull/15486#issuecomment-2000460307
I was thinking about this change and am wondering if we should bump all the
transactional APIs to make it clear this error can be returned. Let me get back
to you @sjhajharia -- I will
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1526861466
##
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java:
##
@@ -392,7 +393,8 @@ public enum Errors {
UNKNOWN_CONTROLLER_ID(116, "This controller ID
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1525774714
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -42,14 +42,26 @@ object AddPartitionsToTxnManager {
val VerificationTimeMsMetricName =
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1525699817
##
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java:
##
@@ -1456,7 +1456,7 @@ public CompletableFuture
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1525699580
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -44,7 +44,7 @@ object AddPartitionsToTxnManager {
/**
* This is an enum which handles
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1523717757
##
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##
@@ -1504,6 +1507,8 @@ public void handleResponse(AbstractResponse
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1523717757
##
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##
@@ -1504,6 +1507,8 @@ public void handleResponse(AbstractResponse
CalvinConfluent commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1523705762
##
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##
@@ -1504,6 +1507,8 @@ public void
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1523608929
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -42,14 +42,26 @@ object AddPartitionsToTxnManager {
val VerificationTimeMsMetricName =
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1523603267
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -42,14 +42,26 @@ object AddPartitionsToTxnManager {
val VerificationTimeMsMetricName =
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1523597267
##
clients/src/main/java/org/apache/kafka/common/errors/AbortableTransactionException.java:
##
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1523586414
##
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java:
##
@@ -392,7 +393,8 @@ public enum Errors {
UNKNOWN_CONTROLLER_ID(116, "This controller ID
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1522533304
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -42,6 +42,11 @@ object AddPartitionsToTxnManager {
val VerificationTimeMsMetricName =
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1521977816
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -42,6 +42,11 @@ object AddPartitionsToTxnManager {
val VerificationTimeMsMetricName =
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1521974720
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -42,6 +42,11 @@ object AddPartitionsToTxnManager {
val VerificationTimeMsMetricName =
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1521780875
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -1007,7 +1010,8 @@ class ReplicaManager(val config: KafkaConfig,
transactionalId,
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1521767609
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -1007,7 +1010,8 @@ class ReplicaManager(val config: KafkaConfig,
transactionalId,
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1521679508
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -1007,7 +1010,8 @@ class ReplicaManager(val config: KafkaConfig,
transactionalId,
sjhajharia commented on PR #15486:
URL: https://github.com/apache/kafka/pull/15486#issuecomment-1987900169
> I took a quick skim of the tests -- just wanted to confirm we have a test
for the old produce request + invalid txn state returned?
@jolshan , if you have a look at
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1519333893
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -1007,7 +1010,8 @@ class ReplicaManager(val config: KafkaConfig,
transactionalId,
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1519327877
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -228,11 +232,17 @@ class AddPartitionsToTxnManager(
val tp = new
sjhajharia commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1519322254
##
clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java:
##
@@ -3151,6 +3152,45 @@ public void testInvalidTxnStateIsAnAbortableError()
jolshan commented on PR #15486:
URL: https://github.com/apache/kafka/pull/15486#issuecomment-1986154766
I took a quick skim of the tests -- just wanted to confirm we have a test
for the old produce request + invalid txn state returned?
--
This is an automated message from the Apache Git
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518065460
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -49,7 +49,8 @@ object AddPartitionsToTxnManager {
*/
class
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518067902
##
core/src/main/scala/kafka/server/ReplicaManager.scala:
##
@@ -1007,7 +1010,8 @@ class ReplicaManager(val config: KafkaConfig,
transactionalId,
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518065460
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -49,7 +49,8 @@ object AddPartitionsToTxnManager {
*/
class
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518064096
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -228,11 +232,17 @@ class AddPartitionsToTxnManager(
val tp = new
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518064096
##
core/src/main/scala/kafka/server/AddPartitionsToTxnManager.scala:
##
@@ -228,11 +232,17 @@ class AddPartitionsToTxnManager(
val tp = new
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518063503
##
clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java:
##
@@ -3151,6 +3152,45 @@ public void testInvalidTxnStateIsAnAbortableError()
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518062498
##
clients/src/main/resources/common/message/ProduceRequest.json:
##
@@ -35,7 +35,10 @@
// Version 9 enables flexible versions.
//
// Version 10 is the same
jolshan commented on code in PR #15486:
URL: https://github.com/apache/kafka/pull/15486#discussion_r1518061441
##
clients/src/main/java/org/apache/kafka/common/errors/AbortableTransactionException.java:
##
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation
47 matches
Mail list logo
