async command callback invoking policy when disconnecting

[redboltz]

I have a question about the policy of invoking callback handler.

When I call RedisAsyncClient::command() during disconnected, callback function doesn't seems to be called. Is that intentional specification?

Here is the small example code that demonstrates the behavior. I've tested it with https://github.com/nekipelov/redisclient/commit/f55ffdcafdcb3cefeec2214e2c19ba6a8f2d179c.

#include <string>
#include <iostream>

#include <boost/asio/ip/address.hpp>

#include <redisclient/redisasyncclient.h>

int main() {
    boost::asio::ip::address address = boost::asio::ip::address::from_string("127.0.0.1");
    const unsigned short port = 6379;
    boost::asio::ip::tcp::endpoint endpoint(address, port);

    boost::asio::io_service ioService;
    redisclient::RedisAsyncClient redis(ioService);
    boost::system::error_code ec;

    redis.connect(
        endpoint,
        [&]
        (auto const& ec) {
            if(ec) {
                std::cerr << __LINE__ <<
                    ": Can't connect to redis: " << ec.message() << std::endl;
                return;
            }
            std::cout << __LINE__ <<
                ": Please restart redis service within 5 seconds for testing!" << std::endl;
            sleep(5);

            std::cout << __LINE__ <<
                ": do async command" << std::endl;
            redis.command(
                "SET",
                {"key", "value"},
                [&]
                (auto const& result) {
                    if( result.isError() )
                    {
                        std::cerr << __LINE__ <<
                            ": SET error: " << result.toString() << "\n";
                        return ;
                    }
                    std::cout << __LINE__ <<
                        ": result: " << result.toString() << std::endl;
                }
            );
            std::cout << __LINE__ <<
                ": finish async command" << std::endl;
        }
    );

    std::function<void(boost::system::error_code const&)> handler;

    handler =
        [&handler, &redis, &endpoint]
        (boost::system::error_code const& ec) {
        if (ec) {
            std::cout << __LINE__ <<
                ": ec: " << ec << std::endl;
            redis.disconnect();
            std::cout << __LINE__ <<
                ": trying reconnect inner..." << std::endl;
            sleep(1);
            redis.connect(
                endpoint,
                handler);
        }
        else {
            std::cout << __LINE__ <<
                ": reconnect success" << std::endl;
        }
    };

    redis.installErrorHandler(
        [&](std::string const& ec) {
            std::cout << __LINE__ <<
                ": ec: " << ec << std::endl;
            std::cout << __LINE__ <<
                ": trying reconnect..." << std::endl;
            redis.disconnect();
            redis.connect(
                endpoint,
                handler
            );
        }
    );

    ioService.run();
}

When I run the program, the following message is appeared:

26: Please restart redis service within 5 seconds for testing!

Then if I do nothing within 5 seconds, I got the following message:

30: do async command
47: finish async command
43: result: OK

The last line (43) indicates that RedisAsyncClient::command() callback is called.

If I restart redis service within 5 seconds, I got the following message:

26: Please restart redis service within 5 seconds for testing!

I do sudo systemctl restart redis.service.

30: do async command
47: finish async command
76: ec: End of file
78: trying reconnect...
69: reconnect success

That indicates that RedisAsyncClient::command() callback is NOT called. But the handler that is installed using Redis::installErrorHandler() is called.

Is that expected behavior?

I want to implement automatic re-execute RedisAsyncClient::command() mechanism if the redis connection is disconnected.

In order to do that, I think that the following mechanism is required:

1. Prepare a queue.
2. Before calling RedisAsyncClient::command(), I copy the contents of the command and push it to the queue.
3. If the RedisAsyncClient::command() callback is called, erase the pushed command. If installed error handler is called, reconnect to redis, and when reconnected, call RedisAsyncClient::command() for each commands in the queue.

Before I implement it in my client program, I'd like to clarify the library spec.

[nekipelov]

Yes, this is expected behavior. But this is wrong and I want the handler to be called in case of errors. RedisSyncClient already has the argument boost::system::error_code.

[redboltz]

@nekipelov , thank you for the answer.

You mean RedisAsyncClient::command()'s handler will have boost::system::error_code some how in the future?

For example, the current RedisAsyncClient::command() is this,

https://github.com/nekipelov/redisclient/blob/v0.6.1/src/redisclient/redisasyncclient.h#L66

    REDIS_CLIENT_DECL void command(
            const std::string &cmd, std::deque<RedisBuffer> args,
            std::function<void(RedisValue)> handler = dummyHandler);

and RedisAsyncClient::connect()''s handler has boost::system::error_code. https://github.com/nekipelov/redisclient/blob/v0.6.1/src/redisclient/redisasyncclient.h#L40

    REDIS_CLIENT_DECL void connect(
            const boost::asio::ip::tcp::endpoint &endpoint,
            std::function<void(boost::system::error_code)> handler);

So RedisAsyncClient::command()'s handler will be something like this, or expand the error case of RedisValue.

    REDIS_CLIENT_DECL void command(
            const std::string &cmd, std::deque<RedisBuffer> args,
            std::function<void(RedisValue, boost::system::error_code)> handler = dummyHandler);

Anyway, if disconnect is detected, both the installed error handler by Redis::installErrorHandler() and RedisAsyncClient::command()'s handler with an error code in the future. Is that right?

[nekipelov]

Redis::installErrorHandler() will be removed.

RedisAsyncClient::command() handler will be something like this:

 REDIS_CLIENT_DECL void command(
            const std::string &cmd, std::deque<RedisBuffer> args,
            std::function<void(RedisValue, boost::system::error_code)> handler);

[redboltz]

What happens in the following case?

1. RedisAsyncClient::connect() is successfully finished. RedisAsyncClient::connect() callback is called.
2. In the RedisAsyncClient::connect() callback, RedisAsyncClient::command() is NOT called. Instead of that, set boost::asio::deadline_timer and called boost::asio::async_wait(). In the boost::asio::async_wait() handler, RedisAsyncClient::command() will be called.
3. Waiting timeout, but the timer isn't fired yet.
4. redis connection is disconnected by some reason.

Now, noRedisAsyncClient::command() is processing. I think that in this case, Redis::installErrorHandler() is the way to know and handle disconnection. If it will be removed, how do I know that?

[nekipelov]

You can use RedisAsyncClient::command("PING") if you want to detect disconnection early.

Redis server can be excluded from cluster. Someone can unplugging ethernet wire from server. Implementation with installErrorHandler can not detect many problems.

[redboltz]

I understand. Thank you for the comment.