Revision 2 of the ksql storage impl #1054
Conversation
… details to iron out. But ready for review
storage/ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/sql/KafkaSqlStore.java
Show resolved
Hide resolved
storage/ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/sql/KafkaSqlStore.java
Show resolved
Hide resolved
storage/ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/KafkaSqlRegistryStorage.java
Show resolved
Hide resolved
| UUID reqId = ConcurrentUtil.get(submitter.submitRule(Str.ActionType.DELETE, GLOBAL_RULES_ID, null, null)); | ||
| coordinator.waitForResponse(reqId); | ||
|
|
||
| // TODO find a better way to implement deleting all global rules? This isn't very scalable if we add more rules. |
There was a problem hiding this comment.
use here log compaction to remove all global rules at once? It's ok if not implemented now, but add a note here then
There was a problem hiding this comment.
How would we use log compaction for this? We need multiple messages - one for each rule. No?
storage/ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/keys/MessageType.java
Outdated
Show resolved
Hide resolved
storage/ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/values/ActionType.java
Outdated
Show resolved
Hide resolved
storage/ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/KafkaSqlRegistryStorage.java
Show resolved
Hide resolved
| sqlStore.deleteArtifactVersion(key.getArtifactId(), key.getVersion()); | ||
| return null; | ||
| case Clear: | ||
| sqlStore.deleteArtifactVersionMetaData(key.getArtifactId(), key.getVersion()); |
There was a problem hiding this comment.
I was not aware we had overlapping concepts , having delete and clear is not perfect as the meaning is similar but I guess it's ok if we have no other option
There was a problem hiding this comment.
What about joining both operations into a single one? Maybe I am missing something, but if the version is deleted, then the metadata can be safely deleted as well.
There was a problem hiding this comment.
Yeah I didn't love this solution, but I didn't have a better one. I'm open to other options. The semantics here are actually:
- Delete == delete the artifact version
- Clear == delete the artifact version metadata
They are different actions associated with the same message key, which is important IMO for log compaction. If you can think of either a better action name, or another approach, I'm happy to hear it.
...ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/serde/KafkaSqlValueSerializer.java
Show resolved
Hide resolved
...e/ksql/src/main/java/io/apicurio/registry/storage/impl/ksql/serde/KafkaSqlKeySerializer.java
Show resolved
Hide resolved
app/src/main/java/io/apicurio/registry/storage/RegistryStorageException.java
Outdated
Show resolved
Hide resolved
app/src/main/java/io/apicurio/registry/storage/RegistryStorageException.java
Outdated
Show resolved
Hide resolved
Hey @Apicurio/developers - this is a PR of a second revision/evolution of the kafka+sql hybrid model we've been working on. I think it captures most of what we documented in our design doc. There are still some missing pieces, but I think it's ready for review so we can identify and itemize the remaining tasks.