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

NHibernate fails if any error happens inside Redis provider for SLC #3415

Open
EngSayed opened this issue Aug 22, 2023 · 10 comments
Open

NHibernate fails if any error happens inside Redis provider for SLC #3415

EngSayed opened this issue Aug 22, 2023 · 10 comments

Comments

@EngSayed
Copy link

EngSayed commented Aug 22, 2023

NHibernate cannot proceed if any error happens inside Redis cache. I believe if any issue happens in caching then NHibernate should skip that and use Database instead.

I got timeout exception while big data was getting written into Redis which is fine (I needed to increase the timeout) but NHibernate should have just skipped it and continue to work normally.

@EngSayed EngSayed changed the title NHibernate fails if any error happens inside Redis NHibernate fails if any error happens inside Redis provider for SLC Aug 22, 2023
@fredericDelaporte fredericDelaporte transferred this issue from nhibernate/NHibernate-Caches Aug 22, 2023
@fredericDelaporte
Copy link
Member

Such a change would have to be done on NHibernate side for a global solution, not in the caches providers "one by one", so I transfer this issue.

I consider this request opinionated. Silently fallback-ing on the database would hide troubles occurring in the cache layer, delaying detection of those troubles. In most cases, it is best to not hide away any error, to allow for quicker detection and fix.

At best, such a feature would have to be implemented as an option, disabled by default, to be enabled by who whishes such a behavior.

@EngSayed
Copy link
Author

In general if cache is not available that should not be a failure. In most cases, applications should survive even if cache is not available.

@bahusoid
Copy link
Member

In most cases, applications should survive even if cache is not available.

That's quite naive expectation.Resilient re-connection is not an easy task and in most cases it's the job of separate libraries.

@EngSayed
Copy link
Author

In most cases, applications should survive even if cache is not available.

That's quite naive expectation.Resilient re-connection is not an easy task and in most cases it's the job of separate libraries.

That's not true. Second level cache is not the main persistent of data but database so having cache is not mandatory and can be disabled but stopping whole execution because a failure inside the cache is a bad process. If we try to read data from cache first but failed then it should silently read from DB. If a process is trying to update data inside DB but fails to connect to cache then it should not fail the main process

@EngSayed
Copy link
Author

At least it should be an option to turn off NHibernate failing in SLC fail. Otherwise the main process always fail because some issue with Cache connection

@bahusoid
Copy link
Member

bahusoid commented Aug 22, 2023

If DB transaction is involved than cache time out can lead to DB transaction timeout. All I'm saying that properly implemented requested feature should handle this case too. And I don't see it as NHibernate job.

@EngSayed
Copy link
Author

EngSayed commented Aug 23, 2023

NHibernate needs to be able to fall back silently to Database if that option falg is enabled to continue execution when cache fails. That's NHibernate job. Otherwise NHibernate Cache must handle it for all providers.
I will leave this for you guys to decide @fredericDelaporte, @bahusoid.
But I request that it gets higher priority than minor as it is stopping the main process to be executed. So in reality this is major not minor.

@daverein
Copy link

In most cases, applications should survive even if cache is not available.

That's quite naive expectation.Resilient re-connection is not an easy task and in most cases it's the job of separate libraries.

Like what separate libraries?

@fredericDelaporte
Copy link
Member

fredericDelaporte commented Aug 23, 2023

Thinking about it again, I do neither think it belongs to NHibernate. You should ask your Redis client for such a feature instead: yield a cache miss instead of an error in case it cannot join other nodes.

NHibernate cannot have knowledge of whether a cache failure is a transient failure that it could "safely" ignore by short-circuiting to DB instead, or is a more systemic failure that should not be ignored. The Redis client would be far more appropriate for such a feature. And I am not speaking about the Redis provider in NHibernate cache project: it has also no knowledge of the internals of the Redis client. That is really a job for the underlying Redis client. (Which by the way is the answer to "Like what separate libraries?".)

Of course in case of a timeout, that will likely incur a double timeout: fetch failure then put failure.

By the way, I do not think ignoring a cache failure can actually be safe in any circumstance. Silently fall-backing to DB on cache failure can cause a database load surge, which will cause other failures. And the actual cause of those other failures (cache failure in the first place) will be harder to diagnose, due to the cache failure being silently ignored.

(If the health of the distributed cache is not critical to your application, it may be worth re-considering whether the application actually needs that additional layer of complexity, or could use a simpler one (non-distributed cache), or do not even really need it.)

@fredericDelaporte
Copy link
Member

That said, the provider and NHibernate being open source, that should be not hard for you to derive your own provider with very little code to add some catch-all-yield-null logic in it.

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

4 participants