Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Error Message for Schema Evolution Failure #4647

Closed
agavra opened this issue Feb 26, 2020 · 4 comments
Closed

Better Error Message for Schema Evolution Failure #4647

agavra opened this issue Feb 26, 2020 · 4 comments
Assignees
Labels
fix-it-week P1 Slightly lower priority to P0 ;) pluggable-schemas
Milestone

Comments

@agavra
Copy link
Contributor

agavra commented Feb 26, 2020

Today we check for avro schema compatibility when we create a new schema registry subject:

https://github.com/confluentinc/ksql/blob/master/ksql-engine/src/main/java/io/confluent/ksql/engine/EngineExecutor.java#L293

This has a lot of avro-specific code that we should make more generic. It probably also doesn't belong when making the sink DDL but when we create the topic.

Either way, we should make sure that this evolution check is there for both JSON and PROTOBUF when using schema registry.

Also see #4219, which will hopefully fix this issue as well.

@agavra agavra added P0 Denotes must-have for a given milestone pluggable-schemas labels Feb 26, 2020
@agavra agavra self-assigned this Feb 26, 2020
@agavra agavra added this to the 0.8.0 milestone Feb 26, 2020
@agavra
Copy link
Contributor Author

agavra commented Mar 14, 2020

Schema evolution is now fixed with #4717 but the error message isn't very good...

ksql> CREATE STREAM foo_one (id VARCHAR) WITH (kafka_topic='foo', value_format='PROTOBUF', partitions=1);

 Message
----------------
 Stream created
----------------
ksql> CREATE STREAM foo_two (id INT) WITH (kafka_topic='foo', value_format='PROTOBUF');
Could not register schema for topic.
ksql> CREATE STREAM foo_two (id VARCHAR, col2 VARCHAR) WITH (kafka_topic='foo', value_format='PROTOBUF');

 Message
----------------
 Stream created
----------------
ksql>

There are two TODOs to come out of this:

  • better error message for when schema registration fails for compatibility reasons
  • remove the old AvroUtil schema registry checks as they are no longer needed

Furthermore, the AVRO path doesn't even get to the part that it used to with AvroUtil because it gets cut short earlier - so the error message has some level of regression.

@agavra agavra removed the P0 Denotes must-have for a given milestone label Mar 14, 2020
@agavra agavra changed the title Check Schema Evolution for PROTOBUF/JSON Better Error Message for Schema Evolution Failure Mar 14, 2020
@vcrfxia vcrfxia modified the milestones: 0.8.0, 0.9.0 Mar 18, 2020
@spena spena modified the milestones: 0.9.0, 0.10.0 Apr 27, 2020
@agavra agavra removed their assignment Jun 23, 2020
@agavra agavra added the P1 Slightly lower priority to P0 ;) label Jun 23, 2020
@agavra agavra removed this from the 0.10.0 milestone Jun 23, 2020
@rmoff
Copy link
Contributor

rmoff commented Oct 1, 2020

Can we look to fix this error message soon? It's not very helpful, and requires the user to go to the Schema Registry log to see the cause, e.g.:

io.confluent.kafka.schemaregistry.rest.exceptions.RestIncompatibleSchemaException: Schema being registered is incompatible with an earlier schema for subject "livecyclehireupdates_protobuf_02-value"

@vcrfxia
Copy link
Contributor

vcrfxia commented Nov 21, 2020

Hey @rmoff , what ksqlDB version were you running in October when you reported that this error message still hadn't been fixed? I tried replicating the issue just now but it appears to have been fixed:

ksql> create stream foo (id int) with (kafka_topic='foo', value_format='avro', partitions=1);

 Message
----------------
 Stream created
----------------
ksql> create stream foo2 with (kafka_topic='foo') as select 'string' as id from foo;
Could not register schema for topic: Schema being registered is incompatible with an earlier schema; error code: 409

Note that I had to try reproducing with a CSAS statement rather than another CS statement since ksqlDB no longer registers new schemas for CS statements if there's already an existing one in Schema Registry.

There are two TODOs to come out of this:

better error message for when schema registration fails for compatibility reasons
remove the old AvroUtil schema registry checks as they are no longer needed

The second TODO has already been completed in #6452, so unless I've missed something here this issue can be closed.

@rmoff
Copy link
Contributor

rmoff commented Nov 23, 2020

@vcrfxia good question :D I should have included that.
If you can't reproduce it then let's close for now and if I encounter it again I'll reopen with info that's more helpful this time.

@rmoff rmoff closed this as completed Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix-it-week P1 Slightly lower priority to P0 ;) pluggable-schemas
Projects
None yet
Development

No branches or pull requests

6 participants