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

Extremely slow protobuf deserialization and serialization for schema registry #1146

Closed
7 tasks
fzmoment opened this issue Mar 4, 2024 · 5 comments
Closed
7 tasks

Comments

@fzmoment
Copy link
Contributor

fzmoment commented Mar 4, 2024

Description

When using protobuf schemas we've noticed that the drop in performance is much worse than expected. For example, compared to the baseline of just a regular protobuf Unmarshal call, which takes on the order of <10 us:

Screenshot 2024-03-01 at 11 16 20 AM

Using the confluent kafka go library, we see a over 2500x increase in processing time:

Screenshot 2024-03-01 at 12 41 35 PM

Furthermore, we know that this decrease is specifically not due to communication with the schema registry. In this example, we are deserializing the same schema over and over again and thus only make a request to the registry once before the result is cached - in general, the schema ID lookup takes ~2us:

Screenshot 2024-03-01 at 12 47 11 PM

The bottleneck is the ToFileDesc function which is parsing the schema string back to a file descriptor.

Screenshot 2024-03-01 at 12 44 45 PM

this can take up on the order of seconds:

Screenshot 2024-03-01 at 12 49 58 PM

Is this a known issue/are there any workarounds? We see similar behavior on the write side (where the bottleneck is the conversion from the file descriptor to a string, not looking up the schema ID for a schema given the caching), though not quite as drastic as the above examples. Any pointers would be appreciated.

How to reproduce

Run a consumer that uses the confluent kafka go library to deserialize all the messages on a given topic. In our setup, the topic schema only contains one message. Can provide a minimal repro example if this gains some traction.

Checklist

Please provide the following information:

  • confluent-kafka-go and librdkafka version (LibraryVersion()):
  • Apache Kafka broker version:
  • Client configuration: ConfigMap{...}
  • Operating system:
  • Provide client logs (with "debug": ".." as necessary)
  • Provide broker log excerpts
  • Critical issue
@rayokota
Copy link
Member

rayokota commented Mar 5, 2024

Did you try this fix? #1128

@rayokota
Copy link
Member

rayokota commented Mar 7, 2024

Closing this for now as the main bottleneck has been addressed

@rayokota rayokota closed this as completed Mar 7, 2024
@fzmoment
Copy link
Contributor Author

fzmoment commented Mar 7, 2024

Thanks Robert! That fix definitely seems to get rid of the bottleneck that we identified in our trace (ToFileDesc) - performance now seems to be pretty close to the raw unmarshal.

I was wondering if we could apply a similar approach on the serialization side as well, where things seem to be slow also because of file descriptor <=> string conversions:

Screenshot 2024-03-07 at 11 38 02 AM

What do you think about adding a similar cache on the serialization side, mapping from desc.FileDescriptor to SchemaInfo (or better yet, to schema ID)? Happy to create a PR for this for you to review if the general approach seems reasonable to you. I think once we resolve things on the serialization side that should address this ticket.

@rayokota
Copy link
Member

rayokota commented Mar 7, 2024

Thanks @fzmoment , sounds good a PR is welcome

@fzmoment
Copy link
Contributor Author

Hi @rayokota , I added an implementation to #1151 if you could take a look. Unfortunately, there was one hiccup to the approach i mentioned which is that if we are caching by file descriptor, and a dependency changes, the caching won't reflect that change, so I put this behavior behind a feature flag. Included more details in the PR, PTAL!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants