csredis icon indicating copy to clipboard operation
csredis copied to clipboard

Unexpected response type: 56 (expecting Bulk)

Open bonesoul opened this issue 11 years ago • 11 comments

[Error] [HybridStorageLayer] [Lottocoin] An exception occured while comitting share: Unexpected response type: 56 (expecting Bulk) Fatal

Working with latest develop tree, the offending code;

try
            {
                if (!IsEnabled || !_redisProvider.IsConnected)
                    return;

                //_client.StartPipe(); // batch the commands.

                // add the share to round 
                var currentKey = string.Format("{0}:shares:round:current", _coin);
                _redisProvider.Client.HIncrByFloat(currentKey, share.Miner.Username, share.Difficulty);

                // increment shares stats.
                var statsKey = string.Format("{0}:stats", _coin);
                _redisProvider.Client.HIncrBy(statsKey, share.IsValid ? "validShares" : "invalidShares", 1);

                // add to hashrate
                if (share.IsValid)
                {
                    var hashrateKey = string.Format("{0}:hashrate", _coin);
                    var entry = string.Format("{0}:{1}", share.Difficulty, share.Miner.Username);
                    _redisProvider.Client.ZAdd(hashrateKey, Tuple.Create(TimeHelpers.NowInUnixTime(), entry));
                }

                //_client.EndPipe(); // execute the batch commands.
            }
            catch (Exception e)
            {
                _logger.Error("An exception occured while comitting share: {0:l}", e.Message);
            }

bonesoul avatar Sep 03 '14 09:09 bonesoul

Do you know which command caused the exception and with what arguments?

I'm able to execute each of HIncrByFloat, HIncrBy, ZAdd without error.

ctstone avatar Sep 03 '14 10:09 ctstone

I'm not sure about it but seems most commands throw the exception, and FYI again mono issue with latest develop.

[Error] [HybridStorageLayer] [Lottocoin] An exception occured while comitting share: Unexpected response type: 10 (expecting Bulk)
 [Lottocoin] An exception occured while getting shares for current round: Unexpected response type: 48 (expecting MultiBulk)
 [Error] [HybridStorageLayer] [Lottocoin] An exception occured while getting previous balances: Unexpected response type: 51 (expecting Bulk)

bonesoul avatar Sep 03 '14 11:09 bonesoul

Unable to reproduce on Linux+Mono either. Could you create a simple test case? E.g. csredis.ZAdd("test", Tuple.Create(123.456, "xyz")) if the issue is isolate to floating point numbers.

Also, what Redis server version are you running?

ctstone avatar Sep 03 '14 11:09 ctstone

# Server
redis_version:2.8.4
redis_git_sha1:00000000
redis_git_dirty:0
redis_build_id:a44a05d76f06a5d9
redis_mode:standalone
os:Linux 3.13.0-24-generic x86_64

bonesoul avatar Sep 03 '14 13:09 bonesoul

this commit works all good mono 3.2.8 -> https://github.com/ctstone/csredis/commit/6667063d156bf04673449996b4f3560918631c4c

should be some other commit that offends

bonesoul avatar Sep 03 '14 13:09 bonesoul

Probably you are right, we are having a floating point numbers issue again after your RedisWriter & RedisReader refactor.

probably these lines are offending; they should be using - CultureInfo.InvariantCulture

  • https://github.com/ctstone/csredis/blob/master/CSRedis/Internal/IO/RedisReader.cs#L93
  • https://github.com/ctstone/csredis/blob/master/CSRedis/Internal/IO/RedisWriter.cs#L34

So basically we have to set RedisReader and RedisWriter's encoding to Invariant

And basically you don't get the errors because you are probably running en-US system. Any other encoding will break floating numbers - we have to make sure we use invariant culture to get stuff working on all systems.

That was what I had done actually with https://github.com/ctstone/csredis/commit/6667063d156bf04673449996b4f3560918631c4c and that's why it was working.

bonesoul avatar Sep 03 '14 13:09 bonesoul

Encoding should not care about Culture, I think. We are just mapping between byte[] and String (default is Encoding.UTF8). I'm curious if you try to set client.Encoding = new UTF8Encoding(false) if it has any effect.

Here is where I moved the CultureInfo.InvairantCulture fix you provided: https://github.com/ctstone/csredis/blob/develop/CSRedis/Internal/IO/RedisWriter.cs#L49

So each argument of every command is wrapped in ToString+Invariant.

Also, you can see here we are still using InvariantCulture on the current thread by default, so en-US should not be a factor: https://github.com/ctstone/csredis/blob/develop/CSRedis/RedisClient.cs#L168

It's still possible I missed something in the refactor, so any repro cases you can provide would be very helpful.

ctstone avatar Sep 03 '14 13:09 ctstone

my initial fix was the same as; https://github.com/ctstone/csredis/blob/develop/CSRedis/RedisClient.cs#L168

but this can not fix all the issues so I had to go in for detailed changes.

anyway I'll be debugging the code to see the problem.

bonesoul avatar Sep 03 '14 13:09 bonesoul

I am getting the same issue using async connections. It happens when I use the "Exists" function. Otherwise too, it hangs after 20 or so calls in quick successions (async). Initially I assumed its something to do with memory on the redis server, but that doesnt seem to be the case. Please help.

anup8000 avatar Sep 08 '14 23:09 anup8000

Please send a small repeatable sample code block that I can use to reproduce the issue. Preferably with no variables in the Redis call arguments.

On Mon, Sep 8, 2014 at 7:08 PM, anup8000 [email protected] wrote:

I am getting the same issue using async connections. It happens when I use the "Exists" function. Otherwise too, it hangs after 20 or so calls in quick successions (async). Initially I assumed its something to do with memory on the redis server, but that doesnt seem to be the case. Please help.

— Reply to this email directly or view it on GitHub https://github.com/ctstone/csredis/issues/23#issuecomment-54901040.

ctstone avatar Sep 08 '14 23:09 ctstone

@anup8000 on mono or .net?

bonesoul avatar Sep 09 '14 08:09 bonesoul