AndrewJSchofield commented on code in PR #19659:
URL: https://github.com/apache/kafka/pull/19659#discussion_r2083611522
##########
core/src/main/java/kafka/server/share/SharePartitionManager.java:
##########
@@ -746,6 +747,29 @@ private Consumer<Set<String>>
failedShareAcknowledgeMetricsHandler() {
};
}
+ /**
+ * The handler for share version feature metadata changes.
+ * @param shareVersion the new share version feature
+ */
+ public void onShareVersionToggle(ShareVersion shareVersion) {
+ if (!shareVersion.supportsShareGroups()) {
+ // Remove all share sessions from share session cache.
+ synchronized (cache) {
Review Comment:
There's a race condition here. The user turns off share groups, which blocks
new requests in `KafkaApis`. However, a request had just got past that check
before the feature was downgraded. Now, this code runs which clears the share
session cache, but the in-flight request calls `cache.maybeCreateSession`. We
want cache additions when the feature is not enabled to be prevented I think.
##########
core/src/main/scala/kafka/server/metadata/BrokerMetadataPublisher.scala:
##########
@@ -232,6 +240,23 @@ class BrokerMetadataPublisher(
if (_firstPublish) {
finishInitializingReplicaManager()
}
+
+ if (delta.featuresDelta != null) {
+ try {
+ val newFinalizedFeatures = new
FinalizedFeatures(newImage.features.metadataVersionOrThrow,
newImage.features.finalizedVersions, newImage.provenance.lastContainedOffset)
+ // Share version feature has been toggled.
+ if
(!(newFinalizedFeatures.finalizedFeatures().getOrDefault(ShareVersion.FEATURE_NAME,
0.toShort) == finalizedShareVersion)) {
+ finalizedShareVersion =
newFinalizedFeatures.finalizedFeatures().getOrDefault(ShareVersion.FEATURE_NAME,
0.toShort)
+ val shareVersion: ShareVersion =
ShareVersion.fromFeatureLevel(finalizedShareVersion)
+ info(s"Share version has been toggled to $shareVersion")
Review Comment:
The message when the feature has been enabled is `Share version has been
toggled to SV_1`. The string `SV_1` is only meaningful in the code. It would be
better to say something like `Feature share.version has been updated to version
1.`
--
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.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
