KAFKA-17613: Remove ZK migration code#17293
Conversation
0e016a9 to
7547217
Compare
|
As a note, I've taken the approach of leaving |
|
There are some test failures. I think they'll be fixed after we merge #17284 |
|
@cmccabe the other removal PR is merged now, can you merge in trunk? |
| @@ -76,8 +70,6 @@ public class QuorumControllerMetrics implements AutoCloseable { | |||
| private final AtomicLong dualWriteOffset = new AtomicLong(0); | |||
There was a problem hiding this comment.
Is dual-write metrics still used in production code after KRaftMigrationDriver is removed?
There was a problem hiding this comment.
good point, I will remove this.
| setFinalizedFeatures(currentFeatures.finalizedFeatures()). | ||
| setFinalizedFeaturesEpoch(currentFeatures.finalizedFeaturesEpoch()). | ||
| setZkMigrationEnabled(zkMigrationEnabled). | ||
| setZkMigrationEnabled(false). |
There was a problem hiding this comment.
why not removing the setZkMigrationEnabled?
There was a problem hiding this comment.
We cannot remove zkMigrationEnabled from the RPC without breaking backwards compatibility. Although it does default to false, so we could leave out this line (which I think might be more confusing to read, but would be technically correct).
There was a problem hiding this comment.
Maybe we can hardcode zkMigrationReady to false when building ApiVersionsResponse (ApiVersionsResponse#Builder#build) and then remove this setter ?
There was a problem hiding this comment.
Wouldn't that make testing harder? It seems fine to just set it here.
|
I've updated this to remove the lingering reference to dual write offset in the metrics class. [edit: also merged in trunk as @mumrah suggested. Thanks for the reviews, everyone.] |
|
I believe those failing tests can be removed if they are specifically designed to run with ZooKeeper migration |
Remove the controller machinery for doing ZK migration in Kafka 4.0. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, David Arthur <mumrah@gmail.com>
3 participants
Remove the controller machinery for doing ZK migration in Kafka 4.0.
As a note, I've taken the approach of leaving KafkaConfig.migrationEnabled but hard-coding it to
false here. That was because I didn't want to mess around with KafkaServer.scala and related classes.
Since they're getting removed soon anyway, it would be a waste.