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 go
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, sug
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 c
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`.
https://ge.apache.org/scans/tests?sear
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: jira-unsubscr...@kafka.ap
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 does
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):
https://ci-builds.apache.org/blue/organizations
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
fail
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:
https://ci-bui
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 respon
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
https://github
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 new
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 = isMi
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 separat
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 ne
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 ne
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 ne
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 == fence
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 = isMi
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 ne
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 ne
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 faili
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 respo
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 new
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 {
*/
privat
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,
MetadataImag
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 ne
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 ne
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 {
*/
priva
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 activate(
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.
https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-14838/9/te
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 new
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 {
*/
privat
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 activate(
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 abov
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 the
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 the
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 implementat
52 matches
Mail list logo
