search

hortonworks / registry

Schema Registry
Apache License 2.0
15 stars 8 forks source link

Return actual record removed instead of cached for CacheBackedStorageManager #389

Closed priyank5485 closed 6 years ago

priyank5485 commented 6 years ago

Raising this as a result of this https://github.com/hortonworks/streamline/pull/1164

For HA setup using CacheBackedStorageManager the remove method returns the cached record rather than record present in DB in case of a cache inconsistency. In case of the SAM issue referenced above the cache did not have the record and it returned a null. As per discussion there may be we need to return the record removed from the DB rather than cache.

priyank5485 commented 6 years ago

https://github.com/hortonworks/registry/pull/390

HeartSaVioR commented 6 years ago

Does even caching make sense for H/A? If we need to have validation for checking cache inconsistency all the time, that means cache doesn't need at all. Cache was introduced when we don't consider H/A scenario, and cache can be always inconsistent unless we are going with active-standby H/A.

arunmahadevan commented 6 years ago

Yes, we can look at removing / replacing with a distributed cache in future.

HeartSaVioR commented 6 years ago

I hope we could take the change sooner, because it will be hard to find the reason when relevant issue is occurred, and making cache inconsistent is just easy: once requests are distributed to several SAM/SR instances, local cache will become error-prone.

satishd commented 6 years ago

Thanks @priyank5485 for fixing this issue now with #390.

We should definitely move to use something like Hazelcast IMDG for addressing distributed cache scenarios in registry/streamline. This can be targeted in next release(can be maint release also).