8000 Update HadExceptions if socket not connected by joplaal · Pull Request #184 · ServiceStack/ServiceStack.Redis · GitHub
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Dec 24, 2022. It is now read-only.

Update HadExceptions if socket not connected#184

Merged
mythz merged 1 commit intoServiceStack:masterfrom
joplaal:master
Oct 17, 2013
Merged

Update HadExceptions if socket not connected#184
mythz merged 1 commit intoServiceStack:masterfrom
joplaal:master

Conversation

@joplaal
Copy link
Contributor
@joplaal joplaal commented Oct 16, 2013

When ConnectTimeout != 0, it does not throw the exception, and
HadExceptions it is not updated. Due to that, the connection is not
disposed and the pool can not recover after a server crash

When ConnectTimeout != 0, it does not throw the exception, and
HadExceptions it is not updated. Due to that, the connection is not
disposed and the pool can not recover after a server crash
@joplaal
Copy link
Contributor Author
joplaal commented Oct 16, 2013

I'm not sure about this, it's only the way I think I can solve an issue I'm experiencing. Using a PooledRedisClientManager, with ConnectTimeOut != 0, when the server crashes, or it's restarted, the clients in the pool remain in a unstable state, since all the clientes are not disposed (since HadExceptions is not updated)

What do you think about? Sorry. but I do not know the internals of the project

@joplaal
Copy link
Contributor Author
joplaal commented Oct 16, 2013

Sorry, but I think that my issue is related to the DefaultIdleTimeOutSecs and the RedisNativeClient AssertConnectedSocket() method. If the the server is restarted, and this method is called before the IdleTimeOutSeconds, the socket is not connected but Reconnect() is not called, so it remains closed forever. The only one solution I see now for handling this issue, is to set the IdleTimeOutSeconds very close to 0.

Thanks for your incredible work!

@joplaal
Copy link
Contributor Author
joplaal commented Oct 16, 2013

I've just read #183 !

@mythz
Copy link
Member
mythz commented Oct 17, 2013

OK yeah sounds good for now. Can you approve the Contributor License Agreement so we can accept this, thx.

@joplaal
Copy link
Contributor Author
joplaal commented Oct 17, 2013

Done!

mythz added a commit that referenced this pull request Oct 17, 2013
Update HadExceptions if socket not connected
@mythz mythz merged commit 8d91472 into ServiceStack:master Oct 17, 2013
@mythz
Copy link
Member
mythz commented Oct 17, 2013

Great, thx 👍

@jeffgabhart
Copy link

@mythz Can we get this in v3 as well?

@mythz
Copy link
Member
mythz commented Nov 6, 2013

@jeffgabhart Sure, just as soon as someone sends a pull-req to the v3 branch? ;)

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.

3 participants

0