liuhaipeng905 / servicestack

Automatically exported from code.google.com/p/servicestack
0 stars 1 forks source link

NativeRedisClient loses Database Context #17

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Set the DB context
2. Make a request to Redis (I'm using push )
3. Keep doing it
4. DB X, in my experience will get 0-1 of the pushes, DB 0 will then start 
getting everything.

What is the expected output? What do you see instead?
Database should only need to be set once. Database needs to be set everytime 
you make a request to ensure reliability

What version of the product are you using? On what operating system?
The version from Friday, June 18, 2010, Windows 7. Although I don't have the 
exact version, I am told by my coworker (who has worked with you on an issue 
during that release) that it is "1.3.7ish"

Please provide any additional information below.

What i'm doing is, using a server to receive some data from a client, put it 
into DB1 of a Redis instance, and then retrieve it via a secondary process 
after the fact. I'm creating my Redis instance via a Wrapper that acts only to 
expose the push/pop methods (a simple queue). It gets created once and is 
persisted for as long as the process remains alive, so the object sits in 
memory more than just a few seconds. 

The issue I'm running into is that the DB property on the RedisNativeClient 
only sets the DB once, but the connection appears to be recycled at some point 
(it doesn't appear as though it happens every time, but it does seem to 
happen). What I'll end up with is 0-1 entries in my chosen DB (1, in this 
case), and the rest in DB0.

The solution i have is to just re-set the DB property before every use of the 
class inside of my wrapper, but I figure you might want to look into persisting 
the selected DB on each connection being opened.

Original issue reported on code.google.com by smattke...@gmail.com on 30 Jun 2010 at 10:09

GoogleCodeExporter commented 9 years ago
I don't have a proper example to put, or I would have. 

I can provide a sample if need be, but its currently working in an httpmodule 
and a windows service, separately. So i can submit concise example when time 
permits.

Original comment by smattke...@gmail.com on 30 Jun 2010 at 10:54

GoogleCodeExporter commented 9 years ago
Hi are you using one of the Connection Pools? Because you will need to set it 
on the connection pool configuration as when the connection is recycled it 
resets itself to what was configured so that changes in connection state does 
not effect the next user.

Original comment by demis.be...@gmail.com on 30 Jun 2010 at 10:58

GoogleCodeExporter commented 9 years ago
I see what you're saying, we are using the PooledManager. The configuration is 
in some common code, so I can just make a separate configuration for the 
service/module.

So theres no way to ensure that a long-lived RedisClient is actually connected 
to the DB you've set, other than always setting the DB property before each 
request?

Original comment by smattke...@gmail.com on 1 Jul 2010 at 12:07

GoogleCodeExporter commented 9 years ago
ehh, The 'Db' property that is directly on the 'PooledRedisClientManager' (if 
that's what you're using) should be the only property you need to set. If after 
doing this you've been given a connection that is not configured to point to 
the configured Db, then that is a bug. Is that what you're seeing?

Original comment by demis.be...@gmail.com on 1 Jul 2010 at 12:11

GoogleCodeExporter commented 9 years ago
Well, It's the Db property on the RedisClient that's being created from the 
ClientManager GetClient call. 

The Db property is set when it's first created (So, the config is defaulting to 
zero, but after getting the Client, i set the Client.Db to my number), then the 
object is alive in memory for a long time. So it's the Client itself with the 
issue, not the Manager I'd say. I don't know if that's what you'd call a bug or 
not, but I'd say it's unexpected behavior.

But I have an easy enough work around regardless. Thanks for the help

Original comment by smattke...@gmail.com on 1 Jul 2010 at 12:42

GoogleCodeExporter commented 9 years ago
Hmmm, the state of the RedisClient should not be changed until its released 
back into the pool. This sounds like a bug and since its happening after a long 
time I think has to do with my 'Auto-Reconnect when Connection times out' 
logic. I'll take a look at it tomorrow.

Original comment by demis.be...@gmail.com on 1 Jul 2010 at 12:46

GoogleCodeExporter commented 9 years ago
Yea that's exactly what I was thinking as well.

When I looked into the code in your repo browser, I see that at line 133 of 
RedisNativeClient (SendCommand()) you're calling Connect() if it's not 
connected, and in Connect() you're just re-opening the connection, and not 
persisting the chosen DB.

I know in the CLI client you can call -i # (# being the DB#) before any command 
to force it to use the DB for that request - not sure if that's an avenue to 
look into or not.

Thanks for responding to the issue so quickly.

Original comment by smattke...@gmail.com on 1 Jul 2010 at 1:00

GoogleCodeExporter commented 9 years ago
Well I've uploaded a new version of the Redis Client (1.37) that saves and sets 
the last db when the Socket is null, although the normal Reconnect() logic 
already did this so I'm not sure if this was the problem. Anyways I've uploaded 
the new version of the Redis Client but as I've had problems with uploads 
taking a long time for the latest version to appear, I've also attached it to 
this comment.

Let me know if the problem still exists.

Original comment by demis.be...@gmail.com on 3 Jul 2010 at 11:58

Attachments:

GoogleCodeExporter commented 9 years ago
Sorry, I've been busy and haven't had a chance to test and confirm that this 
fixes my issue.

I'm going to run a test with the latest binaries shortly and I'll let you know 
my results

Original comment by smattke...@gmail.com on 7 Jul 2010 at 6:22

GoogleCodeExporter commented 9 years ago
Demis,

I've switched to the updated binaries (1.37) but I encounter a new problem.

When trying to store (using an IRedisClients .AddItemToList method) data, the 
redis client throws the following exception

