cmccabe commented on PR #16230: URL: https://github.com/apache/kafka/pull/16230#issuecomment-2174664455
I was originally going to have two records: one `KRaftVersionRecord` for the raft layer, and one `FeatureRecord` for the metadata layer, encoding the same data. The advantage of doing it this way is that it will work well with the current code structure. For example, we have code that validates that FeaturesImage can dump its state and restore that state. If FeaturesImage is taking input from outside of the list of metadata records, that invariant is broken. We also currently don't propagate control records to the metadata layer, so that is something we'd have to consider changing. The big disadvantage of having two records rather than one is the state can get out of sync. Which I do think is a real risk, especially when we are changing things. I realize writing the record at the Raft layer seems useful to you, but in the metadata layer, the thought that `read(image.write) != image` does not spark joy. If this is going to be handled by the Raft layer and not the metadata layer, then maybe it should be excluded from the image altogether. We'll have to talk about that. In the meantime, I merged in trunk. There were import statement conflicts. -- 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
