You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "akhileshchg (via GitHub)" <gi...@apache.org> on 2023/05/18 17:43:53 UTC
[GitHub] [kafka] akhileshchg opened a new pull request, #13732: KAFKA-15007: Always update the MetadataPropagator with correct MV.
akhileshchg opened a new pull request, #13732:
URL: https://github.com/apache/kafka/pull/13732
Even if the MigrationDriver is not in DUAL_WRITE mode, it should pass the MV changes to the MetadataPropagator. If not, the propagator might not have the right version to send UMR, LISR and StopReplica requests when the migration is in DUAL_WRITE mode.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mumrah merged pull request #13732: KAFKA-15007: Always update the MetadataPropagator with correct MV.
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah merged PR #13732:
URL: https://github.com/apache/kafka/pull/13732
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akhileshchg commented on a diff in pull request #13732: KAFKA-15007: Always update the MetadataPropagator with correct MV.
Posted by "akhileshchg (via GitHub)" <gi...@apache.org>.
akhileshchg commented on code in PR #13732:
URL: https://github.com/apache/kafka/pull/13732#discussion_r1200876893
##########
core/src/main/scala/kafka/migration/MigrationPropagator.scala:
##########
@@ -70,7 +68,7 @@ class MigrationPropagator(
val requestBatch = new MigrationPropagatorBatch(
config,
metadataProvider,
- () => metadataVersion,
+ () => _image.features().metadataVersion(),
Review Comment:
Yes. That's correct. For MigrationPropagator to first publish something, it should be called with an associated image. We use this image to setup the correct metadata version required to send the requests to ZkBrokers.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mumrah commented on pull request #13732: KAFKA-15007: Always update the MetadataPropagator with correct MV.
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #13732:
URL: https://github.com/apache/kafka/pull/13732#issuecomment-1554999869
@akhileshchg what do you think about passing a supplier down to MetadataPropagator rather than the MV value? This way, we'll always be guaranteed to be using the MV in the MetadataImage known to KRaftMigrationDriver.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mumrah commented on a diff in pull request #13732: KAFKA-15007: Always update the MetadataPropagator with correct MV.
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #13732:
URL: https://github.com/apache/kafka/pull/13732#discussion_r1200868934
##########
core/src/main/scala/kafka/migration/MigrationPropagator.scala:
##########
@@ -70,7 +68,7 @@ class MigrationPropagator(
val requestBatch = new MigrationPropagatorBatch(
config,
metadataProvider,
- () => metadataVersion,
+ () => _image.features().metadataVersion(),
Review Comment:
One subtle difference in behavior here is that the MV of the empty image is IBP_3_0_IV1, so the default we previously had of `IBP_3_4_IV0` is no longer seen.
However.. I don't think we actually hit these defaults in practice since we won't call the metadata propagator until we've seen at least one metadata publish. @akhileshchg can you confirm if my assumption is correct?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org