feat: fail startup if command contains incompatible version #5104
Conversation
| @VisibleForTesting | ||
| static class VersionChecker { | ||
|
|
||
| private static final RangeMap<ArtifactVersion, Range<ArtifactVersion>> DB_COMPAT_MATRIX = |
There was a problem hiding this comment.
Consider just a simple monotonically incrementing version, rather than this.
e.g. we start at version 1. (anything without a version is version 0.). When the server starts it checks each command version is the current version, otherwise fails.
As we release a new version of ksql with an incompatibilities we bump the verison. The new version will then reject all older (and newer!) commands.
There was a problem hiding this comment.
that is indeed much simpler - I will go down this path, but I think we need to start with version 0 so that this isn't backwards incompatible unnecessarily
ksqldb-rest-app/src/main/java/io/confluent/ksql/rest/server/computation/Command.java
Outdated
Show resolved
Hide resolved
| @@ -61,6 +62,7 @@ public void shouldSerializeDeserializeCorrectly() throws IOException { | |||
| grep(serialized, ".*\"streamsProperties\" *: *\\{ *\"foo\" *: *\"bar\" *\\}.*"); | |||
| grep(serialized, ".*\"statement\" *: *\"test statement;\".*"); | |||
| grep(serialized, ".*\"originalProperties\" *: *\\{ *\"biz\" *: *\"baz\" *\\}.*"); | |||
| grep(serialized, ".*\"version\" *: *\"" + Version.getVersion() + "\".*"); | |||
There was a problem hiding this comment.
Add test to ensure old json, without version, can be deserialized?
There was a problem hiding this comment.
That's what the first test does - I suppose what we need is to make sure that the new json can be deserialized properly 😂
agavra
changed the title
Description
fixes #5030 (somewhat)
Introduces a mechanism to fail if we read commands from the command topic that were produced with an incompatible version. The version compatibility matrix is hard-coded in the
Command.VersionCheckerclass.At the moment, it doesn't fail anything because commands from previous versions won't have a version associated with them.
Testing done
Tested that the framework works with new unit tests
-TODO: when we actually want to leverage this feature, we should make sure that it indeed fails startup. Right now there's no way because all events are accepted :)
Reviewer checklist