soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1847073339
@dajac, @lucasbru: Please take a look at #14976
--
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
soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1847054278
@dajac: I'm having a look.
`ZkMigrationIntegrationTest.testMigrateTopicDeletions` and
`ZkMigrationIntegrationTest.testDualWrite` are consistently failing for
`IBP_3_7_IV2` or greater,
dajac commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1846915396
@soarez Could you take a look?
--
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
lucasbru commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1846898831
There are some tests that seem to have gotten a lot worse since this PR was
merged, as far as I can tell, e.g. `ZkMigrationIntegrationTest`.
rondagostino merged PR #14838:
URL: https://github.com/apache/kafka/pull/14838
--
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:
cmccabe commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1843728301
@rondagostino : I think we can merge this now because I don't think it's
adding more failures. I do think we need to do another sweep looking for people
calling `Exit.exit`, but that
rondagostino commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1843693525
Looking at the [trunk build
jobs](https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka/activity?branch=trunk):
rondagostino commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1843668221
I think the issue is not with our PR. Here's another [PR that is basically
a 2-line change in a single test](https://github.com/apache/kafka/pull/14944)
and that has [144 test
rondagostino commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1843665517
[Latest builds for JDKs 8 and 21
failed](https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-14838/16/pipeline/).
Restarted the build:
soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1843250009
@rondagostino I'm trying to figure the test failures out, I've not yet been
able to reproduce any failures locally.
--
This is an automated message from the Apache Git Service.
To
rondagostino commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1843104348
[Still 110 failing
tests](https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-14838/14/tests),
which seems like a lot, especially since
soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1841864233
@cmccabe:
> I didn't understand the purpose of the HeartbeatManager changes (now that
we've agreed to keep Directory out of HBM). Seems like we should be able to
keep this file the
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416458768
##
metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java:
##
@@ -272,8 +274,17 @@ public void testRegistrationWithIncorrectClusterId() {
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416458244
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +357,25 @@ public ControllerResult
registerBroker(
throw
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416350081
##
metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistration.java:
##
@@ -192,7 +192,7 @@ private BrokerRegistration(
this.isMigratingZkBroker =
cmccabe commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1841725640
Sorry, one more thing: I didn't understand the purpose of the
HeartbeatManager changes. Seems like we should be able to keep this file the
same now? Or if not, can we do them in a
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416342392
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +357,25 @@ public ControllerResult
registerBroker(
throw
rondagostino commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416337073
##
metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistration.java:
##
@@ -192,7 +192,7 @@ private BrokerRegistration(
this.isMigratingZkBroker
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416327866
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +357,25 @@ public ControllerResult
registerBroker(
throw
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416317197
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +357,25 @@ public ControllerResult
registerBroker(
throw
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416324308
##
metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java:
##
@@ -272,8 +274,17 @@ public void testRegistrationWithIncorrectClusterId() {
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416321816
##
metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistration.java:
##
@@ -337,7 +341,7 @@ public boolean equals(Object o) {
other.fenced ==
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416321397
##
metadata/src/main/java/org/apache/kafka/metadata/BrokerRegistration.java:
##
@@ -192,7 +192,7 @@ private BrokerRegistration(
this.isMigratingZkBroker =
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416319067
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +357,25 @@ public ControllerResult
registerBroker(
throw
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416317197
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +357,25 @@ public ControllerResult
registerBroker(
throw
rondagostino commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1841623911
https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-14838/13/tests
```
208 tests have failed
There are 20 new tests failing, 188 existing
soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1841564307
Thanks for the review.
@rondagostino AFAICT, the tests are passing on all JDK versions. Did I miss
something?
--
This is an automated message from the Apache Git Service.
To
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1416166357
##
server-common/src/main/java/org/apache/kafka/common/DirectoryId.java:
##
@@ -121,8 +122,38 @@ public static Map createAssignmentMap(int[]
replicas, Uuid[] dire
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1415818541
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +356,25 @@ public ControllerResult
registerBroker(
throw
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1415817804
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -284,7 +286,7 @@ ReplicaPlacer replicaPlacer() {
public void activate() {
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1415813979
##
metadata/src/main/java/org/apache/kafka/controller/BrokerHeartbeatManager.java:
##
@@ -61,6 +67,11 @@ static class BrokerHeartbeatState {
*/
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1415473599
##
server-common/src/main/java/org/apache/kafka/common/DirectoryId.java:
##
@@ -121,8 +122,38 @@ public static Map createAssignmentMap(int[]
replicas, Uuid[] dire
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414713000
##
metadata/src/main/java/org/apache/kafka/image/loader/MetadataLoader.java:
##
@@ -347,7 +347,7 @@ private void maybePublishMetadata(MetadataDelta delta,
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414710963
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +356,25 @@ public ControllerResult
registerBroker(
throw
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414710963
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +356,25 @@ public ControllerResult
registerBroker(
throw
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414705968
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -284,7 +286,7 @@ ReplicaPlacer replicaPlacer() {
public void activate() {
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414704931
##
server-common/src/main/java/org/apache/kafka/common/DirectoryId.java:
##
@@ -121,8 +122,38 @@ public static Map createAssignmentMap(int[]
replicas, Uuid[] dire
cmccabe commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414470698
##
metadata/src/main/java/org/apache/kafka/controller/BrokerHeartbeatManager.java:
##
@@ -61,6 +67,11 @@ static class BrokerHeartbeatState {
*/
rondagostino commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414314870
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -284,7 +286,7 @@ ReplicaPlacer replicaPlacer() {
public void
rondagostino commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1839212241
Looks like there are lots of test failures, too. Seems like you will have
to look (e.g.
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414266329
##
metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java:
##
@@ -272,8 +273,16 @@ public void testRegistrationWithIncorrectClusterId() {
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414263347
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -354,6 +356,25 @@ public ControllerResult
registerBroker(
throw
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414261999
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -284,7 +286,7 @@ ReplicaPlacer replicaPlacer() {
public void activate() {
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414257022
##
metadata/src/main/java/org/apache/kafka/controller/BrokerHeartbeatManager.java:
##
@@ -61,6 +67,11 @@ static class BrokerHeartbeatState {
*/
rondagostino commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414154141
##
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##
@@ -284,7 +286,7 @@ ReplicaPlacer replicaPlacer() {
public void
rondagostino commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1414131035
##
metadata/src/main/java/org/apache/kafka/controller/BrokerHeartbeatManager.java:
##
@@ -61,6 +67,11 @@ static class BrokerHeartbeatState {
*/
soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1836006504
@cmccabe @rondagostino @pprovenzano please take a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL
soarez commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1406207344
##
server-common/src/main/java/org/apache/kafka/common/DirectoryId.java:
##
@@ -121,8 +122,38 @@ public static Map createAssignmentMap(int[]
replicas, Uuid[] dire
pprovenzano commented on code in PR #14838:
URL: https://github.com/apache/kafka/pull/14838#discussion_r1405538961
##
server-common/src/main/java/org/apache/kafka/common/DirectoryId.java:
##
@@ -121,8 +122,38 @@ public static Map createAssignmentMap(int[]
replicas, Uuid[] dire
soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1825859013
@cmccabe @pprovenzano @rondagostino 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
soarez commented on PR #14838:
URL: https://github.com/apache/kafka/pull/14838#issuecomment-1825858721
This builds on #14820 - KAFKA-15886: Always specify directories for new
partition registrations – so this is marked as draft until #14820 is merged.
**Reviews**: please focus on
soarez opened a new pull request, #14838:
URL: https://github.com/apache/kafka/pull/14838
Controllers should process and persist directory information from the broker
registration request
### Committer Checklist (excluded from commit message)
- [ ] Verify design and
52 matches
Mail list logo