Unknown reply on integer response: 43OK, sPort: 62007, LastCommand: 
at ServiceStack.Redis.RedisNativeClient.CreateResponseError(String error)
ServiceStack.Redis.RedisNativeClient.ReadInt()
ServiceStack.Redis.RedisNativeClient.SendExpectInt(Byte[][] cmdWithBinaryArgs)
ServiceStack.Redis.RedisNativeClient.RPush(String listId, Byte[] value)
ServiceStack.Redis.RedisClient.AddItemToList(String listId, String value)

 .... (The rest of the stacktrace is all my code)

The odd thing here is that this seems incorrect -  Not only does it seem 
strange it would be expecting an integer response from the operation, I can't 
find any mention of AddItemToList (only AddToList ) in your current Trunk. 
Similarly, the execution paths don't seem to line up either with the code I'm 
looking at (I can't follow the method calls in the code as they occured in the 
trace). Is it possible you uploaded the wrong dll? 

Inspecting it via the Object Browser also confirms that the RedisClient and 
IRedisClient use method names that are not currently used in your Trunk.

Original comment by smattke...@gmail.com on 7 Jul 2010 at 7:38

GoogleCodeExporter commented 9 years ago
Hi,

yeah it definitely sounds like you have an older version of the libraries. I 
renamed 'AddToList' as part of a refactor a long time ago. My guess is that 
because I'm using the same file name that googlecode is caching the wrong one. 
I've uploaded a new version to this comment with a different filename. 
Hopefully you will have better successs.

- Demis

Original comment by demis.be...@gmail.com on 7 Jul 2010 at 11:01

Attachments:

GoogleCodeExporter commented 9 years ago
Using the dll in your attachment, I find myself in the exact same issue I had 
yesterday - Upon attempting to use AddItemToList, it get the following 
exception:

"Unknown reply on integer response: 43OK, sPort: 56366, LastCommand: "

The port reported there is not the port my Redis Instance is configured to run 
off of, nor is it the port my web application is running off of.

Your portion of the StackTrace below:
ServiceStack.Redis.RedisNativeClient.CreateResponseError(String error) 
ServiceStack.Redis.RedisNativeClient.ReadInt()
ServiceStack.Redis.RedisNativeClient.SendExpectInt(Byte[][] cmdWithBinaryArgs) 
ServiceStack.Redis.RedisNativeClient.RPush(String listId, Byte[] value) 
ServiceStack.Redis.RedisClient.AddItemToList(String listId, String value)

Is it possible that maybe the version of redis being used is out of date, and 
is not supported by your driver?

Original comment by smattke...@gmail.com on 8 Jul 2010 at 11:46

GoogleCodeExporter commented 9 years ago
Hi, It sounds like a bug but I'm surprised my tests and my usage haven't picked 
it up already. It may have something to do with the redis-server, what version 
are you're using?

Otherwise can you submit a standalone unit test which exhibits this behaviour 
and I can try running it on my end to see if we get different results.

Original comment by demis.be...@gmail.com on 8 Jul 2010 at 12:57

GoogleCodeExporter commented 9 years ago
Yea sure, it should be pretty easy. I'm doing nothing more than using an 
IRedisClient from my ClientManagers.GetClient() method, and calling 
AddItemToList on it. I can put together a standalone unit test for it later 
tonight.

I believe I'm using 1.2.1, which is terribly outdated it seems. I'll try and 
update my local copy of Redis and see if that helps.

Original comment by smattke...@gmail.com on 8 Jul 2010 at 1:14

GoogleCodeExporter commented 9 years ago
I've updated to 1.2.6 (provided by you, thank you very much) but i still get 
the same exception:

"Unknown reply on integer response: 43OK, sPort: 59936, LastCommand: "

Again, unsure of where that port number is coming from.

ServiceStack.Redis.RedisNativeClient.CreateResponseError(String error)
ServiceStack.Redis.RedisNativeClient.ReadInt()
ServiceStack.Redis.RedisNativeClient.SendExpectInt(Byte[][] cmdWithBinaryArgs)  
ServiceStack.Redis.RedisNativeClient.RPush(String listId, Byte[] value)
ServiceStack.Redis.RedisClient.AddItemToList(String listId, String value)

Original comment by smattke...@gmail.com on 8 Jul 2010 at 1:32

GoogleCodeExporter commented 9 years ago
Don't worry about the srcPort, it's an internal debugging message that wont be 
the source of the problem (i.e. it gets dynamically allocated whenever you make 
a TCP/Socket connection).

Ok actually I've found the problem, they've switched the return value of 
LPUSH/RPUSH from a status code to an int reply > 1.26. So basically I need to 
change the code to handle both versions. Can you have a try with the build 
(v1.38) attached and tell me how you go?

Original comment by demis.be...@gmail.com on 8 Jul 2010 at 4:56

Attachments:

GoogleCodeExporter commented 9 years ago
I'd be glad to give it a go shortly and let you know my results.

Original comment by smattke...@gmail.com on 8 Jul 2010 at 4:58

GoogleCodeExporter commented 9 years ago
I'm happy to report this latest version fixes both the original issue (with the 
DB context being lost on a lost connection) and the new issue (Which i have 
dubbed "Push Craziness").

Thanks for the hard work and quick replies.

Original comment by smattke...@gmail.com on 8 Jul 2010 at 7:10

GoogleCodeExporter commented 9 years ago
Sweet, sweet. good to hear another issue bites the dust :)

Original comment by demis.be...@gmail.com on 8 Jul 2010 at 11:29