[GitHub] [kafka] showuon commented on a diff in pull request #13355: Revert "KAFKA-14371: Remove unused clusterId field from quorum-state file"

2023-03-06 Thread via GitHub 


showuon commented on code in PR #13355:
URL: https://github.com/apache/kafka/pull/13355#discussion_r1127240280


##
raft/src/main/resources/common/message/QuorumStateData.json:
##
@@ -16,10 +16,10 @@
 {
   "type": "data",
   "name": "QuorumStateData",
-  // Version 1 removes clusterId field.
-  "validVersions": "0-1",
+  "validVersions": "0",
   "flexibleVersions": "0+",
   "fields": [
+{"name": "ClusterId", "type": "string", "versions": "0+"},

Review Comment:
   Changing the version to "0" cannot fix this backward compatibility issue. 
The version is decided during writing the state file with highest supported 
version. So, once we write the file with a newer version format, the downgrade 
will fail (since we're trying to remove a field). Agree to revert the change 
first, and then decide if we want to raise a KIP to fix it. 



-- 
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

[GitHub] [kafka] showuon commented on a diff in pull request #13355: Revert "KAFKA-14371: Remove unused clusterId field from quorum-state file"

2023-03-06 Thread via GitHub 


showuon commented on code in PR #13355:
URL: https://github.com/apache/kafka/pull/13355#discussion_r1127240280


##
raft/src/main/resources/common/message/QuorumStateData.json:
##
@@ -16,10 +16,10 @@
 {
   "type": "data",
   "name": "QuorumStateData",
-  // Version 1 removes clusterId field.
-  "validVersions": "0-1",
+  "validVersions": "0",
   "flexibleVersions": "0+",
   "fields": [
+{"name": "ClusterId", "type": "string", "versions": "0+"},

Review Comment:
   Changing the version to "0" cannot fix this backward compatibility issue. 
The version is decided during writing the state file with highest supported 
version. So, once we write the file with a newer version format, the downgrade 
will fail. Agree to revert the change first, and then decide if we want to 
raise a KIP to fix it. 



-- 
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

[GitHub] [kafka] showuon commented on a diff in pull request #13355: Revert "KAFKA-14371: Remove unused clusterId field from quorum-state file"

2023-03-06 Thread via GitHub 


showuon commented on code in PR #13355:
URL: https://github.com/apache/kafka/pull/13355#discussion_r1127240280


##
raft/src/main/resources/common/message/QuorumStateData.json:
##
@@ -16,10 +16,10 @@
 {
   "type": "data",
   "name": "QuorumStateData",
-  // Version 1 removes clusterId field.
-  "validVersions": "0-1",
+  "validVersions": "0",
   "flexibleVersions": "0+",
   "fields": [
+{"name": "ClusterId", "type": "string", "versions": "0+"},

Review Comment:
   Changing the version to "0" cannot fix this backward compatibility issue. 
The version is designed during writing the state file with highest supported 
version. So, once we write the file with a newer version format, the downgrade 
will fail. Agree to revert the change first, and then decide if we want to 
raise a KIP to fix it. 



-- 
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