mimaison commented on code in PR #18016:
URL: https://github.com/apache/kafka/pull/18016#discussion_r1877882664
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -921,26 +902,6 @@ class KafkaConfig private(doLog: Boolean, val props:
util.Map[_, _])
validateControllerQuorumVotersMustContainNodeIdForKRaftController()
validateAdvertisedControllerListenersNonEmptyForKRaftController()
validateControllerListenerNamesMustAppearInListenersForKRaftController()
- } else {
- // ZK-based
- if (migrationEnabled) {
- require(brokerId >= 0,
- "broker.id generation is incompatible with ZooKeeper migration.
Please stop using it before enabling migration (set broker.id to a value
greater or equal to 0).")
- validateQuorumVotersAndQuorumBootstrapServerForMigration()
- require(controllerListenerNames.nonEmpty,
- s"${KRaftConfigs.CONTROLLER_LISTENER_NAMES_CONFIG} must not be empty
when running in ZooKeeper migration mode: ${controllerListenerNames.asJava}")
- require(interBrokerProtocolVersion.isMigrationSupported, s"Cannot
enable ZooKeeper migration without setting " +
- s"'${ReplicationConfigs.INTER_BROKER_PROTOCOL_VERSION_CONFIG}' to
3.4 or higher")
- if (logDirs.size > 1) {
- require(interBrokerProtocolVersion.isDirectoryAssignmentSupported,
- s"Cannot enable ZooKeeper migration with multiple log directories
(aka JBOD) without setting " +
- s"'${ReplicationConfigs.INTER_BROKER_PROTOCOL_VERSION_CONFIG}' to
${MetadataVersion.IBP_3_7_IV2} or higher")
- }
- } else {
Review Comment:
Should we need to keep this `else` block for now? It does not seem part of
the migration logic
##########
core/src/main/scala/kafka/zk/ZkData.scala:
##########
@@ -1047,39 +1046,6 @@ object FeatureZNode {
object MigrationZNode {
Review Comment:
Do we need to keep this object?
##########
metadata/src/main/java/org/apache/kafka/image/MetadataDelta.java:
##########
@@ -247,8 +246,7 @@ public void replay(ApiMessage record) {
*/
break;
case ZK_MIGRATION_STATE_RECORD:
- replay((ZkMigrationStateRecord) record);
- break;
+ throw new UnsupportedOperationException("ZK migration is no
longer supported.");
Review Comment:
Same as in QuorumController
##########
metadata/src/main/java/org/apache/kafka/controller/QuorumController.java:
##########
@@ -1250,8 +1248,7 @@ private void replay(ApiMessage message,
Optional<OffsetAndEpoch> snapshotId, lon
// NoOpRecord is an empty record and doesn't need to be
replayed
break;
case ZK_MIGRATION_STATE_RECORD:
- featureControl.replay((ZkMigrationStateRecord) message);
- break;
+ throw new UnsupportedOperationException("ZK migration is no
longer supported.");
Review Comment:
When adding a new controller, would it be possible that
ZK_MIGRATION_STATE_RECORD records are still in the metadata log from a
migration done previously?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
