Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-18 Thread via GitHub
ijuma commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1817732550 @cmccabe Please check all the Java/Scala versions before merging, this PR clearly broke the Java 8/Scala 2.12 build in multiple ways. -- This is an automated message from the Apache Git

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-17 Thread via GitHub
dajac commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1816721578 I've also seen `Build / JDK 8 and Scala 2.12 / testAssignmentAggregation() – kafka.server.AssignmentsManagerTest` failing consistently in a few builds

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-16 Thread via GitHub
dajac commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1815873092 It seems that a compilation error was introduced by this PR. From the last "JDK 8 and Scala 2.12" build: ``` [Error]

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-16 Thread via GitHub
cmccabe commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1815535748 commited, thanks -- 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

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-16 Thread via GitHub
cmccabe closed pull request #14369: KAFKA-15357: Aggregate and propagate assignments URL: https://github.com/apache/kafka/pull/14369 -- 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

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-16 Thread via GitHub
cmccabe commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1815518652 > @cmccabe I'm confused, I thought that was the whole point of kafka.utils.Exit https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/utils/Exit.scala#L21-L24

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-16 Thread via GitHub
soarez commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1814927398 @cmccabe @pprovenzano PTAL -- 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

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-14 Thread via GitHub
soarez commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1810719729 > avoids calling exit(1) in junit tests, which will kill Jenkins dead (even after 3 decades of Java, we don't have the technology to intercept exit() in unit testrs >:( ) @cmccabe

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-14 Thread via GitHub
OmniaGM commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1392653351 ## core/src/test/java/kafka/server/AssignmentsManagerTest.java: ## @@ -0,0 +1,234 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-14 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1392636901 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-14 Thread via GitHub
OmniaGM commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1810263837 I agree with @cmccabe regarding passing a reference for `AssignmentHandler` to `ReplicaManager`. Maybe one other note, many integration tests are failing now with the following error

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-14 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1392624170 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-14 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1392624170 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-14 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1392487870 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391783416 ## core/src/main/scala/kafka/server/BrokerServer.scala: ## @@ -277,6 +279,24 @@ class BrokerServer( time ) + if (config.logDirs.size > 1) {

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391782832 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391781726 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391780510 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391779074 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,384 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1809242988 I don't know if we want to do this in this PR, but one thing I would suggest for ReplicaManager is to use the `FaultHandler` paradigm we have in the `QuorumController` code.

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1809239029 I don't have a good place to put this comment (github only lets me comment on changed lines) but there is a problem with this code: ``` private class

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391771537 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -2323,15 +2326,56 @@ class ReplicaManager(val config: KafkaConfig, }

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391770284 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -269,7 +270,9 @@ class ReplicaManager(val config: KafkaConfig,

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
cmccabe commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1391763417 ## clients/src/main/java/org/apache/kafka/common/requests/AssignReplicasToDirsRequest.java: ## @@ -27,6 +27,8 @@ public class AssignReplicasToDirsRequest extends

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-13 Thread via GitHub
pprovenzano commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1808630427 LGTM -- 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

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-10 Thread via GitHub
soarez commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1805892764 Is there a way to trigger the tests again or do I need to push additional changes? All the test pipelines show a Jenkins error: "No space left on device". -- This is an automated

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-10 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1389229875 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,386 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-08 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1386805266 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,386 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-08 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1386805266 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,386 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-08 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1386805266 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,386 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-08 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1386810622 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -2296,13 +2298,41 @@ class ReplicaManager(val config: KafkaConfig, if (sendZkNotification)

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-08 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1386805266 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,386 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-08 Thread via GitHub
pprovenzano commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1386788917 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -2296,13 +2298,41 @@ class ReplicaManager(val config: KafkaConfig, if (sendZkNotification)

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-08 Thread via GitHub
soarez commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1386609951 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -2296,13 +2298,41 @@ class ReplicaManager(val config: KafkaConfig, if (sendZkNotification)

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-07 Thread via GitHub
pprovenzano commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1385399770 ## core/src/main/java/kafka/server/AssignmentsManager.java: ## @@ -0,0 +1,386 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-07 Thread via GitHub
pprovenzano commented on code in PR #14369: URL: https://github.com/apache/kafka/pull/14369#discussion_r1385385403 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -2296,13 +2298,41 @@ class ReplicaManager(val config: KafkaConfig, if (sendZkNotification)

Re: [PR] KAFKA-15357: Aggregate and propagate assignments [kafka]

2023-11-06 Thread via GitHub
soarez commented on PR #14369: URL: https://github.com/apache/kafka/pull/14369#issuecomment-1795077968 @cmccabe @pprovenzano this one also builds on [KAFKA-15451](https://github.com/apache/kafka/pull/14368) (#14368). Please take a look. -- This is an automated message from the Apache