You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/04/20 20:45:55 UTC

[GitHub] [kafka] abbccdda opened a new pull request #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

abbccdda opened a new pull request #8522:
URL: https://github.com/apache/kafka/pull/8522


   This PR tries to fix the flaky EOSUncleanShutdownIntegrationTest.shouldWorkWithUncleanShutdownWipeOutStateStore by making the bootstrapping of the test to be less painful with fewer number of partitions of txn log.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] mjsax commented on a change in pull request #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #8522:
URL: https://github.com/apache/kafka/pull/8522#discussion_r411719647



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -84,7 +84,7 @@ public EmbeddedKafkaCluster(final int numBrokers,
     /**
      * Creates and starts a Kafka cluster.
      */
-    public void start() throws IOException, InterruptedException {
+    public void start() throws IOException {

Review comment:
       If we would have written `throws Exception` from the beginning on, this change would not be necessary... (Just to back up my preferred coding stile to only use `throws Exception` in tests.)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] mjsax commented on issue #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
mjsax commented on issue #8522:
URL: https://github.com/apache/kafka/pull/8522#issuecomment-616831664


   Retest this please.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] abbccdda commented on issue #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
abbccdda commented on issue #8522:
URL: https://github.com/apache/kafka/pull/8522#issuecomment-616917436


   Got 2/3 green, one failed test is due to
   ```
   kafka.api.SaslSslConsumerTest.testCoordinatorFailover
   
   java.lang.AssertionError: expected:<None> but was:<Some(org.apache.kafka.clients.consumer.CommitFailedException: Offset commit cannot be completed since the consumer is not part of an active group for auto partition assignment; it is likely that the consumer was kicked out of the group.)>
   ```
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] abbccdda commented on issue #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
abbccdda commented on issue #8522:
URL: https://github.com/apache/kafka/pull/8522#issuecomment-616839537


   Run 350 times on local and no failure.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] abbccdda commented on a change in pull request #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
abbccdda commented on a change in pull request #8522:
URL: https://github.com/apache/kafka/pull/8522#discussion_r411727585



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -84,7 +84,7 @@ public EmbeddedKafkaCluster(final int numBrokers,
     /**
      * Creates and starts a Kafka cluster.
      */
-    public void start() throws IOException, InterruptedException {
+    public void start() throws IOException {

Review comment:
       Lol, makes sense.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] abbccdda commented on issue #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
abbccdda commented on issue #8522:
URL: https://github.com/apache/kafka/pull/8522#issuecomment-616840036


   @mjsax I suppose this should help for all integration tests, as they share `EmbeddedBroker`


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] abbccdda commented on a change in pull request #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
abbccdda commented on a change in pull request #8522:
URL: https://github.com/apache/kafka/pull/8522#discussion_r411680309



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -98,6 +98,7 @@ public void start() throws IOException, InterruptedException {
         putIfAbsent(brokerConfig, KafkaConfig$.MODULE$.GroupInitialRebalanceDelayMsProp(), 0);
         putIfAbsent(brokerConfig, KafkaConfig$.MODULE$.OffsetsTopicReplicationFactorProp(), (short) 1);
         putIfAbsent(brokerConfig, KafkaConfig$.MODULE$.OffsetsTopicPartitionsProp(), 5);
+        putIfAbsent(brokerConfig, KafkaConfig$.MODULE$.TransactionsTopicPartitionsProp(), 5);

Review comment:
       This is the actual fix, other parts are just side cleanups.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] abbccdda commented on issue #8522: KAFKA-9868: Reduce transaction log partitions for embed broker

Posted by GitBox <gi...@apache.org>.
abbccdda commented on issue #8522:
URL: https://github.com/apache/kafka/pull/8522#issuecomment-616808689


   @vvcephei @mjsax Call for a review


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org