search

riferrei / srclient

Golang Client for Schema Registry
Apache License 2.0
235 stars 70 forks source link

Add reset cache mechanism to SchemaRegistryClient #58

Closed melihaydogd closed 2 years ago

melihaydogd commented 3 years ago

After caching, the schema in the cache does not change. If the latest schema changes on the schema registry, the new schema will not be retrieved. This reset mechanism allows retrieving the most updated version of the schema without the need to exit from application.

The cache could be reset periodically to retrieve the most updated version.

AtakanColak commented 3 years ago

I would like to ask why we would prefer to add a ResetCache method rather than the two alternatives below;

  1. Just recreate a SchemaRegistryClient object (no change required)
  2. Just disable caching, and a PR to update cache even though caching is disabled
riferrei commented 3 years ago

Yeah, I agree with @AtakanColak. If the idea is to favor updated versions of the schema — then disabling caching makes more sense. Enabling caching must always be a conscious trade-off between performance and consistency.

melihaydogd commented 3 years ago

Answer for Atakan:

  1. I think it would be inefficient to create the schema registry again considering I only need to modify two of the fields.
  2. I still need to enable and disable cache periodically and I have couple concerns about this:
    1. If I have high traffic and cache is disabled the same schema will be retrieved again and again until the caching is enabled.
    2. If I have low traffic on a topic, the new schema may not be retrieved because no data is retrieved when caching is disabled for that topic.

Yes, caching has always a trade-off and the solution I came up with is a very simple solution. However, I think there should be some expire or purge mechanism in a cache to be able to refresh it as in this repository. If I publish 10000 data per second and I only update the schema one a day, disabling cache altogether would reduce the performance unnecessarily.

melihaydogd commented 2 years ago

Thank you for your comments.

AtakanColak commented 2 years ago

@melihaydogd I disagree with closing this PR, especially your point in 2 is highly in favour of this change. However I wanted to double check our requirement by discussing alternatives.

AtakanColak commented 2 years ago

@riferrei I'll merge this if you won't object

riferrei commented 2 years ago

Nothing to object. LGTM 👍🏻