-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix inconsistent prompt message when schema version is empty using AVRO. #14626
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
Conversation
nodece
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
congbobo184
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
gaoran10
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| if (commandGetSchema.getSchemaVersion().length == 0) { | ||
| commandSender.sendGetSchemaErrorResponse(requestId, ServerError.IncompatibleSchema, | ||
| "Empty schema version"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I debugged testAvroSchemaWithTcpLookup and testAvroSchemaWithHttpLookup, it never went here. Then I removed these lines, tests still passed.
Motivation
When create a topic with schema - AVRO, if a producer sends bytes data directly to the topic, a consumer with HTTP and TCP lookup has different prompt message. See below test:
If the consumer is using TCP lookup, it will get the below message :
But with HTTP lookup, the error message change to :
The root cause is that : if producer produce with bytes data, the schema version is empty. When in TCP lookup, the server will throw unchecked exception here(line-244) :
pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/BookkeeperSchemaStorage.java
Lines 236 to 245 in 7c1f17a
When in HTTP lookup, the error will throw by the client here(line-164):
pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpLookupService.java
Lines 161 to 165 in 7c1f17a
So in order to keep the consistent prompt message, we will check the empty schema at the client-side and throw the same exception.
Modifications
Documentation
no-need-doc