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

Trying to enable to selection of clustered or non clustered when I bootup my program #170

Closed
ShadowKai opened this issue Feb 17, 2021 · 18 comments

Comments

@ShadowKai
Copy link

ShadowKai commented Feb 17, 2021

I'm trying to get my project to run clustered/non-clustered based on a specific configuration when I start the program which the value will be stored into the variable mRedisClusterEnable, the value is not changeable after initialization.
This is how I initialize it, the mMyRedisC and mMyRedis are pointers of their respective classes (RedisCluster and Redis) which I only initialize after checking the value mRedisClusterEnable.

My Initialization:
sw::redis::ConnectionOptions connection_options;
connection_options.host = mRedisServer.mRedisSvrCfg.mRedisIp;
connection_options.port = mRedisServer.mRedisSvrCfg.mRedisPort;

sw::redis::ConnectionPoolOptions pool_options;
pool_options.wait_timeout = std::chrono::milliseconds(100);
pool_options.connection_lifetime = std::chrono::minutes(1);

if (this->mRedisClusterEnable == 1)
{
this->mMyRedisC = new sw::redis::RedisCluster(connection_options, pool_options);
if (this->mMyRedisC == NULL)
return -1;
}
else
{
this->mMyRedis = new sw::redis::Redis(connection_options, pool_options); //<--------- over here
if (this->mMyRedis == NULL)
return -1;
}

my get and set function is as follows:
int MyRedisServer::Set(std::string name, std::string value, int cache_expiry)
{
if (!value.empty() && !name.empty())
{
try
{
if (this->mRedisClusterEnable == 1)
{
if (cache_expiry == 0)
mMyRedisC->set(name.c_str(), value.c_str());
else
mMyRedisC->set(name.c_str(), value.c_str(), std::chrono::milliseconds(cache_expiry1000));
}
else
{
if (cache_expiry == 0)
mMyRedis->set(name.c_str(), value.c_str());
else
mMyRedis->set(name.c_str(), value.c_str(), std::chrono::milliseconds(cache_expiry
1000));
}
return 0;
}
catch (const sw::redis::Error &err){}
}
return -1;
}

int MyRedisServer::Get(std::string name, std::string* output) const
{
try
{
if (this->mRedisClusterEnable == 1)
{
auto val = mMyRedisC->get(name.c_str());
if (val)
{
output->assign(*val);
return 0;
}
else
output->assign("");
}
else
{
auto val = mMyRedis->get(name.c_str());
if (val)
{
output->assign(*val);
return 0;
}
else
output->assign("");
}
}
catch (const sw::redis::Error &e) {}
return -1;
}

However when I call the functions Set/Get with mRedisClusterEnable=1 (it's not interchangeable and it's fixed at the start of the program) my program resulted in a coredump. However, when mRedisClusterEnable=0, it has no issue. I tried commenting out the initialize part (this part "//<--------- over here") for the non-clustered when my mRedisClusterEnable=1 and then the coredump no longer occurs and I'm getting the correct output from my Set/Get functions.
This is confusing because when mRedisClusterEnable=1 it shouldn't even go through the non-clustered part so i'm suspecting the issue is somewhere else but I have no idea where to start.

@sewenew
Copy link
Owner

sewenew commented Feb 17, 2021

@ShadowKai The code looks good to me. It might be some problem elsewhere. I suggest you writing a simple test that only contains the definition of MyRedisServer, and test code. Also, you need to catch possible exceptions when you create Redis and RedisCluster object:

try {
    mMyRedisC = new sw::redis::RedisCluster(connection_options, pool_options);
} catch (const Error &e) {
    // handle error, e.g. print e.what()
}

Regards

@ShadowKai
Copy link
Author

@sewenew I added try catch exceptions when i create Redis and RedisCluster object, it doesn't show anything when I start it.

