Skip to content
This repository has been archived by the owner on Nov 16, 2022. It is now read-only.

Change from lazy to double check locking to avoid caching exceptions when initialize #49

Closed
wants to merge 1 commit into from

Conversation

mknet3
Copy link

@mknet3 mknet3 commented Oct 20, 2020

Fix #48

Avoiding change LazyThreadSafetyMode to avoid concurrent problems. Instead of will be use double check locking.

@AliBazzi
Copy link
Owner

Hi @m-knet can you elaborate why you didn't use what is suggested in the Issue you've opened ?

your current implementation in this PR will delay the connection creation until the first time the multiplexer is needed, while the current implementation will attempt to connect on the initialisation, why not just apply what you've suggested ?

@mknet3
Copy link
Author

mknet3 commented Nov 1, 2020

Hi @AliBazzi, I didn't used LazyThreadSafetyMode because I was wrong about the default value of this property. The default value is ExecutionAndPublication. This means that lazy initialization is thread safe but is caching the exception if fails when is initialized.

None is not thread safe and is better avoid it.

PublicationOnly is not caching the exception and is thread safe. It was the first option to choose (seems that fit with solution). But this solution has a problem: The first thread to complete initialization sets the value of the Lazy instance.. This means that can have multiple threads trying to initialize the instance and trying to open connections with Redis.

@AliBazzi
Copy link
Owner

Hi @m-knet ,

Thanks for the clarification, but still, your changes is behavioural changes to the library, for example, in my case, I do want the instance to fail on initialisation if it was not successful in connecting to Redis, you are differing this to the first connection attempt, and that will not fail the instance.

So in my use case, I do want the instance to fail and report unhealthy if it wasn't able to connect to Redis on initialisation.

@AliBazzi AliBazzi closed this Feb 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lazy initialization of ConnectionMultiplexer is not working if throw exception first time
2 participants