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

async command callback invoking policy when disconnecting #57

Open
redboltz opened this issue Apr 16, 2018 · 6 comments
Open

async command callback invoking policy when disconnecting #57

redboltz opened this issue Apr 16, 2018 · 6 comments

Comments

@redboltz
Copy link
Contributor

redboltz commented Apr 16, 2018

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 f55ffdc.

#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
Copy link
Owner

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
Copy link
Contributor Author

@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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

redboltz commented May 4, 2018

I understand. Thank you for the comment.

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

2 participants