This is my coredump log:
#0 get (this=0x8) at /usr/include/c++/4.8.2/bits/unique_ptr.h:234
234 { return std::get<0>(_M_t); }
Missing separate debuginfos, use: debuginfo-install glibc-2.17-157.el7.x86_64 keyutils-libs-1.5.8-3.el7.x86_64 krb5-libs-1.15.1-37.el7_6.x86_64 libcom_err-1.42.9-13.el7.x86_64 libgcc-4.8.5-39.el7.x86_64 libselinux-2.5-14.1.el7.x86_64 libstdc++-4.8.5-39.el7.x86_64 openssl-libs-1.0.2k-16.el7.x86_64 pcre-8.32-17.el7.x86_64 zlib-1.2.7-18.el7.x86_64
(gdb) bt'
#0 get (this=0x8) at /usr/include/c++/4.8.2/bits/unique_ptr.h:234
#1 release (this=0x8) at /usr/include/c++/4.8.2/bits/unique_ptr.h:251
#2 unique_ptr (
__u=<unknown type in /lib64/libredis++.so, CU 0x53fa5, DIE 0x66467>,
this=0x7fd5cb20fda0) at /usr/include/c++/4.8.2/bits/unique_ptr.h:161
#3 Connection (this=0x7fd5cb20fda0)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/connection.h:114
#4 sw::redis::ConnectionPool::_fetch (this=this@entry=0x24b0e50)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/connection_pool.cpp:228
#5 0x00007fd5cdd2a256 in sw::redis::ConnectionPool::fetch (this=0x24b0e50)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/connection_pool.cpp:93
#6 0x00007fd5cdd40ded in SafeConnection (pool=..., this=0x7fd5cb210000)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/connection_pool.h:118
#7 sw::redis::RedisCluster::_command<void ()(sw::redis::Connection&, sw::redis::StringView const&, sw::redis::StringView const&, long long, sw::redis::UpdateType), sw::redis::StringView const&, sw::redis::StringView const&, long, sw::redis::UpdateType&>(void ()(sw::redis::Connection&, sw::redis::StringView const&, sw::redis::StringView const&, long long, sw::redis::UpdateType), sw::redis::StringView const&, sw::redis::StringView const&, sw:---Type to continue, or q to quit---bt'
, sw::redis::UpdateType&) (this=0x24b0410, cmd=
0x7fd5cdd1dff0 <sw::redis::cmd::set(sw::redis::Connection&, sw::redis::StringView const&, sw::redis::StringView const&, long long, sw::redis::UpdateType)>, key=...)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/redis_cluster.hpp:1290
#8 0x00007fd5cdd3d6b5 in _command<void ()(sw::redis::Connection&, sw::redis::StringView const&, sw::redis::StringView const&, long long, sw::redis::UpdateType), sw::redis::StringView const&, long, sw::redis::UpdateType&> (key=...,
cmd=, this=)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/redis_cluster.hpp:1252
#9 command<void (
)(sw::redis::Connection&, sw::redis::StringView const&, sw::redis::StringView const&, long long, sw::redis::UpdateType), sw::redis::StringView const&, sw::redis::StringView const&, long, sw::redis::UpdateType&> (
key=..., cmd=, this=)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/redis_cluster.hpp:37
#10 sw::redis::RedisCluster::set (this=, key=..., val=...,
ttl=..., type=sw::redis::ALWAYS)
at /home/unified/builds/kai/lib/redis-plus-plus/src/src/sw/redis++/redis_cluster.cpp:260
#11 0x0000000000418cc8 in MyRedisClusterServer::Set (this=0x2193070,
---Type to continue, or q to quit---bt'
name="abc", value="abc=123;efg=456;", cache_expiry=10)
at src/MyRedisClusterServer.cc:89

@sewenew
Copy link
Owner

sewenew commented Feb 18, 2021

@ShadowKai As I mentioned above, you'd better give me a simple test code, i.e. only have redis-plus-plus related code, that can reproduce the problem. Otherwise, the problem might be some elsewhere.

Regards

@Sharonlalala
Copy link

I got the similar problem about core dump. Did you find a solution for it? @ShadowKai

@sewenew
Copy link
Owner

sewenew commented Feb 23, 2021

@Sharonlalala Can you give me the code that can reproduce your problem? And what's your version of hiredis and redis-plus-plus? Also please check if you installed multiple hiredis versions? Thanks!

Regrads

@Sharonlalala
Copy link

image
I used the code you wrote in README. I found that if I use Redis redis("tcp://127.0.0.1:6379"), it can successfully executed. But if I use ConnectionOptions to create a redis node or a cluster. It will failed.

@Sharonlalala
Copy link

image
I got this error when I use Redis redis(connection_options) to connect to 127.0.0.1:6379

@sewenew
Copy link
Owner

sewenew commented Feb 23, 2021

@Sharonlalala Did you enable TLS support when installing redis++, while you didn’t enable TLS support when installing hiredis? Please check the doc to enable both or neither.

@Sharonlalala
Copy link

Sharonlalala commented Feb 24, 2021

image
Hi, @sewenew thank you for your quick support! I enabled TLS for both redis++ and hiredis again. But still got the error that cannot use functions in hiredis.

@sewenew
Copy link
Owner

sewenew commented Feb 24, 2021

@Sharonlalala Since you enabled TLS support, you also need to link libhiredis_ssl.a, -lssl and -lcrypto:

g++ -std=c++11 -I/path/to/hiredis-header -I/path/to/redis-plus-plus-header -o demo main.cpp /path/to/libhiredis.a /path/to/libhiredis_ssl.a /path/to/libredis++.a -pthread -lssl -lcrypto

Please check the doc for detail.

@Sharonlalala
Copy link

Thank you so much for your help! @sewenew It can run now! The only problem is when I use cluster with setting more than 5MB value, the cluster will down. Do you know why that happened? I got same problem when using benchmark. I've changed each node's maxmemory to 100G.
image

@sewenew
Copy link
Owner

sewenew commented Feb 25, 2021

@Sharonlalala First of all, you should never try to set a large key, e.g. 5MB, since it might block Redis for a while, and have a huge performance penalty. This seems not a redis-plus-plus problem. Maybe one node is blocking on the large key setting work, and cannot communicate with other nodes in the cluster. So others thought this node is down, and reply with client with CLUSTERDOWN error. You'd better ask a Redis expert to dig into this problem.

@Sharonlalala
Copy link

Ok, I got it. Thank you very much! @sewenew

@ShadowKai
Copy link
Author

ShadowKai commented Feb 26, 2021

@ShadowKai As I mentioned above, you'd better give me a simple test code, i.e. only have redis-plus-plus related code, that can reproduce the problem. Otherwise, the problem might be some elsewhere.

hi, sorry for the late reply. I was met with a personal emergency.
anyway, this is a simple test code.

Regards
image

this is the output.
image

also I have enabled TLS support but am currently not using it for this testcode and I have linked with
-lpthread -lredis++ -lhiredis -lhiredis_ssl -lssl -lcrypto

@sewenew
Copy link
Owner

sewenew commented Feb 26, 2021

@ShadowKai You code cannot compile. I modified it as follows:

#include <iostream>
#include <string>
#include <thread>
#include <sw/redis++/redis++.h>

using namespace std;
using namespace sw::redis;

int main() {
    try {
        ConnectionOptions opts;
        opts.host = "127.0.0.1";
        opts.port = 8000;
        ConnectionPoolOptions pool_opts;
        pool_opts.size = 1;
        auto *r = new Redis(opts, pool_opts);

        opts.host = "127.0.0.1";
        opts.port = 9000;
        auto *c = new RedisCluster(opts, pool_opts);
        r->set("aaa", "111");
        auto out = r->get("aaa");
        cerr << "redis: " << *out << endl;
        c->set("bbb", "111");
        out = c->get("bbb");
        cerr << "redis cluster: " << *out << endl;
    } catch (const Error &e) {
        cerr << e.what() << endl;
        return -1;
    } catch (const std::exception &e) {
       cerr << e.what() << endl;
       return -1;
    }
    return 0;
}

However, I cannot reproduce your problem with the above code. In my test, both Redis and Redis Cluster are alive, and the test can output all results.

You can try the above code to test if it fails. Also try to catch possible std::exception to see if there's any exception thrown. And also tell me if both your Redis server and Redis Cluster are alive.

B.T.W. what's your version of redis-plus-plus and hiredis?

@ShadowKai
Copy link
Author

@sewenew , I tried compiling your code and I get the same issue, which coredumps when trying to execute set on cluster. If I comment out the non cluster, it works correctly. I added the catch (const std::exception &e) and there isn't any exception thrown as well.

both Redis server and Redis Cluster are alive. I can connect to both and execute commands using redis-cli.

redis-plus-plus version is v1.2.1
hiredis version is v1.0.0

@sewenew
Copy link
Owner

sewenew commented Mar 1, 2021

@ShadowKai Can you try the latest code of redis-plus-plus, i.e. code on master branch or v1.2.2? I fixed a crash problem recently. However, that's a problem on Pipeline or Transaction. If your code doesn't use Pipeline or Transaction, and the Redis instances are always alive, you should not get the problem.

Also please check if you installed multiple versions of hiredis. If you do, you need uninstall those extra ones, and only keep one hiredis version installed. Otherwise, you might get some wired problems.

If you still have problem with it, can you give me a docker image which can reproduce the problem. So that I can help to debug.

@ShadowKai
Copy link
Author

@ShadowKai Can you try the latest code of redis-plus-plus, i.e. code on master branch or v1.2.2? I fixed a crash problem recently. However, that's a problem on Pipeline or Transaction. If your code doesn't use Pipeline or Transaction, and the Redis instances are always alive, you should not get the problem.

Also please check if you installed multiple versions of hiredis. If you do, you need uninstall those extra ones, and only keep one hiredis version installed. Otherwise, you might get some wired problems.

If you still have problem with it, can you give me a docker image which can reproduce the problem. So that I can help to debug.

@sewenew, thanks for your help, it seems to be because i have multiple hiredis installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants